9

Reading this DZone article about Java concurrency I was wondering if the following code:


    private volatile List list;
    private final Lock lock = new ReentrantLock();

    public void update(List newList) {
        ImmutableList l = new ImmutableList().addAll(newList);
        lock.lock();
        list = l;
        lock.unlock();
    }

    public List get() {
        return list;
    }

is equivalent to:


    private volatile List list;

    public void update(List newList) {
        ImmutableList l = new ImmutableList().addAll(newList); 
        list = l;
    }

    public List get() {
        return list;
    }

The try { } finally { } block was omitted for brevity. I assume the ImmutableList class to be a truly immutable data structure that holds its own data, such as the one provided in the google-collections library. Since the list variable is volatile and basically what's going on is a copy-on-the-fly, isn't it safe to just skip on using locks?

Improve this question
1
  • I think both should behave exactly the same, but it depends on the implementation of the ImmutableList. I would expect that calling addAll() on an ImmutableList would throw an Exception, so the lock will never be used. Commented Feb 3, 2010 at 21:43

6 Answers 6

Reset to default
6

In this very specific example, I think you would be OK with no locking on the variable reassignment.

In general, I think you are better off using an AtomicReference instead of a volatile variable as the memory consistency effects are the same and the intent is much clearer.

Improve this answer
Sign up to request clarification or add additional context in comments.

Comments

4

Yes, both of those code samples behave the same way in a concurrent environment. Volatile fields are never cached thread-locally, so after one thread calls update(), which replaces the list with a new list, then get() on all other threads will return the new list.

But if you have code which uses it like this:

list = get()
list = list.add(something) // returns a new immutable list with the new content
update(list)

then it won't work as expected on either of those code examples (if two threads do that in parallel, then the changes made by one of them may be overwritten by the other). But if only one thread is updating the list, or the new value does not depend on the old value, then no problem.

Improve this answer

3 Comments

get() returns an ImmutableList, as I stated in the description of the problem, trying to modify it will only raise an exception.
It's always possible to create a new immutable list which has the elements of the old immutable list plus some more - see in my comment how the add() method returns a new list. That's how it's done in functional programming. (If your example uses google-collections' ImmutableList, then it might not have such an operation, but every functional programming language's libraries do have.)
The original article does [now] update the list. This is part of the reason why synchronisation on the likes of collections rarely does what you want. / The example in the article was simple enough to be easily replaced by a cas loop.
1

After re-reading this yes they are equivalent.

Improve this answer

5 Comments

What is the problem with another thread making a copy of newList before locking the reference to list?
@Clint following on Kevin's question, since l is local to the method and the update to list is protected, wouldn't a thread calling get() still get a consistent result?
@Teto yes. The member variable list isn't being modified, only the reference is being updated.
@Clint update() simply updates the list reference to newList. It doesn't add the contents of newList to the current list
@Clint your logic is right but it doesn't represent the case in question, as list's contents are discarded and replaced completely with every update(). The new values don't depend at all on the old ones.
1

If we are talking about timing and memory visibility. A volatile read is very close to the time it takes to do a normal read. So if you are doing get() alot then there is little difference. The time it takes to do a volatile write is about 1/3 time to acquire and release a lock. So your second suggestion is a bit faster.

The memory visibility as most people suggested is equivalent, that is any read before the lock acquisition happens before any write after the lock acquisition similar to any read before a volatile read happens before any subsequent write

Improve this answer

Comments

1

The following criteria must be met for volatile variables to provide the desired thread-safety:

  1. Writes to the variable do not depend on its current value.
  2. The variable does not participate in invariants with other variables.

Since both are met here - code is thread-safety

Improve this answer

Comments

0

I think the default synchronization behavior of volatile doesn't guarantee the ReentrantLock behavior, so it might help with performance. Otherwise, I think it's fine.

Improve this answer

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.