views:

2130

answers:

9

I need to make an ArrayList of ArrayLists thread safe. I also cannot have the client making changes to the collection. Will the unmodifiable wrapper make it thread safe or do I need two wrappers on the collection?

+1  A: 

The unmodifiable wrapper only prevents changes to the structure of the list that it applies to. If this list contains other lists and you have threads trying to modify these nested lists, then you are not protected against concurrent modification risks.

Dan Dyer
+1  A: 

From looking at the Collections source, it looks like Unmodifiable does not make it synchronized.

static class UnmodifiableSet<E> extends UnmodifiableCollection<E>
        implements Set<E>, Serializable;

static class UnmodifiableCollection<E> implements Collection<E>, Serializable;

the synchronized class wrappers have a mutex object in them to do the synchronized parts, so looks like you need to use both to get both. Or roll your own!

John Gardner
+3  A: 

It depends. The wrapper will only prevent changes to the collection it wraps, not to the objects in the collection. If you have an ArrayList of ArrayLists, the global List as well as each of its element Lists need to be wrapped separately, and you may also have to do something for the contents of those lists. Finally, you have to make sure that the original list objects are not changed, since the wrapper only prevents changes through the wrapper reference, not to the original object.

You do NOT need the synchronized wrapper in this case.

Michael Borgwardt
I have learned so much since I posted this question. Just goes to prove that just because the answer is accepted and/or popular that it is correct. To many wrong answers an this site are accepted just because that they are popular. I changed the accepted answer to this one after I learned more.
WolfmanDragon
A: 

this is neccessary if 1) there is still a reference to the original modifiable list 2) the list will possibly be accessed though an iterator.

if you intend to read from the arraylist by index only you could assume this is thread-save.

when in doubt, chose the synchronized wrapper.

Andreas Petersson
A: 

Not sure if I understood what you are trying to do, but I'd say the answer in most cases is "No".

If you setup an ArrayList of ArrayList and both, the outer and inner lists can never be changed after creation (and during creation only one thread will have access to either inner and outer lists), they are probably thread safe by a wrapper (if both, outer and inner lists are wrapped in such a way that modifying them is impossible). All read-only operations on ArrayLists are most likely thread-safe. However, Sun does not guarantee them to be thread-safe (also not for read-only operations), so even though it might work right now, it could break in the future (if Sun creates some internal caching of data for quicker access for example).

Mecki
A: 

I believe that because the UnmodifiableList wrapper stores the ArrayList to a final field, any read methods on the wrapper will see the list as it was when the wrapper was constructed as long as the list isn't modified after the wrapper is created, and as long as the mutable ArrayLists inside the wrapper aren't modified (which the wrapper can't protect against).

djpowell
+2  A: 

On a related topic - I've seen several replies suggesting using synchronized collection in order to achieve thread safety. Using synchronized version of a collection doesn't make it "thread safe" - although each operation (insert, count etc.) is protected by mutex when combining two operations there is no guarantee that they would execute atomically. For example the following code is not thread safe (even with a synchronized queue):

if(queue.Count > 0) { queue.Add(...); }

Dror Helper
+2  A: 

An immutable object is by definition thread safe (in this context), so synchronization is not necessary.

Wrapping the outer ArrayList using Collections.unmodifiableList() prevents the client from changing its contents (and thus makes it thread safe), but the inner ArrayLists are still mutable.

Wrapping the inner ArrayLists using Collections.unmodifiableList() too prevents the client from changing their contents (and thus makes them thread safe), which is what you need.

Let us know if this solution causes problems (overhead, memory usage etc); other solutions may be applicable to your problem. :)

volley
The wrapped collection is *not* immutable, however, as long as another Thread maintains a reference to the mutable version. That other thread can modify the collection, and with synchronization, there is no guarantee that other threads will see the changes.
Eddie
+1  A: 

It will be thread-safe if the unmodifiable view is safely published, and the modifiable original is never ever modified (including all objects recursively contained in the collection!) after publication of the unmodifiable view.

If you want to keep modifying the original, then you can either create a defensive copy of the object graph of your collection and return an unmodifiable view of that, or use an inherently thread-safe list to begin with, and return an unmodifiable view of that.

You cannot return an unmodifiableList(synchonizedList(theList)) if you still intend to access theList unsynchronized afterwards; if mutable state is shared between multiple threads, then all threads must synchronize on the same locks when they access that state.

Christian Vest Hansen