wood burning stoves 2.0*
The moose likes Threads and Synchronization and the fly likes Producer-Consumer Big Moose Saloon
  Search | Java FAQ | Recent Topics | Flagged Topics | Hot Topics | Zero Replies
Register / Login


Win a copy of Murach's Java Servlets and JSP this week in the Servlets forum!
JavaRanch » Java Forums » Java » Threads and Synchronization
Bookmark "Producer-Consumer" Watch "Producer-Consumer" New topic
Author

Producer-Consumer

Pho Tek
Ranch Hand

Joined: Nov 05, 2000
Posts: 761

I've written a Producer-Consumer sample code which uses a BoundedBuffer.
I'm trying to reinforce my understanding of the wait/notify protocol and would
appreciate corrections/improvements.

I've based the code on the algorithm (based on monitors) explained on wikipedia.

Thanks.



Regards,

Pho
Nitesh Kant
Bartender

Joined: Feb 25, 2007
Posts: 1638

There are a few things i have to say as follows:
  • I would suggest that you have different monitors for add and remove. This gets more important since you are using notify() and notifyAll(). Even if you have the waits inside the while loop checking the bound condition but still waking up a wrong thread wastes CPU cycles. In the current shape, if i start multiple consumers/producers, the code will not perform well. The reason being a consumer may end up notifying another consumer which may not have anything to consume. Also, a producer may wake up another producer and realise that the queue is already full. (Ofcourse, this may occur in spurious wakeups but that can not be avoided)
  • Add/Remove should not eat the interrupted exception. This would mean that there is no way of interrupting the producer/consumer in case the queue is full/empty but for altering the queue.
  • I would suggest to use notifyAll instead of notify or make the arraylist private. (This is in case you are not using different monitors for add/remove)
  • I dont think you need to iterate to get the first item out of the arraylist. You can just use get(0).
  • Not sure why are you using delays in Producer/consumer code.


  • apigee, a better way to API!
    Pho Tek
    Ranch Hand

    Joined: Nov 05, 2000
    Posts: 761

    Nitesh,

    I will modify it to have two monitors, as you suggested. In fact the algorithm (on wikipedia) clearly had two different conditions: full and empty.

    Add/Remove should not eat the interrupted exception

    I'm concluding that a method that throws the InterruptedException means that it is cancellable.

    I would suggest to use notifyAll instead of notify.

    I've been searching for an explanation as to why #notifyAll is preferred over #notify. jchq offers this explanation:

    The notify method will wake up one thread waiting to reacquire the monitor for the object. You cannot be certain which thread gets woken. If you have only one waiting thread then you do not have a problem. If you have multiple waiting threads then it will probably the thread that has been waiting the longest that will wake up. However you cannot be certain, and the priorities of the threads will influence the result. As a result you are generally advised to use notifyAll instead of notify, and not to make assumptions about scheduling or priorities. Of course this is not always possible and you may have to try to test your code on as many platforms as possible.


    Not sure why are you using delays in Producer/consumer code.

    The delays are just for testing.

    Thanks

    Pho
    Pho Tek
    Ranch Hand

    Joined: Nov 05, 2000
    Posts: 761

    Here's my modified code. SharedBuffer is renamed to MonitorSharedBuffer.

    Notable changes:
  • the arraylist is now made threadsafe via Collections.synchronizedList( List<T> )
  • Introduced distinct monitors for add & remove.


  • Nitesh Kant
    Bartender

    Joined: Feb 25, 2007
    Posts: 1638

    Originally posted by Pho Tek:
    I've been searching for an explanation as to why #notifyAll is preferred over #notify.

    If a homogenous set of threads are waiting on a monitor then it does not make sense to call notifyAll(). It will have the same semantics as waking up a single thread.
    If a heterogenous set of threads are waiting on a monitor then you should always call notifyAll() so there is a fair chance for the correct thread to wake up.
    Henry Wong
    author
    Sheriff

    Joined: Sep 28, 2004
    Posts: 18496
        
      40

    If a heterogenous set of threads are waiting on a monitor then you should always call notifyAll() so there is a fair chance for the correct thread to wake up.


    This is an "issue" with Java synchronization that has been fixed -- with version 5. For some reason, the original design of Java assumed only one wait condition per lock. This is clearly wrong, as no other threading library makes this assumption.

    With Java 5, you need to replace the synchronization with the ReentrantLock class, then get a differnt condition object for each wait condition, that uses the lock. Finally, you can wake up one thread on the correct condition object, instead of waking up all the threads on the single object.

    Henry


    Books: Java Threads, 3rd Edition, Jini in a Nutshell, and Java Gems (contributor)
    Nitesh Kant
    Bartender

    Joined: Feb 25, 2007
    Posts: 1638

    Hi Pho,

    The modified code you have put is deadlock prone. You are having nested locks and that two in revered order in the add and remove methods. This will very easily end up in a deadlock.
    You do not need to lock on the list during add as you are already using a Synchronized list. Instead i would say use a Vector as in your case it will have the same effect.

    Following is a little modification of your code to remove deadlock prone code and use vector instead on sync list.:



    However, if you are working with jdk 5 you must use a ReentrantReadWriteLock . Infact in that case you can use a BlockingQueue implementation . People have already done the hardwork for you!!!
     
    I agree. Here's the link: http://aspose.com/file-tools
     
    subject: Producer-Consumer
     
    Similar Threads
    Two questions uppon a deitel's project
    Threads, that processed two shared ArrayLists like Queue
    Producer Consumer Problem
    wait() and sleep()
    CopyOnWriteArrayList