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.

Answer 1

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).

Answer 2

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.

Answer 3

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

Answer 4

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);
    }
}

Answer 5

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