Using ` LinkedBlockingQueue` may cause null pointer exception

Question

I am learning java concurrent programming recently. I know that the final keyword can guarantee a safe publication. However, when I read the LinkedBlockingQueue source code, I found that the head and last field did not use the final keyword. I found that the enqueue method is called in the put method, and the enqueue method directly assigns the value to last.next. At this time, last may be a null because last is not declared with final. Is my understanding correct? Although lock can guarantee last read and write thread safety, but can lock guarantee that last is a correct initial value instead of null

public class LinkedBlockingQueue<E> extends AbstractQueue<E>
        implements BlockingQueue<E>, java.io.Serializable {
transient Node<E> head;
private transient Node<E> last;
public LinkedBlockingQueue(int capacity) {
        if (capacity <= 0) throw new IllegalArgumentException();
        this.capacity = capacity;
        last = head = new Node<E>(null);
    }
 private void enqueue(Node<E> node) {
        // assert putLock.isHeldByCurrentThread();
        // assert last.next == null;
        last = last.next = node;
    }

public void put(E e) throws InterruptedException {
        if (e == null) throw new NullPointerException();
        // Note: convention in all put/take/etc is to preset local var
        // holding count negative to indicate failure unless set.
        int c = -1;
        Node<E> node = new Node<E>(e);
        final ReentrantLock putLock = this.putLock;
        final AtomicInteger count = this.count;
        putLock.lockInterruptibly();
        try {
            /*
             * Note that count is used in wait guard even though it is
             * not protected by lock. This works because count can
             * only decrease at this point (all other puts are shut
             * out by lock), and we (or some other waiting put) are
             * signalled if it ever changes from capacity. Similarly
             * for all other uses of count in other wait guards.
             */
            while (count.get() == capacity) {
                notFull.await();
            }
            enqueue(node);
            c = count.getAndIncrement();
            if (c + 1 < capacity)
                notFull.signal();
        } finally {
            putLock.unlock();
        }
        if (c == 0)
            signalNotEmpty();
    }
}

They can't be `final` because they get changed. Make a variable `final` doesn't guarantee it is never null. Your question does't tmake sense. — user207421, May 14 '20 at 03:07

tomtzook · Answer 1 · 2020-05-14T03:14:26.870

You are correct that last is equal to a node with a null value. However this is intentional. The lock is only meant to ensure that each thread can perform modifications in this class correctly.

Sometimes using null values is intentional, to indicate a lack of value (empty queue in this case). Because the variable is private it can only be modified from within the class, so as long as the one writing the class is aware of the possibility of null, everything is alright.

I think you are confusing multiple different concepts which are not necessarily connected. Note that because last is private there is no publication. In addition head and last are meant to be modified, so they can't be final.

Edit

Perhaps I misunderstood your question...

null is never assigned to last directly. So the only place this could happen is in the constructor, before last is assigned new Node<E>(null). Although we can be sure that the constructor finishes before it is used by many threads, there is no visibility guarantee for the values.

However put uses a lock which does guarantees visibility in use. So if there was no lock used, then last could actually be null.

I meant that last may be null (last == null) – caihuadaye May 14 '20 at 02:59 — caihuadaye, May 14 '20 at 02:59

score 0 · Answer 2 · answered May 14 '20 at 02:55

0

Maybe you miss understanding about the of Java's continuous assignment

//first last is inited in the constructor
last = head = new Node<E>(null); // only the filed's value in last is null(item & next)

// enqueue
last = last.next = node;
//equals:
last.next = node;
last = last.next;

Only if you call last.next otherwise there will no NPE.

answered May 14 '20 at 02:55

PatrickChen

1,350
1
11
19

I meant that last may be null (last == null) – caihuadaye May 14 '20 at 03:00
I understand, but last is inited in the constructor method. `last = head = new Node(null);` so last will not be null, only the fields inside is null – PatrickChen May 14 '20 at 03:53

score 0 · Accepted Answer · answered May 14 '20 at 03:02

According to this blog post https://shipilev.net/blog/2014/safe-public-construction/ even writing to one final property in constructor is enough to achieve safe initialization (and thus your object will be always published safely). And capacity property is declared as final.

In short, we emit a trailing barrier in three cases:

A final field was written. Notice we do not care about what field was actually written, we unconditionally emit the barrier before exiting the (initializer) method. That means if you have at least one final field write, the final fields semantics extend to every other field written in constructor.

Using ` LinkedBlockingQueue` may cause null pointer exception

3 Answers3

Edit