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

jason marshall jdmarshall at gmail.com
Wed Mar 19 16:40:45 EDT 2008


In encouraging the other David to file a bug against Findbugs, I had a
"wait, what?" moment, but I realized my mistake.  I see on a re-read
of the thread that you described it here.  I just wanted to restate it
to make sure I have the right of it:

Happens-before doesn't save a thread that has already blocked waiting
on a monitor, because the variable has already been dereferenced.

-Jason

On Sat, Mar 15, 2008 at 12:49 AM, David Holmes <dcholmes at optusnet.com.au> wrote:
> 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
>
>
>
>  _______________________________________________
>  Concurrency-interest mailing list
>  Concurrency-interest at altair.cs.oswego.edu
>  http://altair.cs.oswego.edu/mailman/listinfo/concurrency-interest
>



-- 
- Jason


More information about the Concurrency-interest mailing list