Concurrency In Practice Circular Buffer Error?

Question

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;
     }
}

Answer 1

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.