Use not thread-safe list for LinkedBlockingQueue

Question

I am wondering why wouldn't LinkedBlockingQueue work if we change the underlying data structure to a unthread-safe list like java.util.LinkedList? I get a NoSuchElementException when I tried it. Doesn't it being guarded by a lock (takeLock) which makes it thread-safe ?

private final List<E> list;
private final ReentrantLock takeLock;
private final ReentrantLock putLock;
private final Condition     notFull;
private final Condition     notEmpty;
private final AtomicInteger count;

public LinkedBlockingQueue() {
    takeLock = new ReentrantLock();
    putLock = new ReentrantLock();
    notFull = putLock.newCondition();
    notEmpty = takeLock.newCondition();
    count = new AtomicInteger(0);
    list = new LinkedList<E>();
}

public E get() {
   E item = null;
   int c = -1;
   try {
      takeLock.lockInterruptibly();
      while (count.get() == 0) notEmpty.await();

      // original -> item = dequeue();
      item = list.remove();   // NoSuchElementException

      c = count.getAndDecrement();
   } 
   catch (InterruptedException ie) {} 
   finally {
      takeLock.unlock();
   }

   if (c == capacity) signalNotFull();
   return item;
}

public void put(E e) {
   int c = -1;
   try {
      putLock.lockInterruptibly();
      while (count.get() == BOUND_SIZE) notFull.await();

      // original -> enqueue(node);
      list.add(e);

      c = count.getAndIncrement();
   } 
   catch (InterruptedException ie) {} 
   finally {
      putLock.unlock();
   }

   if (c == 0) signalNotEmpty();
}

What's the difference between `count` and `size`, and `putLock` and `takeLock`? — user253751, Feb 22 '15 at 07:32
Your code looks quite thread-unsafe. (Locks don't magically make code thread-safe) — user253751, Feb 22 '15 at 07:56
why is that ? the only change from the original implementation (http://www.docjar.com/html/api/java/util/concurrent/LinkedBlockingQueue.java.html) is just switching to java.util.LinkedList — peter, Feb 22 '15 at 08:00
LinkedLists aren't thread-safe. (that *doesn't* mean you can't use them from several threads; it does mean you can't use them from several threads at the same time). If you try to use one from several threads at the same time, there are no guarantees whatsoever (in particular, things like "if you add an element, the other thread can get the element" aren't guaranteed) — user253751, Feb 22 '15 at 08:06
I understand LinkedList is not thread safe as I mentioned in the question. When you look at the implementation http://www.docjar.com/html/api/java/util/concurrent/LinkedBlockingQueue.java.html that it uses and maintains a linked list (but not from java.util.LinkedList). It defines this new inner class Node for linking all the items. What's the differences ? and what makes it thread safe ? — peter, Feb 22 '15 at 16:45
ConcurrentBlockingQueue is very carefully designed for thread-safety; LinkedList is not. (For one thing, notice how `enqueue` and `dequeue` don't use the same references, except for `next` pointers... I'm not sure why manipulating the `next` pointers concurrently is okay) — user253751, Feb 22 '15 at 18:39
Btw, if you got an exception on your very first call to `get`, then you possibly have a bug somewhere. Thread safety problems like to be unpredictable and only happen *sometimes*. — user253751, Feb 22 '15 at 18:42

score 1 · Answer 1 · answered Feb 22 '15 at 11:18

1

You are using two separate lock objects:

takeLock = new ReentrantLock();
putLock = new ReentrantLock();
notFull = putLock.newCondition();
notEmpty = takeLock.newCondition();

This is wrong. First of all, you must use same lock object for both put and take operations. Additionally, you must create your conditions using the same lock object.

lock = new ReentrantLock();
notFull = lock.newCondition();
notEmpty = lock.newCondition();

And you should replace your takeLock and putLock usages with given lock reference.

http://docs.oracle.com/javase/7/docs/api/java/util/concurrent/locks/Lock.html http://docs.oracle.com/javase/7/docs/api/java/util/concurrent/locks/Condition.html

answered Feb 22 '15 at 11:18

Basri Kahveci

391
3
7

That will work but then you can't call put and take at the same time. which will behave just like putting synchronized on both methods. the current implementation http://www.docjar.com/html/api/java/util/concurrent/LinkedBlockingQueue.java.html is using 2 locks – peter Feb 22 '15 at 16:40
1

The implementation you mention is working with 2 references, "head" and "last". putLock guards last reference, and takeLock guards "head" reference. Therefore, two locks guard two different references. putLock guarantees the visibility and atomicity of operations on "last" reference. Similarly, takeLock goes for "head" reference. Visibility guarantees between readers and writers are already documented in the code. For your example, you make a change to an object in a lock, and you expect it to be visible to other thread under a different lock. It does not work that way. – Basri Kahveci Feb 22 '15 at 16:55
that totally makes sense. This is the answer that i am looking for. Thanks – peter Feb 22 '15 at 16:58

Use not thread-safe list for LinkedBlockingQueue

1 Answers1