File APIs for Java Developers
Manipulate DOC, XLS, PPT, PDF and many others from your application.
http://aspose.com/file-tools
The moose likes Threads and Synchronization and the fly likes Producer Consumer Problem using Thread Big Moose Saloon
  Search | Java FAQ | Recent Topics | Flagged Topics | Hot Topics | Zero Replies
Register / Login


Win a copy of Soft Skills this week in the Jobs Discussion forum!
JavaRanch » Java Forums » Java » Threads and Synchronization
Bookmark "Producer Consumer Problem using Thread" Watch "Producer Consumer Problem using Thread" New topic
Author

Producer Consumer Problem using Thread

V K Gupta
Ranch Hand

Joined: Aug 07, 2008
Posts: 55
Hi Friends,

Please help me with the Producer Consumer Problem using Thread in Java. I have written the program for Producer Consumer Problem using Thread but my program is incomplete , so I want that somebody make the correction and addition in my program to make it complete so that i can understand Threads.


class A {
int value;

public void setA(int v) {
value=v;
}

public int getA() {
return value;
}
}

class put implements Runnable {

A obj;

public void run() {
for(int i=1;i<=10;i++) {
System.out.println("putting in A value is " + i );
obj.setA(i);
}
}

put(A ob) {
obj=ob;
}
}


class get implements Runnable {
A obj;

get(A ob) {
obj = ob;
}


public void run() {
while(true) {
System.out.println(obj.getA());
if(obj.getA() == 10)
break;
}
}

}

public class X1 {

public static void main(String arg[]) {
A obj = new A();

put p = new put(obj);
get g = new get(obj);

Thread p1 = new Thread(p);
Thread g2 = new Thread(g);

p1.start();
g2.start();
}
}
Chan Ag
Bartender

Joined: Sep 06, 2012
Posts: 1049
    
  15
You could tell us how you expect your program to behave. Do you mean your producer and consumer threads need to be synchronized? If yes, then what kind of synchronization are you looking for? Are you ok if the get thread misses a couple of values set by the other thread, i.e you just want that each of these threads taken separately behaves in a consistent way as regards the shared object-- this is relatively simple. Or do you also want that the updates to the state of the shared object are also not missed - in which case you might need to have some custom wait, notification logic build up.

Also it seems, you aren't new to ranch. You might want to consider using the code tags to make your question readable.

Also unless you don't have a strong reason to not follow Java naming conventions, I believe you might want to consider changing your class names and method names.

Thanks,
Chan.
V K Gupta
Ranch Hand

Joined: Aug 07, 2008
Posts: 55
Thanks Chang Ag,

I have re-written by program according to Java Conventions.

Second Question ?

I need producer and consumer thread to be synchronized . If producer produce one item then it should notify the consumer thread about it and after the consumer consume the item it should notify the producer to produce again and it should go on.

consumer thread should not miss any produce item by the producer thread.



V K Gupta
Ranch Hand

Joined: Aug 07, 2008
Posts: 55
please reply
Chan Ag
Bartender

Joined: Sep 06, 2012
Posts: 1049
    
  15
Ok, so this is sort of a rough draft of a response. Hence you should treat it as such. The point is I'm also studying Java Threads and I'm yet to start with wait() and notify() logic and their inherent intricacies. So while this solution works, it may break while you test it. Hence you might want to test it and make amends to it. I have a feeling that this ain't a good solution ( You see the redundancy, right? ). If you can wait, I may post a better solution later.



Request others to provide feedback, so we may all learn.

Thanks,
Chan.
Steve Luke
Bartender

Joined: Jan 28, 2003
Posts: 4181
    
  21

V K Gupta wrote:please reply

Hi V K Gupta, You posted this all of four minutes after your previous post. The good folks on this forum are volunteers and have their own things to do: they aren't here to read and respond to your posts within seconds of your posting. Even if they were, allowing just four minutes between prods hardly gives time to understand the code you posted an provide a detailed response (after all if it were that easy you probably wouldn't have needed the help, you are clearly bright, so what is hard for you is not going to be easy for anyone else). So for next please EaseUp (<-- link).

Chan, the response you gave is pretty good, but not perfect. You can get a situation where the output is not consistent. For example, if the producer gets the lock on itself first, then the output could be:

But if the consumer gets the lock on the producer first then the output could be:

A small difference, but that could mean an application 'sporadically' works or crashes if the consumer gets a value before the producer produces one.

