2
int capacity = ...
BlockingQueue q = new LinkedBlockingQueue<Element>(capacity);

Now, I do feel mildly ridiculous asking this but I'm not particularly savvy when it comes to java concurrency, so I would appreciate some help with choosing the correct way to enqueue something (and dequeue, actually, but I expect when we cleared up the one, the other will fall into place by itself).

There is, of course

while(!q.offer(e));

But I'm a bit wary of spinning implementations in a multi-threaded environment.

And I can't do

synchronized(q){
    while(!q.offer(e))q.wait();
}

either because the wakeup calls will go to internal (private) instances of Condition, meaning this would be a suicide-by-sleeping-pills implementation.

However, I'm also not particularly fond of

try{
    q.put(e);
}catch(InterruptedException ex){}

(even though it does seem to be a popular choice in online examples) because although this would do the waiting for me, I know of no reliable way to detect when an exception would force me to try again. I could do something like

boolean success = false;
do{
    try{
        q.put(e);
        success = true;
    }catch(InterruptedException ex){}
}while(!success)

But then I'd end up enqueuing the same element multiple times if the exception takes place in-between the put and the assignment to success.

I could do

boolean success = true;
do{
    try{
        q.put(e);
    }catch(InterruptedException ex){
        success = false;
    }
}while(!success)

But I remember having read (way back) that you shouldn't rely on exception handling for conditionals (though I can't seem to remember the reason why this is discouraged).

So ... what options do I have? Do I need to spin or is there something more intelligent?

User1291
  • 7,664
  • 8
  • 51
  • 108
  • 1
    What do you think Blocking Queue by name means? Why would you need synchronized over it? – SMA Oct 02 '16 at 15:33
  • @SMA because the point in that snippet is to get the thread waiting and ``offer`` does not? – User1291 Oct 02 '16 at 15:40

3 Answers3

1

The put() implementation would be the correct one, blocking until interrupted or successful. The offer() is a bad idea if all you are doing is spinning (see first comment for disclaimer).

As Nicolas explained, the handling of the InterruptedException is not straightforward and depends a lot on what your other code is doing, but your concern with "if the exception takes place in-between the put and the assignment to success", that can never happen: a blocking call (like put()) can throw that exception, but it cannot occur between put() and the assignment, or at the assignment.

Lastly, there's no need to synchronize on anything. The main idea in many of java.util.concurrent classes is to avoid or abstract away the explicit synchronization.

Kayaman
  • 72,141
  • 5
  • 83
  • 121
  • 1
    Spinning on `offer` is not a bad idea in all cases. If you're after a high-throughput system, then it may be bitter to spin briefly than give up control of the processor by blocking. But that's a minor point that's applicable in only a tiny handful of use cases. – Joe C Oct 02 '16 at 16:20
  • @JoeC Indeed, a rarer use case. – Kayaman Oct 02 '16 at 16:47
  • Spurious interrupt? Aren't you confusing this with spurious wakeups? – bowmore Oct 03 '16 at 04:52
  • @bowmore Good catch. Shouldn't do SO while tired. – Kayaman Oct 03 '16 at 06:02
1

It is not a good practice to catch an InterruptedException as you do since your code won't be responsive to interruption anymore. An InterruptedException is usually thrown by methods that are responsive to interruptions (current Thread being interrupted) such as the methods of type await, wait, join, sleep and many others, this should not be considered as a failure but rather as it really is, a Thread's status change that needs be taken into consideration.

As Brian Goetz explains in Java Concurrency in Practice, I quote:

When your code calls a method that throws InterruptedException, then your method is a blocking method too, and must have a plan for responding to interruption. For library code, there are basically two choices:

Propagate the InterruptedException. This is often the most sensible policy if you can get away with it just propagate the InterruptedException to your caller. This could involve not catching InterruptedException, or catching it and throwing it again after performing some brief activity-specific cleanup.

Restore the interrupt. Sometimes you cannot throw InterruptedException, for instance when your code is part of a Runnable. In these situations, you must catch InterruptedException and restore the interrupted status by calling interrupt on the current thread, so that code higher up the call stack can see that an interrupt was issued.

So in your case, you should simply use put(E) as it will make the calling thread waits for space to become available if needed and propagate the InterruptedException in order to keep on being responsive to interruptions.

But then I'd end up enqueuing the same element multiple times if the exception takes place in-between the put and the assignment to success.

This can simply never happen since an assignment of a boolean will never throw any exceptions (except a NPE in case of an un-boxing). And only methods responsive to interruption can throw such kind of exceptions as explained above which is clearly not the case of an assignment.

Nicolas Filotto
  • 43,537
  • 11
  • 94
  • 122
0

So a few points from the LinkedBlockingQueue Javadoc:

  • The put method will only throw exceptions in two circumstances:
    • The thread is interrupted, in which case you should stop whatever you're doing anyway. See this question for more about InterruptedExceptions.
    • The element you're inserting is null, which is another bug entirely.

So overall, you can just use put to wait for space to become available. If either of these particular exceptions is thrown, then you shouldn't retry anyway.

Community
  • 1
  • 1
Joe C
  • 15,324
  • 8
  • 38
  • 50