0

I have two threads in my Android app which are effectively in a producer/consumer relationship; the producer thread (a subclass of Thread) populates a buffer with objects, and the consumer thread (a subclass of AsyncTask) operates on that buffer. From the Java guarded locks concurrency tutorial here I gather that I could use a 'third-party' thread to coordinate this exchange, but ideally I'd like to be able to shut down the consumer thread itself via a call to wait, since its only real task is to operate on the buffer once it has been populated by the producer. The producer would then wake it via a call to notify or notifyAll when and only when the producer finishes populating the buffer completely. In order to facilitate this, I have the following configuration for my consumer and producer, respectively:

Consumer.java

public class Consumer extends AsyncTask<Object,Integer,Object>{
private String TAG = "Consumer";
private String SUBCLASS_TAG = "";
private String FUNCTION_TAG = "";
private int UUID = MasterSemaphore.getAndIncrementUuidTracker();

public synchronized void getMonitorForNotification(){
FUNCTION_TAG = "::getMonitorForNotification";
while(MasterSemaphore.getIsBufferingMap().get(UUID)){
try {
Log.i(TAG+SUBCLASS_TAG+FUNCTION_TAG, "about to wait...");
  wait();
} catch (InterruptedException e) {
  e.printStackTrace();
  }
}
FUNCTION_TAG = "::getMonitorForNotification";
Log.i(TAG+SUBCLASS_TAG+FUNCTION_TAG, "Received notification!");
}

@Override
protected Object doInBackground(Object... bgTaskResources) {
Producer hProducer = (Producer)bgTaskResources[0];
//The next call is supposed to freeze this thread's execution via an invocation
//of wait-- see the Producer::populateBuffer(...) method
hProducer.populateBuffer(5,this); 
//...handle other AsyncTask callbacks

Producer.java

//..snip 

public synchronized int populateBuffer(int numElements, Consumer externalTaskCaller){
TAG = "Producer"
SUBCLASS_TAG = "";
FUNCTION_TAG = "::populateBuffer";

//First set the bufferingMap over the external caller's UUID to true
MasterSemaphore.getIsBufferingMap().put(externalTaskCaller.getUUID(), true);
Log.i(TAG+SUBCLASS_TAG+FUNCTION_TAG, "just set "+externalTaskCaller.getUUID()+" Key in
the bufferingMap to true");

//Next acquire the monitor of the external caller, and tell it to wait for notification
externalTaskCaller.getMonitorForNotification();
Log.i(TAG+SUBCLASS_TAG+FUNCTION_TAG, "just acquired a monitor lock on 
externalCaller"+externalTaskCaller.toString()+", hopefully");

int elementsProduced = 0;

for (int i=0;i<numElements;i++){
mvElemVector.add(new Element());
Log.i(TAG+SUBCLASS_TAG+FUNCTION_TAG, "just created element number "+i+" of 
    "+numElements);
elementsProduced++;
}

if(externalTaskCaller != null){
MasterSemaphore.getIsBufferingMap().put(externalTaskCaller.getUUID(), false);
Log.i(TAG+SUBCLASS_TAG+FUNCTION_TAG, "just set "+externalTaskCaller.getUUID()+" Key
    in the bufferingMap to false since our buffer writing is done");
externalTaskCaller.notifyAll();
Log.i(TAG+SUBCLASS_TAG+FUNCTION_TAG, "just notified the external caller 
    "+externalTaskCaller.toString());
}

return threadsProduced;

}

//..snip

The results I'm seeing when I run a unit test over this functionality (essentially just creates and starts a Producer thread, then creates and executes a Consumer task) returns only the log entries:

01-02 09:01:53.530: I/Producer::populateBuffer(21932): just set 0 Key in the
bufferingMap to true
01-02 09:01:53.530:I/Consumer::getMonitorForNotification(21932): about to wait...

and that's it... so we're getting to

externalTaskCaller.getMonitorForNotification();

but never reach

Log.i(TAG+SUBCLASS_TAG+FUNCTION_TAG, "just acquired a monitor lock on 
externalCaller"+externalTaskCaller.toString()+", hopefully");

What is wrong with my wait-notify implementation here? Is a 'third-party' object like the Drop object in the linked tutorial necessary to coordinate a producer/consumer exchange?

CCJ
  • 1,619
  • 26
  • 41
  • 2
    I'd consider using a `BlockingQueue`. Your consumer will do a `take()` on the queue and the producer will do a `put()`. No waits or anything. – Gray Mar 19 '13 at 23:02
  • 2
    Code isn't formatted well, it's pretty hard to follow. This doesn't obviously follow wait/notify patterns I've seen. I'd echo @Gray in suggesting you use a canned synchronized queue like j.u.c.BlockingQueue. One thing to note is that you've spread locking behavior amongst two classes which is usually a bad sign. Factor out your synchronized shared queue into a separate class that handles all the locking/sync in one place. (You'll probably discover you have reimplemented a BQ) – andersoj Mar 19 '13 at 23:04
  • 1
    You'd be much better using a `BlockingQueue` as @Gray points out. [Here](http://stackoverflow.com/a/15388291/823393) is an example I posted earlier. – OldCurmudgeon Mar 19 '13 at 23:16
  • BlockingQueue looks reasonable, thanks. I'd still like to understand how the wait-notify idiom should be implemented in order to facilitate the behavior I was looking for above, if possible-- after reading up on the JVM monitor model a bit, it sounds like that exact behavior (a worker thread suspending execution until it receives notification of some state change, then resuming with code dependent on said state change) may not be possible or at least may not be advisable. Should I open a new question regarding the Java monitor concurrency model or is it relevant enough to be answered here? – CCJ Mar 20 '13 at 15:54

0 Answers0