[RFC][icedtea-web]: possible race condition.

[Dr Andrew John Hughes]

On 19:02 Mon 07 Mar     , Omair Majid wrote:
> On 03/07/2011 06:55 PM, Denis Lila wrote:
> > This is better. It fixes a few issues I discussed with
> > Omair on irc.
> >
> 
> Looks fine to me.
> 

Have you thought about using an AtomicLong instead?

> 
> > ----- Original Message -----
> >> >  Hi.
> >> >
> >> >  This patch fixes what I think is a concurrency problem.
> >> >  We do synchronized(requestIdentityCounter) in order to
> >> >  prevent race conditions, but that doesn't work because
> >> >  "synchronized" locks the object, not the reference. As
> >> >  soon as we write something to the reference (i.e. the =0L
> >> >  or the ++), we allow other threads to enter the synchronized
> >> >  region while we are still in it (because we don't have
> >> >  the lock of the new object that the reference points to).
> >> >  This could result in 2 different requests having the same
> >> >  identifier, which would be very bad.
> >> >
> >> >  The patch also fixes the use of "==" to compare 2 Long
> >> >  objects.
> >> >
> >> >  Ok to push?
> >> >
> >> >  Thank you,
> >> >  Denis.
> >
> >
> > synchronization.patch
> >
> >
> > diff -r 2446ec028f75 ChangeLog
> > --- a/ChangeLog	Mon Mar 07 14:48:01 2011 -0500
> > +++ b/ChangeLog	Mon Mar 07 18:59:03 2011 -0500
> > @@ -1,3 +1,10 @@
> > +2011-03-07  Denis Lila<dlila at redhat.com>
> > +
> > +	* plugin/icedteanp/java/sun/applet/PluginAppletViewer.java
> > +	(getRequestIdentifier): fixed race condition by synchronizing
> > +	on mutex.
> > +	(requestIdentityCounter): made it a long.
> > +
> 
> It would be nice if the ChangeLog would contain a little more of the 
> reasoning behind it (like your email does).
> 
> >   2011-03-07  Omair Majid<omajid at redhat.com>
> >
> >   	* NEWS: Update.
> > diff -r 2446ec028f75 plugin/icedteanp/java/sun/applet/PluginAppletViewer.java
> > --- a/plugin/icedteanp/java/sun/applet/PluginAppletViewer.java	Mon Mar 07 14:48:01 2011 -0500
> > +++ b/plugin/icedteanp/java/sun/applet/PluginAppletViewer.java	Mon Mar 07 18:59:03 2011 -0500
> > @@ -356,7 +356,8 @@
> >
> >       public static final int APPLET_TIMEOUT = 180000;
> >
> > -    private static Long requestIdentityCounter = 0L;
> > +    private static final Object requestMutex = new Object();
> > +    private static long requestIdentityCounter = 0L;
> >
> >       private Image bufFrameImg;
> >       private Graphics bufFrameImgGraphics;
> > @@ -990,11 +991,11 @@
> >        *
> >        *  @return A unique Long identifier for the request
> >        */
> > -    private static Long getRequestIdentifier() {
> > -        synchronized (requestIdentityCounter) {
> > -
> > -            if (requestIdentityCounter == Long.MAX_VALUE)
> > +    private static long getRequestIdentifier() {
> > +        synchronized(requestMutex) {
> > +            if (requestIdentityCounter == Long.MAX_VALUE) {
> >                   requestIdentityCounter = 0L;
> > +            }
> >
> >               return requestIdentityCounter++;
> >           }
> 
> 
> Cheers,
> Omair

-- 
Andrew :)

Free Java Software Engineer
Red Hat, Inc. (http://www.redhat.com)

Support Free Java!
Contribute to GNU Classpath and IcedTea
http://www.gnu.org/software/classpath
http://icedtea.classpath.org
PGP Key: F5862A37 (https://keys.indymedia.org/)
Fingerprint = EA30 D855 D50F 90CD F54D  0698 0713 C3ED F586 2A37