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

[Omair Majid]

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.


> ----- 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