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

David Holmes dcholmes at optusnet.com.au
Wed Mar 19 19:35:55 EDT 2008


Right - I think :) Happens-before doesn't really come into it here - once
you are on the monitor queue, the reference you used to access the object on
which monitor you are now waiting, is of no consequence and any change to
the value of that reference has no affect on you.

David Holmes

> -----Original Message-----
> From: concurrency-interest-bounces at cs.oswego.edu
> [mailto:concurrency-interest-bounces at cs.oswego.edu]On Behalf Of jason
> marshall
> Sent: Thursday, 20 March 2008 6:41 AM
> To: Concurrency-interest at cs.oswego.edu
> Subject: Re: [concurrency-interest] New bug pattern, way to common
>
>
> 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
> _______________________________________________
> Concurrency-interest mailing list
> Concurrency-interest at altair.cs.oswego.edu
> http://altair.cs.oswego.edu/mailman/listinfo/concurrency-interest
>



More information about the Concurrency-interest mailing list