Careful how you synchronize toArray()

This is a fuller version of the code snippit I submitted to the interesting thread on java synchronization issues on stack overflow

In general it can be tempting to assume too much thread safety when using synchronized collection classes.
It can be easy to think they somehow grant you more protection than they actually do, and forget to hold the lock between calls.

If have seen this mistake a few times:

 List<String> l = Collections.synchronizedList(new ArrayList<String>());
 String[] s = l.toArray(new String[l.size()]);

In the second line above, the toArray and size() methods are both thread safe in their own right, but the size() is evaluated separately from the toArray(), and the lock on the List is not held between these two calls. If you run this code with another thread concurrently removing items from the list, sooner or later you will end up with a new String[] returned which is larger than required to hold all the elements in the list, and has null values in the tail, which will quite possibly result in a NullPointerException eventually.

It is easy to think that because the two method calls to the List occur in a single line of code this is somehow an atomic operation, but it is not. The best way to fix this is to hold the lock explicitly :

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

 synchronized(l) {
    String[] s = l.toArray(new String[l.size()]);
 }

You can follow any responses to this entry through the RSS 2.0 feed. You can leave a response, or trackback from your own site.

AddThis Social Bookmark Button

Leave a Reply