<AWT Dev> numAppContexts in AppContext modified in not-thread-safe way.

[David Holmes]

On 20/08/2011 8:12 AM, Clemens Eisserer wrote:
>     The original code is rather strange. The static field will be default
>     initialized to zero, the AppContext constructor will increment it to 1,
>     then the static block explicitly sets it to one.
>     It would be cleaner/clearer if the field were declared and initialized
>     before the static block.
>
> Agreed. Please find the modified patch at:
> http://cr.openjdk.java.net/~ceisserer/7080700/webrev.04/

Looks good. Just need someone from AWT to chime in.

>
>     That's an uninteresting case from a data-race perspective though because
>     the counter is only ever written once.
>
> In the constructor the non-atomic increment can cause a missed update and
> therefor cause numAppContexts to become 1 or even 0, which could trigger
> that logic unintentionally.

My point was that in a Swing app the constructor is only ever called once 
and so there can not be a missed update. Anyway I was just musing :)

>
>     However I now wonder about how often this gets called in a Swing app? If
>     this is a frequent call then the overhead of the atomic might be
>     significant. Perhaps the Swing folk should be evaluating this change as
>     well?
>
> Did some benchmarks,  and at least on x86 AtomicInteger.get() performs
> identical to a volatile field read.

And of course I knew that because the intrinsics that underlie the atomics 
are the same as those for volatiles. :) Thanks for verifying.

Cheers,
David

>
> Thanks, Clemens