mathfish
mathfish

Reputation: 194

Concurrency In Practice Circular Buffer Error?

Finally reading through the excellent Concurrency In Practice book and I am came across listing 14.2 for BaseBoundedBuffer. As is, the put and take methods will allow for count to exceed the buffer capacity or go below 0. I get that the class is abstract but it seems strange this is the default behaviour. Is there some good reason why there would not be some logic to not allow count to go beyond capacity or below 0? Maybe something like,

if(count != buf.length)
     ++count;
@ThreadSafe
public abstract class BaseBoundedBuffer<V> {
     @GuardedBy("this") private final V[] buf;
     @GuardedBy("this") private final int tail;
     @GuardedBy("this") private final int head;
     @GuardedBy("this") private final int count;

     protected BaseBoundedBuffer (int capacity) {
          this.buf = (V[]) new Object[capacity];
     }

     protected synchronized final void doPut(V v) {
          buf[tail] = v;
          if (++tail == buf.length)
               tail = 0;
          ++count;
     }

      protected synchronized final V doTake() {
          V v = buf[head];
          buf[head] = null;
          if (++head == buf.length)
               head = 0;
          --count;
          return v;
     }

     public synchronized final boolean isFull() {
          return count == buf.length;
     }

     public synchronized final boolean isEmpty() {
          return count == 0;
     }
}

Upvotes: 3

Views: 108

Answers (2)

Forketyfork
Forketyfork

Reputation: 7810

We should never let count run out of bounds, but this example supposes that checking this condition is propagated to the caller. We can't just throw an exception, because in a multithreaded program such behavior may be expected and handled in a non-exceptional way (e.g. just waiting for the condition to fulfill). We can't just say if(count != buf.length) ++count; either, because this would be a part of handling logic and could clash with the logic implemented in the caller or subclass.

This example is a part of a bigger picture - the chapter 14.1.1. Example: propagating precondition failure to callers describes an approach where the exceptional case is handled by the subclass. The chapter describes two "painful" ways to implement such functionality (throwing an exception or sleeping the thread) and then provides a more robust approach - using condition queues (see chapter 14.1.3).

I'd like to stress that the code example you've mentioned is not an implementation to copy-and-paste, it's just means of getting to the point.

Upvotes: 0

Michael
Michael

Reputation: 44200

It seems given the example child class in the book that it was intended for the child class to have the responsibility of checking isFull before putting and isEmpty before taking. With such an implementation, checking again is a waste of time.

@ThreadSafe
public class GrumpyBoundedBuffer<V> extends BaseBoundedBuffer<V> {
    public GrumpyBoundedBuffer(int size) { super(size); }

    public synchronized void put(V v) throws BufferFullException {
        if (isFull())
            throw new BufferFullException();
        doPut(v);
    }

    public synchronized V take() throws BufferEmptyException {
        if (isEmpty())
            throw new BufferEmptyException();
        return doTake();
    }
}

In the real world, an appropriate JavaDoc explaining how these methods are intended to be used would be crucial to avoiding the two potential bugs you have identified.

It should go without saying that just because something is in a book doesn't mean it is correct, optimal, or even good. You were right to be skeptical about the implementation.

Upvotes: 1

Related Questions