<AWT Dev> [8] Review request for 8019623: Lack of synchronization in AppContext.getAppContext()

[Artem Ananiev]

On 10/8/2013 6:54 PM, Leonid Romanov wrote:
> Hi,
> I tried different variations of your suggestion and none of them worked. The reason is timing: suppose two threads, A and B, entered AppContetx.getAppContext() and then AppContext.initMainAppContext() at the same time. We need thread A to be able to initialize mainAppContext, return from initMainAppContext() and then initialize local variable with AppContext to be returned without thread B intervening in between. This just doesn't happen.

This is perfectly expected, that the created regression test won't fail 
without your fix. It will still be useful to prevent regression in the 
future.

Thanks,

Artem

> On Oct 4, 2013, at 1:06 PM, Artem Ananiev <artem.ananiev at oracle.com> wrote:
>
>> Hi, Leonid,
>>
>> the fix looks fine.
>>
>> I believe it is possible to create a regression test for this fix. The test should create several thread groups and call AC.getAC() there, then check that all the returned AC's are equal. Although it won't 100% fail before your fix, it should 100% pass after it. What do you think about it?
>>
>> Thanks,
>>
>> Artem
>>
>> On 10/2/2013 6:22 PM, Leonid Romanov wrote:
>>> Hello,
>>> Please, review a fix for 8019623: Lack of synchronization in AppContext.getAppContext(). I don't think it makes sense to ensure that AppContext.getAppContext()/SunToolkit.createNewAppContext() is thread safe, because in practice we haven't had any problems with this combination. What we have had problems with is the getAppContext()/getAppContext() combination because of the double mainAppContext initialization.  Therefore, the fix takes care of this issue. While I could have simply made the whole AppContext.getAppContext() method synchronized, I wanted the most common path through it (returning cached AppContext) to remain lock-free, so I decided to place the lock around the most critical part, where initMainAppContext() gets called.
>>> As for a regression test, I couldn't find a way to detect double initMainAppContext initialization. We put newly created AppContexts into a thread-safe map, so whatever AppContext wins the race remains in the map, and the other is simply lost, without any convenient means to detect its existence.
>>>
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8019623
>>> Webrev: http://cr.openjdk.java.net/~leonidr/8019623/webrev.00/
>>>
>>> Thanks,
>>> Leonid.
>>>
>