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

[Anthony Petrov]

Hi Clemens,

On 08/20/11 02:12, 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/

The fix looks fine to me.

--
best regards,
Anthony


>     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.
>
>
>     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.
>
>
> Thanks, Clemens