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

[David Holmes]

On 19/08/2011 8:02 AM, Clemens Eisserer wrote:
> Mario wrote:
>
>     It looks good to me, and I don't expect any issues after this patch.
>     The only thing is that you should check the settings for the editor, I
>     think you still use a mix of tabs and spaces :)
>
>
> Hmm, I should really check those eclipse settings ^^
>
>
>     Also,
>
>     public final static AppContext getAppContext()
>
>     Doesn't seem to be thread safe by nature, I don't know if this is part
>     of the issue you found, so maybe you should consider to synchronize
>     this as well.
>
>
> Could be I got this wrong ... but isn't the idea behind getAppContext() that
> it can be called  from any thread, and get the AppContext(if there is any) back?

Right. The appContext table is a synchronized map keyed on the current 
ThreadGroup. The first thread created in a ThreadGroup for a given 
AppContext will create the AppContext before any other threads can be 
created, hence this is thread-safe.

The change to use AtomicInteger seems okay - the main concern would be class 
initialization problems, but presumably that doesn't happen. Part of the 
missing picture here is how AppContexts get created and dispose()'d as it 
may be that the way AppContext is used you can't actually get concurrent 
modification of numAppContexts. But there's no way to discern that from the 
AppContext code so it would be safer to use the AtomicInteger.

Cheers,
David Holmes

>
> - Clemens