Is there really a race condition in this multithreaded java code?

Question

I saw a piece of code in this question that I could not understand (most likely due to the fact that I am new to this area). This question refers to "a state of obvious race, where sometimes the producer finishes, signals this, and ConsumerWorkers stops before consuming everything in line."

• In my opinion, isRunning will only be installed on the consumer after the manufacturer decides not to add more items to the queue. Thus, if the consumer thread sees isRunning as FALSE, and then sees that inputQueue is empty, then there is no way to add something to the queue in the future. Obviosuly, I am mistaken and something is missing, since no one who answered this question said that the scenario of the question is impossible. So, can anyone explain what sequence of events causes this race condition?

• In fact, I see a problem with something else. For example, if several consumer threads saw that the producer isRunning, and they say that the queue has ONE element, many threads can enter a blocked take. If the STOPS producer now, while one thread exits take, the other threads are blocked on take forever. Interestingly, no one who answered this question pointed to this problem. So, my understanding of this is also probably wrong ?!

I did not want to add this as a comment to this question, since this is an old question, and my doubts will never get an answer! I will copy / post the code from this question here for a quick reference.

public class ConsumerWorker implements Runnable{ private BlockingQueue<Produced> inputQueue; private volatile boolean isRunning = true; public ConsumerWorker(BlockingQueue<Produced> inputQueue) { this.inputQueue = inputQueue; } @Override public void run() { //worker loop keeps taking en element from the queue as long as the producer is still running or as //long as the queue is not empty: while(isRunning || !inputQueue.isEmpty()) { System.out.println("Consumer "+Thread.currentThread().getName()+" START"); try { Object queueElement = inputQueue.take(); //process queueElement } catch (Exception e) { e.printStackTrace(); } } } //this is used to signal from the main thread that he producer has finished adding stuff to the queue public void setRunning(boolean isRunning) { this.isRunning = isRunning; } 

Answer 1

I think the OP of the original question probably meant

 while(isRunning && !inputQueue.isEmpty()) 

but not

 while(isRunning || !inputQueue.isEmpty()) 

The first clearly describes the problem described by the original poster (*), and the later version has the problem that you described in your second paragraph. Simple oversight is there, but now we can notice that both approaches are wrong.

_{(*) and for some reason suggests that the queue will never be empty.}

Answer 2

You are right in both questions. Yes && correct, but || - no. Regarding the second question, the answers were to use a poison pill or timeout in both directions to solve the problem.

As for me, I would create a new synchronization class that combines both the queue and the isRunning variable, so changing isRunning causes an exception in take() , which signals a shutdown.