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

[Artem Ananiev]

On 8/22/2011 8:24 AM, David Holmes wrote:
> 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.

Looks fine from my perspective too.

Thanks,

Artem

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