Calling wait in AsyncTask (e.g. inside doInBackground) raises IllegalMonitorStateException

Showing 1-10 of 10 messages
Calling wait in AsyncTask (e.g. inside doInBackground) raises IllegalMonitorStateException Ali Chousein 6/5/10 1:19 PM
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
Re: [android-developers] Calling wait in AsyncTask (e.g. inside doInBackground) raises IllegalMonitorStateException Mark Murphy 6/5/10 1:24 PM

Rewrite the AsyncTasks. Either combine them into one, or have A1 execute
A2 when A1 is completed.

--
Mark Murphy (a Commons Guy)
http://commonsware.com | http://github.com/commonsguy
http://commonsware.com/blog | http://twitter.com/commonsguy

_Android Programming Tutorials_ Version 2.0 Available!

Re: Calling wait in AsyncTask (e.g. inside doInBackground) raises IllegalMonitorStateException Ali Chousein 6/5/10 1:43 PM
Thank you for the reply and the suggestion. Having two threads, one
waiting for the other was the best solution for my problem, but
apparently calling wait() in AsyncTask is not supported. Probably I'll
have multiple AsyncTasks, which don't wait for each other, but which
combine subsets of the functionality of the original A1 and A2
AsyncTasks.

Until tonight I used to like the idea of AsyncTask. I don't understand
why the creators of Android cannot make such handy ideas more generic.

@Mr Murphy: BTW, it could be a good idea to extend each chapter of
your Android books with a section of known issues. The books are
great, but they could be better if they also explain the limitations
to an extent.
Re: [android-developers] Re: Calling wait in AsyncTask (e.g. inside doInBackground) raises IllegalMonitorStateException Mark Murphy 6/5/10 2:07 PM
Ali Chousein wrote:
> Thank you for the reply and the suggestion. Having two threads, one
> waiting for the other was the best solution for my problem, but
> apparently calling wait() in AsyncTask is not supported. Probably I'll
> have multiple AsyncTasks, which don't wait for each other, but which
> combine subsets of the functionality of the original A1 and A2
> AsyncTasks.

Or switch out from AsyncTasks to something else. For example, when I
think "producer-consumer", I think LinkedBlockingQueue. Of course,
LinkedBlockingQueue has always been my favorite of the concurrency
classes, so I may tend to overuse it.

> Until tonight I used to like the idea of AsyncTask. I don't understand
> why the creators of Android cannot make such handy ideas more generic.

Particularly when it comes to Java and threading, there is no single
solution that fits every pattern. AsyncTask is designed for discrete,
short-lived, independent operations. Outside of that, grab relevant
classes out of java.util.concurrent, and use Handler or related methods
to invoke logic on the main application thread.

> @Mr Murphy: BTW, it could be a good idea to extend each chapter of
> your Android books with a section of known issues. The books are
> great, but they could be better if they also explain the limitations
> to an extent.

Uh, there aren't enough bytes on my hard drive for that. :-)

More seriously, take your circumstance. As far as I'm concerned, the use
of wait() by ordinary Java developers has been obsolete for about a
decade. Once Doug Lea published his concurrency library -- what you now
think of as java.util.concurrent -- no ordinary Java developer should be
using low-level blocking protocols like wait(), IMHO. There are plenty
of viable patterns already implemented in java.util.concurrent. And Mr.
Lea is way smarter than the rest of us when it comes to Java threading.
Hence, I would not be advising people in my book to avoid wait() in an
AsyncTask, because I would not be advising people to use wait() in the
first place.

I am certainly interested in concrete suggestions for improving the
books -- just head over to the [cw-android] Google Group and chime in. I
am not sure how I can do a "known issues" section in every chapter, but
I will give it some thought. Thanks for the feedback!

--
Mark Murphy (a Commons Guy)
http://commonsware.com | http://github.com/commonsguy
http://commonsware.com/blog | http://twitter.com/commonsguy

_The Busy Coder's Guide to Android Development_ Version 3.1
Available!

Re: Calling wait in AsyncTask (e.g. inside doInBackground) raises IllegalMonitorStateException Al 6/5/10 3:20 PM
What does the exception stack trace say? Are you synchronizing access
to the object before calling wait()? Just wait() without
synchronization will cause an IllegalMonitorStateException.
Re: Calling wait in AsyncTask (e.g. inside doInBackground) raises IllegalMonitorStateException Streets Of Boston 6/5/10 8:22 PM
But a simple

synchronized(object) {
  if(!someConditation)
    object.wait();
}

and

public boolean changeSomeCondition() {
  synchronized(object) {
    someCondition = true;
    object.notifiyAll();
  }
}

can be easy enough to warrant using a wait() statement.

But i agree, as soon as you start writing message consumer/producers,
latches, etc... then using the java.util.concurrent should be stronly
recommended :-)
> Mark Murphy (a Commons Guy)http://commonsware.com|http://github.com/commonsguyhttp://commonsware.com/blog|http://twitter.com/commonsguy
Re: [android-developers] Re: Calling wait in AsyncTask (e.g. inside doInBackground) raises IllegalMonitorStateException Dianne Hackborn 6/6/10 12:14 AM
On Sat, Jun 5, 2010 at 3:20 PM, Al <alca...@googlemail.com> wrote:
What does the exception stack trace say? Are you synchronizing access
to the object before calling wait()? Just wait() without
synchronization will cause an IllegalMonitorStateException.

Indeed, this problem likely has nothing to do with AsyncTask, but improper use of wait.  There is nothing restricting you from using wait() from inside of AsyncTask.  It's just running a thread.

That said, AsyncTask schedules all work on a single thread in the process.  So if you try to schedule two things, and one may wait for the other, you are likely going to cause a deadlock since the task can't complete, to allow the other to run.

--
Dianne Hackborn
Android framework engineer
hac...@android.com

Note: please don't send private questions to me, as I don't have time to provide private support, and so won't reply to such e-mails.  All such questions should be posted on public forums, where I and others can see and answer them.

Re: Calling wait in AsyncTask (e.g. inside doInBackground) raises IllegalMonitorStateException Bob Kerns 6/6/10 9:51 AM
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.
Re: Calling wait in AsyncTask (e.g. inside doInBackground) raises IllegalMonitorStateException Ali Chousein 6/6/10 1:04 PM
Guys, thank you all for your suggestions and comprehensive
explanation. I think LinkedBlockingQueue sounds the most attractive
way to go.
Re: Calling wait in AsyncTask (e.g. inside doInBackground) raises IllegalMonitorStateException Ali Chousein 6/6/10 1:19 PM
By the way, guys, below please find my code. I was trying to use
wait() inside synchronization. I have the same logic working perfectly
fine in the scenario when the main thread cancels an AsyncTask and
waits for it to finish. Whether there will be deadlock or not, depends
on the implementation mistakes I think.

synchronized(mContext)
{
  if (mContactsMessages.isEmpty())
  {
    mConsumerThreadWaitingForMessages = true;
    while(mConsumerThreadWaitingForMessages)
    {
      try
      {
        this.wait();
      }
      catch (InterruptedException e)
      {
        // Do nothing!
      }
    }
  }
}