The second problem with it is that you have two synchronized locks in that code, and they are nested. One is the Item (the item's two methods are synchronized) and the second is the Producer. Worse yet is that the two locks are nested. In the code you have this isn't a problem, you use the locks correctly and always generate the locks in a consistent order (Producer then Item). But having nested locks like this can be dangerous and lead to deadlocks (much easier to happen if the code gets more complicated). My preference is to run on a single lock whenever possible. In your producer/consumer you could easily replace the lock on the Producer with the Item lock. You would pass the Item to the Consumer instead of the Producer.

One final suggestion. Whenever possible I would suggest using the classes in the java.util.concurrent package. For example, in this scenario where you want the Producer to run once, then the consumer to run once, then repeat, using a SynchronousQueue is the ideal solution. You do not have to code your own Item class, and you do not have to code your own signaling. The SynchronousQueue does it for you. And if you reference the SynchronousQueue as a BlockingQueue, you make it easier to modify the Queue's behavior later on (for example, if you wanted to allow the Producer to run faster, you might substitute in an ArrayBlockingQueue to allow multiple items to be stored in order for the Consumer, and you wouldn't have to change any of the working code.)


Steve
Chan Ag
Bartender

Joined: Sep 06, 2012
Posts: 1049
    
  15
Greetings, Steve.

Thanks so much for such an informative and constructive feedback. I greatly appreciate it.

Based on your suggestions, I have reworked on my code to take care of point 1 ( what if consumer gets the lock first ) by having an extra condition in the run method of the Consumer that can issue an extra notify() if that condition is met. I'm not sure if this is a graceful solution to handle the issue-- it's like too many conditions based on hard coded data values which makes your system difficult to be reused ( Just a guess). I'd like to post my solution here so I could (hope to) get a feedback again but I can't steal this post from the OP who reserves the first right to work on it and may have a better solution than mine. So I'll wait for a while. :-)

You almost provided us with the solution for point 2 ( nested locks ). So this part was easy. Thanks.

BlockingQueue description provided by you seems like a very pretty solution. It seems like a nice advanced construct in Java that I'd like to cover as soon as I can. Once I've studied about it, I will come back to this post to implement this requirement ( point 1 might become very easy then ) using a BlockingQueue.

Thanks,
Chan.

Chan Ag
Bartender

Joined: Sep 06, 2012
Posts: 1049
    
  15
Greetings,

It seems I didn't do it right the second time too. :-) The extra notify() and extra wait() was redundant and not 'properly planned' code. Skipping past how I didn't do it right the second time, here is my new solution.



Am I doing it right this time ( let us put aside finer solutions involving BlockingQueue for now cause I will be able to get to those pretty things later I guess-- Much later)?


Thanks,
Chan.





Steve Luke
Bartender

Joined: Jan 28, 2003
Posts: 4181
    
  21

Chan Ag wrote:Am I doing it right this time ( let us put aside finer solutions involving BlockingQueue for now cause I will be able to get to those pretty things later I guess-- Much later)?


You are, as long as...



you know that zero is not a valid value. You make two such assumptions in your code: 1) that 0 means producing has not started, and 2) 9 is the last value. Again, in your specific case, that works. Do you think you could make the system work with any value?
Chan Ag
Bartender

Joined: Sep 06, 2012
Posts: 1049
    
  15
Thanks Steve for the feedback. I've been working on your suggestions. Yeah, even I don't like the assumptions ( the hard coded values) in the code.

I've been trying to remove the dependency on hard coded values to start/stop/manage the producer and consumer logic. Didn't realize it would be so difficult but it turns out that it is/was ( depending on whether it is still close to at least a decent solution ). It has taken me a good amount of time to come up with even the current solution.

I have used a helper class, SharedResource, that encapsulates the shared resource ( i.e Item object ) and a boolean contentChanged. Every time producer changes the value of the item object, it also sets this flag and consumer clears this flag after consuming the value. This change could take care of ensuring that the consumer consumed a value only after producer had produced one regardless of which thread started first. But once producer thread was dead, I could not find a nice logic to stop the consumer loop from waiting. All I could think of was making the consumer wait for a few seconds and if it does not receive a notification in that period and the content hasn't changed in this period, producer is likely dead. Not sure how else I can make consumer thread more intelligent. I also considered adding another flag in the SharedResource class called doneProducing or something that the producer could set and consumer could poll. But that idea didn't appeal to me much. But still, is that what good programmers should do?

Here is what I have.



Here are the two sets of outputs.

