paranoidAndroid
paranoidAndroid

Reputation: 583

Volatile and ArrayBlockingQueue and perhaps other concurrent objects

I understand (or at least I think I do;) ) the principle behind volatile keyword. When looking into ConcurrentHashMap source, you can see that all nodes and values are declared volatile, which makes sense because the value can be written/read from more than one thread:

static class Node<K,V> implements Map.Entry<K,V> {
    final int hash;
    final K key;
    volatile V val;
    volatile Node<K,V> next;
    ...
}

However, looking into ArrayBlockingQueue source, it's a plain array that is being updated/read from multiple threads:

private void enqueue(E x) {
    // assert lock.getHoldCount() == 1;
    // assert items[putIndex] == null;
    final Object[] items = this.items;
    items[putIndex] = x;
    if (++putIndex == items.length)
        putIndex = 0;
    count++;
    notEmpty.signal();
}

How is it guaranteed that the value inserted into items[putIndex] will be visible from another thread, providing that the element inside the array is not volatile (i know that declaring the array itself doesnt have any effect anyhow on the elements themselves) ? Couldn't another thread hold a cached copy of the array?

Thanks

Upvotes: 3

Views: 564

Answers (2)

flobjective
flobjective

Reputation: 66

According to my understanding volatile is not needed as all BlockingQueue implementations already have a locking mechanism unlike the ConcurrentHashMap. If you look at he public methods of the Queue you will find a ReentrantLock that guards for concurrent access.

Upvotes: 1

OldCurmudgeon
OldCurmudgeon

Reputation: 65859

Notice that enqueue is private. Look for all calls to it (offer(E), offer(E, long, TimeUnit), put(E)). Notice that every one of those looks like:

public void put(E e) throws InterruptedException {
    checkNotNull(e);
    final ReentrantLock lock = this.lock;
    lock.lockInterruptibly();
    try {
        // Do stuff.
        enqueue(e);
    } finally {
        lock.unlock();
    }
}

So you can conclude that every call to enqueue is protected by a lock.lock() ... lock.unlock() so you don't need volatile because lock.lock/unlock are also a memory barrier.

Upvotes: 5

Related Questions