[concurrency-interest] New bug pattern, way to common

David Holmes dcholmes at optusnet.com.au
Sat Mar 15 03:49:35 EDT 2008


Bill Pugh writes:
> I identified a new concurrency bug pattern that is way too common:
>
> Synchronizing on a field in order to prevent concurrent access to that
> field.
>
> For example:
>    /**
>       * Add a lifecycle event listener to this component.
>       *
>       * @param listener The listener to add
>       */
>      public void addInstanceListener(InstanceListener listener) {
>
>        synchronized (listeners) {
>            InstanceListener results[] =
>              new InstanceListener[listeners.length + 1];
>            for (int i = 0; i < listeners.length; i++)
>                results[i] = listeners[i];
>            results[listeners.length] = listener;
>            listeners = results;
>        }
>
>      }
>
> There are 26 occurrences of this pattern in apache-tomcat.
>
> Of course, if the field in question is of type Boolean, it becomes
> exceptionally problematical.

I've seen the Boolean case a number of times in forum postings. Typically it
is used with a wait/notify predicated on the Boolean's value. So at least in
that case you get an IllegalMonitorStateException. Though the posters are
always baffled as to why.

Now for the folks out there that are still new to all this and can't quite
see what the problem is, consider this: Suppose listeners refers to array A
and Thread-1 is busy making a new array B, when Thread-2 tries to do the
same. Thread-2 blocks on the lock for array A. When Thread-1 is done with
the copy it sets listeners to refer to array B and releases the lock on
array A. Thread-2 now locks array A and then starts to make a copy of array
B. Meanwhile Thread-3 comes along and grabs the lock of array B and so two
threads are now making copies of array B because they are using different
locks. At a minimum one of the new listeners will get lost, and its possible
that one thread will get ArrayIndexOutOfBoundsException as the listeners
reference (and hence the length) can change at any point during the method.
Ouch indeed!

Plus there are memory model issues due to synchronization on different
locks - but the lack of mutual exclusion is much more serious.

Nice catch Bill!

Cheers,
David Holmes



More information about the Concurrency-interest mailing list