Producer started
Producing 1
Producer waiting for consumer
Consumer started
Consuming 1
Consumer waiting for producer
Producing 2
Producer waiting for consumer
Consuming 2
Consumer waiting for producer
Producing 3
Producer waiting for consumer
Consuming 3
Consumer waiting for producer
Producing 4
Producer waiting for consumer
Consuming 4
Consumer waiting for producer
Producing 5
Producer waiting for consumer
Consuming 5
Consumer waiting for producer
Producing 6
Producer waiting for consumer
Consuming 6
Consumer waiting for producer
Producing 7
Producer waiting for consumer
Consuming 7
Consumer waiting for producer
Producing 8
Producer waiting for consumer
Consuming 8
Consumer waiting for producer
Producing 9
Producer waiting for consumer
Consuming 9
Consumer waiting for producer
Producing 10
Consuming 10
Consumer waiting for producer
Producer likely dead. I must die too.


Consumer started
Consumer waiting for producer
Producer started
Producing 1
Producer waiting for consumer
Consuming 1
Consumer waiting for producer
Producing 2
Producer waiting for consumer
Consuming 2
Consumer waiting for producer
Producing 3
Producer waiting for consumer
Consuming 3
Consumer waiting for producer
Producing 4
Producer waiting for consumer
Consuming 4
Consumer waiting for producer
Producing 5
Producer waiting for consumer
Consuming 5
Consumer waiting for producer
Producing 6
Producer waiting for consumer
Consuming 6
Consumer waiting for producer
Producing 7
Producer waiting for consumer
Consuming 7
Consumer waiting for producer
Producing 8
Producer waiting for consumer
Consuming 8
Consumer waiting for producer
Producing 9
Producer waiting for consumer
Consuming 9
Consumer waiting for producer
Producing 10
Consuming 10
Consumer waiting for producer
Producer likely dead. I must die too.

Is it better now? Is< stop after value hasn't changed even though the wait time is elapsed> a decent approach to stop the threads in this scenario? What would you say?

Thanks and Greetings,
Chan.

Steve Luke
Bartender

Joined: Jan 28, 2003
Posts: 4181
    
  21

Chan Ag wrote:But once producer thread was dead, I could not find a nice logic to stop the consumer loop from waiting. All I could think of was making the consumer wait for a few seconds and if it does not receive a notification in that period and the content hasn't changed in this period, producer is likely dead. Not sure how else I can make consumer thread more intelligent. I also considered adding another flag in the SharedResource class called doneProducing or something that the producer could set and consumer could poll. But that idea didn't appeal to me much.


I am not a particular fan of the timeout approach, because it is rather delicate if the amount of time to do the production varies or there can be hickups (if it was always fast there would be little reason to use separate threads...) There are two normal approaches:
1) Set a flag. You already thought of this one, and I think the problem you seem to have with it is it is 'yet another flag' to set. To get around that (sort of) you could use a state instead of flags. For example:

Some state variable in SharedResources starts out as INIT. When the Producer first sets a value it sets the state to PRODUCED as it sets the new value, then waits for the CONSUMED state. When it has produced the last result, instead of setting the PRODUCED state it sets the DONE state. Meanwhile the Consume waits for the PRODUCED state, and gets the value. As it gets the value it sets the state to CONSUMED, then before waiting again it checks for the DONE state, and when it gets it, it stops. This way you get one variable which descriptively replaces both the contentChanged flag you already have and the productionComplete flag.

2) Use a PoisonPill. A PoisonPill is a specific object that the Producer sets that (a) never should come up as a natural value of production and (b) when encountered causes the consumer to die. This is much easier to do using Object references than primitive data. An example:

Now, I haven't tested with Integers, I am not sure if there are gotchas in this particular code (but it would be easy to test, just cycle through -10 to 10 and make sure it doesn't kill the consumer). But this is the strategy I almost always implement my code (usually because I am not producing primitives, but more complex Data Access Objects). It also translates to using a Queue...
Chan Ag
Bartender

Joined: Sep 06, 2012
Posts: 1049
    
  15
public enum DataState { INIT, PRODUCED, CONSUMED, DONE }


Wow. This is brilliant. Four clean, neat enum objects. One reference. And it's sorted. Wow. Very creative. Wonder why I couldn't think of it.
Thanks a lot, Steve.

I think the problem you seem to have with it is it is 'yet another flag' to set


Yes, exactly. I felt all I was doing is - go create another flag. Every time. Come a new situation, all I could think of was have another flag. And the worst thing is this flag had to be a shared flag.

