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

Peter Veentjer alarmnummer at gmail.com
Sun Mar 16 17:08:49 EDT 2008


Besides the neat race problem, there also is a visiblity problem:
there is no happens before relation between the write and the read of
the listeners field. I just checked the sourcecode of InstanceSupport
of Tomcat and the listeners field is not volatile.

http://jsourcery.com/api/apache/jakarta/tomcat/6.0.10/org/apache/catalina/util/InstanceSupport.source.html#j1338697154

Using a final CopyOnWriteArrayList list would have been a better
solution than the current approach.

On Sun, Mar 16, 2008 at 2:57 PM, Joshua Bloch <josh at bloch.us> wrote:
> Jeremy,
>
> No worries, I've always believed that.
>
>         Josh
>
>
>
> On Sun, Mar 16, 2008 at 12:15 PM, Jeremy Manson <jmanson at cs.umd.edu> wrote:
>
> > Josh,
> >
> > The following may be obvious. :)
> >
> > I think that is only part of the answer.  In this case, you really want
> > a completely separate object ("private ReentrantLock listenersLock")
> > that controls the synchronization on listeners.
> >
> > I think we need to emphasize that synchronization is orthogonal to
> > program logic and data.  Not that this is a particularly original
> > observation, but I think Java made a mistake by integrating
> > synchronization and control logic.  We see it over and over again in
> > this notion that objects provide their own synchronization.
> >
> > There are other benefits to this approach, too, like the fact that you
> > have much better control over the lock of an entirely separate lock
> > object than you do over an arbitrary instance of an object whose methods
> > may acquire its lock.
> >
> >                                        Jeremy
> >
> >
> > Joshua Bloch wrote:
> > > Bill,
> > >
> > > Ouch!  "Synchronize only on final fields."
> > >
> > >           Josh
> > >
> > > P.S.  (No, I hadn't said that previously, but I will now.)
> > >
> > >
> > >
> > > On Fri, Mar 14, 2008 at 1:08 PM, Bob Lee <crazybob at crazybob.org
> >
> > > <mailto:crazybob at crazybob.org>> wrote:
> > >
> > >     Nice one.
> > >
> > >
> > >     On Fri, Mar 14, 2008 at 12:28 PM, Bill Pugh <pugh at cs.umd.edu
> >
> > >     <mailto:pugh at cs.umd.edu>> wrote:
> > >
> > >         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.
> > >
> > >         Bill
> > >         _______________________________________________
> > >         Concurrency-interest mailing list
> > >         Concurrency-interest at altair.cs.oswego.edu
> > >         <mailto:Concurrency-interest at altair.cs.oswego.edu>
> >
> > >
> http://altair.cs.oswego.edu/mailman/listinfo/concurrency-interest
> > >
> > >
> > >
> > >     _______________________________________________
> > >     Concurrency-interest mailing list
> > >     Concurrency-interest at altair.cs.oswego.edu
> > >     <mailto:Concurrency-interest at altair.cs.oswego.edu>
> >
> >
> >
> > >     http://altair.cs.oswego.edu/mailman/listinfo/concurrency-interest
> > >
> > >
> > >
> > > ------------------------------------------------------------------------
> > >
> > > _______________________________________________
> > > Concurrency-interest mailing list
> > > Concurrency-interest at altair.cs.oswego.edu
> > > http://altair.cs.oswego.edu/mailman/listinfo/concurrency-interest
> >
> >
>
>
> _______________________________________________
>  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