CODEWITHSUNDEEP
javamultithreadingarraylistcollections
thedarkpassenger
thedarkpassenger

Reputation: 7358

Does Collections.unmodifiableList(list) require a lock?

I have a productList maintained in a file named Products.java

private List<String> productList = Collections.synchronizedList(new ArrayList());

Now creating a synchronized list, will ensure that operations like add/remove will have a implicit lock and I don't need to lock these operations explicitily.

I have a function exposed which returns an unmodifiableList of this list.

public List getProductList(){

 return Collections.unmodifiableList(productList);
}

In my application, various threads can call this function at the same time. So do I need to put a synchronized block when converting a List into an unmodifiable List or will this be already taken care of since I am using a sychronizedList ?

TIA.

Upvotes: 12

Views: 771

Answers (4)

ZhongYu
ZhongYu

Reputation: 19702

The easiest solution is to get a snapshot everytime, 

list = get snapshot
{
    work on the list
}
discard list // GC it

since the snapshot is a constant, frozen data structure, client can access it freely.

But if the client accesses a data structure that may be modified by another party, it gets problematic. Forget about concurrency issues; think about the semantics of the following code

{
    n = list.size();
    list.get(n-1);
}

get(n-1) may fail, because the list may have shrunk when it's called.

To have some kind of consistency guarantee, the client side must provide explicit transaction demarcations during the session of access, like

acquire lock // preferably a read-lock
{
    work on the list
}
release lock

Note that this code is not simpler than the snapshot solution. And the client can still "miss out" updates, just like the snapshot solution.

And you have to decide whether you want to force client codes to perform this kind of locking.

It's not without merits of course; it can be better in performance than the snapshot solution if the list is big and updates are infrequent.

If this approach is more appropriate for the application, we may design something like

interface CloseableList extends List, Closeable {}

public CloseableList readProducts() // acquire read-lock when called

-- client code
try{CloseableList list = readProducts())
{
    ....
}   
// on close(), release the read-lock

If client only needs to iterate the products, we can use java8 Stream

final ReadWriteLock lock = ...

private ArrayList productList = new ArrayList();
// modifications to `productList` must be protected by write lock

// javadoc: reader must close the stream!
public Stream readProducts()
{
    lock.readLock().lock();
    return productList.stream().onClose( lock.readLock()::unlock );
}

-- client code
try(Stream products = readProducts())
{
    ...
}

We may also design the API to take in the client code so that we can surround it with protections

public void readProducts(Consumer<List> action)
{
    lock read

    action.accept(productList);

    finally unlock read
}

-- client code
readProducts(list->
{
    ...
});

Upvotes: 0

Ferrybig
Ferrybig

Reputation: 18834

The only place where the synchronized should be used is when you loop over it, as explained by the javadoc:

It is imperative that the user manually synchronize on the returned list when iterating over it:

However, you cannot do that once you wrapped it in a unmodifiableList, making it unsafe for a return result. It may return corrupted data in the case of concurrent access.

Instead of returning your backend list, it may be better to return a copy of the backend, so the calling doesn't need to worry about synchronized performance.

public List getProductList(){
    synchronized (productList) {
       return new ArrayList<>(productList);
    }
}

Upvotes: 4

shmosel
shmosel

Reputation: 50776

It doesn't need to be synchronized since the unmodifiable list is wrapping the synchronized one. But there's not much use for synchronizing on an unmodifiable list, other than for the purpose of iteration, which requires manual synchronization regardless:

It is imperative that the user manually synchronize on the returned list when iterating over it:

List list = Collections.synchronizedList(new ArrayList());
    ...
synchronized (list) {
    Iterator i = list.iterator(); // Must be in synchronized block
    while (i.hasNext())
        foo(i.next());
}

Failure to follow this advice may result in non-deterministic behavior.

EDIT: As Ferrybig points out, it's actually not possible to synchronize safely with the unmodifiable wrapper. You may want to consider an alternative thread-safety solution, such as CopyOnWriteArrayList.

Upvotes: 5

A Ahmad
A Ahmad

Reputation: 31

No Need to put the synchronized block.

Upvotes: 0

Related Questions