I realize they use PoisonKill a lot many times in my current project and one of the default PoisonKill value they have here is the String "-999". But it's a different programming language ( a proprietary programming language). I didn't know there was a term for 'PoisonKill' values. I think it's a good idea depending on the implementation. For this case I think enum reference approach is better though. I say this because we are also setting Item content to the PoisonKill value and that is not where PoisonKill belongs. Also like you said, it's an int. I like your explanation of why if we'd ever use PoisonKill approach, we should always test using ==. Not sure if they've given due consideration to that in our project, but I'd check.

Just a side note --

Given that today I've started studying about volatile variables, I have also started re-evaluating my solution with respect to everything I'm studying. One of the things I kept on thinking about today was I have encapsulated item and the flag in an object and my run has a synchronized block on that object, not on item. So item can still be inconsistent. main() method still has item reference and hence I should have synchronized those blocks on item object. Also I was thinking content in Item is private and there are only getters and setters. Hence having content variable as a volatile variable was not a necessity. But again my synchronized blocks ( synchronized on another object) are accessing it - one is even doing a ++ operation on the content. So I should have it as volatile. And then I thought- should it be better to have synchronized blocks synchronize on resource.getItem(). Just explaining my state of mind. Not really seeking help with it currently cause I feel as I study further, my understanding will improve. Synchronized methods and blocks come next in the book I'm using as my reference.

And if there are things I'd still need help with, I'm sure you guys will help me like you have so far. All this help means a lot to me. Thanks.

Regards,
Chan.


Chan Ag
Bartender

Joined: Sep 06, 2012
Posts: 1049
    
  15
I tried to find some of the answers myself, but it seems I still require guidance. I've posted my questions in a separate, new post. Here is the link, if you'd like to take a look.
http://www.coderanch.com/t/615123/java/java/synchronized-blocks-methods

I'd appreciate any help I can get on the subject.

Thanks,
Chan.
Chan Ag
Bartender

Joined: Sep 06, 2012
Posts: 1049
    
  15
@Steve

I was considering whether to write this note or not. Although it's an old post, I felt it was still open.

Now that I'm actually working on the BlockingQueues, I am able to appreciate how indispensable the Poison kill approach is to have consumer know when to stop. It keeps my producers and consumers decoupled. Following was a stupid comment and a result of my ignorance.


I realize they use PoisonKill a lot many times in my current project and one of the default PoisonKill value they have here is the String "-999". But it's a different programming language ( a proprietary programming language). I didn't know there was a term for 'PoisonKill' values. I think it's a good idea depending on the implementation. For this case I think enum reference approach is better though. I say this because we are also setting Item content to the PoisonKill value and that is not where PoisonKill belongs.


By the way, here is my latest DelayedQueue implementation.

Pizza class - The raw type of elements.

package BlockingQueues.DelayedQueues;

import java.util.Objects;




DelayedPizza class-


DelayedPizzaProducer class



DelayedPizzaConsumer


And the PizzaScheduler class with the main method.


And here is my output.

run:
Thread-0 produced item 1 at 1375603968800
Thread-0 produced item 2 at 1375603968800
Thread-0 produced item 3 at 1375603968800
Thread-0 produced item 4 at 1375603968800
Thread-0 produced item 5 at 1375603968800
Thread-0 produced item 6 at 1375603968800
Thread-0 produced item 7 at 1375603968800
Thread-0 produced item 8 at 1375603968800
Thread-0 produced item 9 at 1375603968800
Thread-0 produced item 10 at 1375603968800
Thread-0 produced item 11 at 1375603968800
Thread-0 produced item 12 at 1375603968800
Thread-0 produced item 13 at 1375603968800
Thread-0 produced item 14 at 1375603968800
Thread-0 produced item 15 at 1375603968800
Thread-0 produced item 16 at 1375603968800
Thread-0 produced item 17 at 1375603968800
Thread-0 produced item 18 at 1375603968800
Thread-0 produced item 19 at 1375603968800
Thread-0 produced item 20 at 1375603968800
Thread-0 produced Poison Kill item -1 at 1375603968800
Thread-1 consumed Pizza with id 1 at time 1375603970800
Thread-1 consumed Pizza with id 2 at time 1375603970800
Thread-1 consumed Pizza with id 20 at time 1375603970800
Thread-1 consumed Pizza with id 19 at time 1375603970800
Thread-1 consumed Pizza with id 18 at time 1375603970800
Thread-1 consumed Pizza with id 17 at time 1375603970800
Thread-1 consumed Pizza with id 4 at time 1375603970800
Thread-1 consumed Pizza with id 15 at time 1375603970800
Thread-1 consumed Pizza with id 14 at time 1375603970800
Thread-1 consumed Pizza with id 13 at time 1375603970800
Thread-1 consumed Pizza with id 12 at time 1375603970800
Thread-1 consumed Pizza with id 11 at time 1375603970800
Thread-1 consumed Pizza with id 10 at time 1375603970800
Thread-1 consumed Pizza with id 9 at time 1375603970800
Thread-1 consumed Pizza with id 8 at time 1375603970800
Thread-1 consumed Pizza with id 7 at time 1375603970800
Thread-1 consumed Pizza with id 6 at time 1375603970800
Thread-1 consumed Pizza with id 5 at time 1375603970800
Thread-1 consumed Pizza with id 16 at time 1375603970800
Thread-1 consumed Pizza with id 3 at time 1375603970800
BUILD SUCCESSFUL (total time: 20 seconds)

