Java concurrency question - synchronizing on a collection

Question

Will the following code snippet of a synchronized ArrayList work in a multi-threaded environment?

class MyList {
    private final ArrayList<String> internalList = new ArrayList<String>();

    void add(String newValue) {
        synchronized (internalList) {
            internalList.add(newValue);
        }
    }

    boolean find(String match) {
        synchronized (internalList) {
            for (String value : internalList) {
                if (value.equals(match)) {
                    return true;
                }
            }
        }

        return false;
    }
}

I'm concerned that one thread wont be able to see changes by another thread.

Is there a reason why you aren't using List.contains(match)? in find()? — notnoop
– notnoop, Commented Jan 6, 2010 at 21:55
Also, I think you are better off using a Set rather than a list. Finding an element in Set is much faster. — notnoop
– notnoop, Commented Jan 6, 2010 at 21:58

Chandra Patni · Accepted Answer · 2010-01-07 07:00:24Z

5

Your code will work and is thread-safe but not concurrent. You may want to consider using ConcurrentLinkedQueue or other concurrent thread-safe data structures like ConcurrentHashMap or CopyOnWriteArraySet suggested by notnoop and employ contains method.

class MyList {
    private final ConcurrentLinkedQueue<String> internalList = 
         new ConcurrentLinkedQueue<String>();

    void add(String newValue) {
        internalList.add(newValue);
    }

    boolean find(String match) {
        return internalList.contains(match);
    }
}

edited Jan 7, 2010 at 7:00

answered Jan 6, 2010 at 21:54

Chandra Patni

17.6k10 gold badges58 silver badges65 bronze badges

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

2 Comments

notnoop Over a year ago

You should also consider using ConcurrentHashMap or CopyOnWriteArraySet, as those would significantly increase your find operation

Pool Over a year ago

Apologies that my code was a bit simplified. The actual code stores objects and need to perform custom action.

meriton · Accepted Answer · 2010-01-07 01:35:25Z

3

This should work, because synchronizing on the same object establishes a happens-before relationship, and writes that happen-before reads are guaranteed to be visible.

See the Java Language Specification, section 17.4.5 for details on happens-before.

edited Jan 7, 2010 at 1:35

answered Jan 6, 2010 at 21:52

meriton

70.9k14 gold badges117 silver badges190 bronze badges

2 Comments

Pool Over a year ago

Thanks, so another thread is guaranteed to see any changes because of the synchronize flushes the array list state?

meriton Over a year ago

Actually, this is not a problem with flushing, but with optimizations done by the JIT. The JIT assumes that fields are only modified by the current thread, unless the fields are accessed within a synchronized block, or are volatile. For details, check the JLS.

axtavt · Accepted Answer · 2010-01-06 21:50:25Z

2

It will work fine, because all access to the list is synchronized. Hovewer, you can use CopyOnWriteArrayList to improve concurrency by avoiding locks (especially if you have many threads executing find).

answered Jan 6, 2010 at 21:50

axtavt

243k42 gold badges518 silver badges486 bronze badges

Comments

fastcodejava · Accepted Answer · 2010-01-06 21:54:36Z

2

It will work, but better solution is to create a List by calling Collections.synchronizedList().

answered Jan 6, 2010 at 21:54

fastcodejava

41.3k31 gold badges142 silver badges191 bronze badges

Comments

Milhous · Accepted Answer · 2010-01-07 00:45:04Z

0

You may want to consider using a Set(Tree or Hash) for your data as you are doing lookups by a key. They have methods that will be much faster than your current find method.

HashSet<String> set = new HashSet<String>();
Boolean result = set.contains(match); // O(1) time

answered Jan 7, 2010 at 0:45

Milhous

14.8k16 gold badges67 silver badges83 bronze badges

Collectives™ on Stack Overflow

Java concurrency question - synchronizing on a collection

5 Answers 5

2 Comments

2 Comments

Comments

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

5 Answers 5

2 Comments

2 Comments

Comments

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related