Return to Answer

Calling wait() automatically releases the lock of the object you are waiting on, but does not release locks on other objects. Therefore this queue will deadlock quite easily.

For readability, I'd recommend using java.util.concurrent.locks.Condition instead of wait() and notify() which are fairly low level and difficult to reason about. In fact, the example usages in the Javadoc for Condition come from the implementation of a bounded buffer.

I also have to echo Brian's concern: it's important that you don't silently swallow InterruptedException. You have two choices on how to handle interruption. If you want to handle the exception yourself, then JCIP says you need to set the interrupted status back.

catch (InterruptedException e)
{
  Thread.interrupt();
  // Handle the interruption
}

The other choice, which is better in my opinion, is to just propagate the exception. Java's built in libraries use this strategy, for an example see BlockingQueue.take().

Have you tried running this implementation? It fails immediately with a java.lang.IllegalMonitorStateException which according to the Javadoc is:

Thrown to indicate that a thread has attempted to wait on an object's monitor or to notify other threads waiting on an object's monitor without owning the specified monitor.

The problem is that the implementation synchronizes on the field lock but calls notify() and wait() on full and free without holding their locks. When you fix this, it's important to keep in mind that calling wait() automatically releases the lock of the object you are waiting on, but does not release locks on other objects. If you don't take that into account, it's quite easy to create deadlocks.

For readability, I'd recommend using java.util.concurrent.locks.Condition instead of wait() and notify() which are fairly low level and difficult to reason about. In fact, the example usages in the Javadoc for Condition come from the implementation of a bounded buffer.

I also have to echo Brian's concern: it's important that you don't silently swallow InterruptedException. You have two choices on how to handle interruption. If you want to handle the exception yourself, then JCIP says you need to set the interrupted status back.

catch (InterruptedException e)
{
  Thread.interrupt();
  // Handle the interruption
}

The other choice, which is better in my opinion, is to just propagate the exception. Java's built in libraries use this strategy, for an example see BlockingQueue.take().

Calling wait() automatically releases the lock of the object you are waiting on, but does not release locks on other objects. Therefore this queue will deadlock quite easily.

For readability, I'd recommend using java.util.concurrent.locks.Condition instead of wait() and notify() which are fairly low level and difficult to reason about. In fact, the example usages in the Javadoc for Condition come from the implementation of a bounded buffer.

I also have to echo Brian's concern: it's important that you don't silently swallow InterruptedException. You have two choices on how to handle interruption. If you want to handle the exception yourself, then JCIP says you need to set the interrupted status back yourself.

catch (InterruptedException e)
{
  Thread.interrupt();
  // Handle the interruption
}

The other choice, which is better in my opinion, is to just propagate the exception. Java's built in libraries use this strategy, for an example see BlockingQueue.take().

Calling wait() automatically releases the lock of the object you are waiting on, but does not release locks on other objects. Therefore this queue will deadlock quite easily.

For readability, I'd recommend using java.util.concurrent.locks.Condition instead of wait() and notify() which are fairly low level and difficult to reason about. In fact, the example usages in the Javadoc for Condition come from the implementation of a bounded buffer.

I also have to echo Brian's concern: it's important that you don't silently swallow InterruptedException. You have two choices on how to handle interruption. If you want to handle the exception yourself, then JCIP says you need to set the interrupted status back.

catch (InterruptedException e)
{
  Thread.interrupt();
  // Handle the interruption
}

The other choice, which is better in my opinion, is to just propagate the exception. Java's built in libraries use this strategy, for an example see BlockingQueue.take().

I don't see any bugs or concurrency problems.

For readability, I'd recommend using java.util.concurrent.locks.Condition instead of wait() and notify() which are fairly low level and difficult to reason about. In fact, the example usages in the Javadoc for Condition come from the implementation of a bounded buffer.

I also have to echo Brian's concern: it's important that you don't silently swallow InterruptedException. You have two choices on how to handle interruption. If you want to handle the exception yourself, then JCIP says you need to set the interrupted status back yourself.

catch (InterruptedException e)
{
  Thread.interrupt();
  // Handle the interruption
}

The other choice, which is better in my opinion, is to just propagate the exception. Java's built in libraries use this strategy, for an example see BlockingQueue.take().

Calling wait() automatically releases the lock of the object you are waiting on, but does not release locks on other objects. Therefore this queue will deadlock quite easily.

For readability, I'd recommend using java.util.concurrent.locks.Condition instead of wait() and notify() which are fairly low level and difficult to reason about. In fact, the example usages in the Javadoc for Condition come from the implementation of a bounded buffer.

I also have to echo Brian's concern: it's important that you don't silently swallow InterruptedException. You have two choices on how to handle interruption. If you want to handle the exception yourself, then JCIP says you need to set the interrupted status back yourself.

catch (InterruptedException e)
{
  Thread.interrupt();
  // Handle the interruption
}

The other choice, which is better in my opinion, is to just propagate the exception. Java's built in libraries use this strategy, for an example see BlockingQueue.take().