Yipeeee!!!

Thank you..
Chan.





Steve Luke
Bartender

Joined: Jan 28, 2003
Posts: 4181
    
  21

Hi Chan, first, I would like to say that the comment you quoted from yourself wasn't stupid. You said the choice between the enum or poison pill would depend on implementation, and for the implementation you had, the enum seemed the better choice. I think that is a good comment, not a stupid one. Every choice should be made in context to the design, and when you feel the design calls for one option over another, then at least you know the options and make an educated choice. The fact that later, with a different design and leaning towards the opposite decision does not make the other choice based on the old design any worse.

Now, about the current implementation, I wanted to highlight a few things.
In your producer you have this:

With this code, if you make Pizza with an id of 12, but the Queue is full, then the pizza is thrown away, and the next pizza is made. The alternative is to use put() instead of offer(). The put() method will block, so you will be holding up your production line, but it prevents lost pizza. So your choice (and it is a specification choice, so I can't say which is correct) is: If the Queue were to fill up, should the production wait for room to put the pizzas, or should production continue at the same rate, with any new pizza that can't fit being waste. Both options are correct for different scenarios, I just wanted you to be aware of the choice you were making.

You do the same sort of thing on the Consumer side, but there it is a bit more nefarious:

In this scenario, you use poll() to check if there is something available, but poll() returns immediately, so if nothing is available it might return null. You get around this by putting it in a tight loop which checks for null. This could lead to high CPU usage as the consumers constantly check for the next available item. A much better approach would be to use the take() method, which blocks until something is available - generally a much more CPU-efficient approach.



Also, in both the poll() and take() cases, the poison is actually removed from the queue, so if you have multiple consumers, later consumers will not see the poison and could continue to wait indefinitely. So I always put the poison back into the queue:



My last bit is about the poison pill object you use. You give it a special-case ID, and then you reference that special-case ID in two separate locations. This has the feel of 'magic numbers' and in the very least should be defined as a constant somewhere.

I would also worry about using the special case ID as the kill signal itself, because it can run you into trouble. For example, say you kept this producer running for a long time and the ID counter ran out of positive integers. It may 'loop around' and begin counting up from Integer.MIN_VALUE, and could eventually get to -1 as a real ID. But in this case, your application would stop. I know it is remote, but you can get around it using Object identity comparison (==) and a specific, pre-defined Object that means 'die'.

Now we can't be trapped by a number coming up when we don't expect it, and we know the consumer will only end when the Poison is specifically added to the Queue. The only issues I tend to have with this approach is where the POISON best resides (I don't like putting it in either the producer or consumer because that creates an unnecessary dependence. The final thing is the identity comparison might not work in some complex situations, like distributed applications or applications with different class loaders for the producer and consumer, so there a different system would have to be used.
Chan Ag
Bartender

Joined: Sep 06, 2012
Posts: 1049
    
  15
Hi Steve,

First things first. Thanks so much. It means a lot to me to have you ( and others too when they do it ) go through my code and provide such a nice feedback along with help on how to resolve the problems in my code and explaining why another approach is better. I greatly appreciate it.

I will work on it and try to not have any new mistakes in my code. :-)

About putting the Poison Pill back into the queue -- Every time I've worked on a BlockingQueue implementation where there were multiple consumers, I made producers produce multiple Poison Pill value. Such a bad design ( I know )! I had thought about it multiple times but could not come up with the idea of putting the Poison back into the queue. Once I vaguely considered putting the posion back into the queue but I was too quick to discard the idea ( without any reason ) even before it could register. Now that you've showed it to me, .....

I'm going to work on it with all those wonderful suggestions.

Thanks and greetings,
Chan.


 
I agree. Here's the link: http://aspose.com/file-tools
 
subject: Producer Consumer Problem using Thread