Hi David, 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/
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
