Sunday, June 6, 2010

[android-developers] Re: Calling wait in AsyncTask (e.g. inside doInBackground) raises IllegalMonitorStateException

While the others who responded gave you good advice, and also touched
on what I'm about to tell you, I'd like to direct your attention to
the documentation for the wait() call.

"The current thread must own this object's monitor. The thread
releases ownership of this monitor and waits until another thread
notifies threads waiting on this object's monitor to wake up either
through a call to the notify method or the notifyAll method. The
thread then waits until it can re-obtain ownership of the monitor and
resumes execution.

This method should only be called by a thread that is the owner of
this object's monitor. See the notify method for a description of the
ways in which a thread can become the owner of a monitor.

Throws:
IllegalMonitorStateException - if the current thread is not the owner
of the object's monitor."

Now, if you haven't read the documentation for the method, you're
clearly not presently prepared to take on Java threading. That's not a
problem, I'm just emphasizing you should follow the advice, and avoid
writing thread-based synchronization yourself unless absolutely
necessary (and if it really is necessary, be sure you understand it
well, and test it well).

It's not enough to know the primitives like wait and notify, or the
concurrency package. You also need to know very specific usage
patterns to reliably get the result you want.

And then, knowing them, you have to actually do them correctly. And on
a project with other people, hope they don't later screw them up.

Where I'm going with this is -- any effort you were thinking of
putting into learning how to use wait() / notify() is probably better
spent in figuring out how NOT to use them, but something higher level
instead.

For example, with if you are using a producer/consumer pattern, why
not use a LinkedBlockingQueue to communicate between them? If the
consumer goes blocked, then the producer can continue working, up to
some limit, resulting in better throughput. Yes, you can use notify/
wait to implement this -- and yes, it's not really all that hard to
do, if you know what you're doing. But it's a whole lot clearer
without the queuing implementation being part of your program. One
thread starts up and stuffs things into the queue in a loop. The other
thread starts up and reads things from the queue in a loop. Easy to
write, and easy to read.

The tricky part, though, is handling when the threads should go away,
in event of an error on the other side. A flag per side and
synchronize can tell each side to exit its loop, but making sure that
if one side exits abnormally while the other side is waiting on it,
that the other side is not left hanging, is harder. On the receive
side you have to drain the queue so the sender side can return from
the put() call. On the send side, you have to put something distinct
so the receiver will return from the take() call, and notice that the
sender has gone a way.

To my mind, this is a design failure of the java.util.concurrent
package. There should be a abort() method on all the blocking objects,
which causes anything waiting on it to throw an unchecked exception.
Then this simple pattern would suffice:

// Queue for both sides
LinkedBlockingQueue<MyElt> q = new
LinkedBlockingQueue<MyElt>(CAPACITY);
...
boolean ok = false;
try {
doSomething(q);
ok = true;
} catch (QueueAborted e)(
// The other side exited -- silently ignore, or do any needed
cleanup.
} finally {
if (! ok) q.abort();
}

But no, we have to put a fair bit of code in both the finally clause,
and the doSomething() method. Worse, it's different on each side....

As you can see, even using the java.util.concurrent package, there's a
fair amount of subtlety. I don't see these issues handled correctly
very often, I'm afraid.

Unlike Mark, I don't assume Doug Lea is smarter than I am (though he
could be). But experience and the laws of probability show that the
odds of my getting it right ALL the time are extremely low, no matter
HOW smart I am.

The Android designers put a fair amount of thought and effort into
making it possible to avoid threading pitfalls. But I think they need
to revisit the issue, and capture a few more of the patterns -- such
as this one.

There's already a way to do this in Android, BTW, but it requires a
bit of a shift in thinking. Handler's already queue and consume. Take
the inside of your loop in your receiver's doSomething() (let's call
it doOneThing(item) ) -- and post it to a Handler. You'll probably
want to hook the Handler to a separate HandlerThread.

That all works; the main problem I have with it is that it's a bit too
complex to set up for this use, and it requires too much of a rethink,
so people don't do it this way, but ask threading questions instead.
It also may require some rethinking to avoid creating a new Runnable()
object at each step.

But it's there, and using facilities like this are how you should be
thinking.

On Jun 5, 1:19 pm, Ali Chousein <ali.chous...@gmail.com> wrote:
> Hi,
>
> I have two AsyncTasks running; let's say A1 and A2. A2 is manipulating
> the data produced by A1 (in fact is a typical producer-consumer
> scenario). It's very tempting to use wait() inside doInBackground()
> implementation of A2, BUT this causes an IllegalMonitorStateException.
> Is it really not possible to "wait()" inside an AsyncTask? What to do
> when you have an AsyncTask which needs to wait for something?
>
> Thank you in advance,
>
> -Ali

--
You received this message because you are subscribed to the Google
Groups "Android Developers" group.
To post to this group, send email to android-developers@googlegroups.com
To unsubscribe from this group, send email to
android-developers+unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/android-developers?hl=en

No comments:

Post a Comment