<AWT Dev> [9, 8] Review request for 8081485: EDT auto shutdown is broken in case of new event queue usage

[Sergey Bylokhov]

Hi, Anton.
The fix looks fine, but after the fix was approved the modules came to 
play. Please add correct @modules tag in the test before the push, 
because it uses internal api from sun.awt package. I guess it should be 
@modules java.desktop/sun.awt


On 03.09.15 18:39, Anton Litvinov wrote:
> Hello Sergey and Artem,
>
> After Anton Nashatyrev left the organization, I was assigned to this bug
> to complete work on it. Though Sergey approved the 1st version of the
> fix, the second approval was not received, and the fix was not
> integrated into "jdk9/client". Today I have compiled "jdk9/client" with
> the 1st version of the fix for all supported platforms and:
>
> 1. Verified using the manual test case from the bug description that
> *the fix resolves the bug*.
> 2. Found out that *the regression test "EventQueuePushAutoshutdown.sh"*
> from the fix *always fails on OS X* with the message:
>
>      "Unrecognized system!  Darwin"
>
> because the shell script expects only "SunOS", "Linux", "CYGWIN*",
> "Windows_*" versions of OS and does not execute the test on OS X.
> Therefore the 2nd version of the fix resolving this issue was created.
>
> Could you please review the 2nd version of the fix.
>
> Webrev (2nd version of the fix):
> http://cr.openjdk.java.net/~alitvinov/8081485/webrev.01
> Webrev (1st version of the fix):
> http://cr.openjdk.java.net/~anashaty/8081485/webrev.00
>
> The 2nd version of the fix modifies *only the regression test part* of
> the fix:
>
> Changes in
> "test/java/awt/Toolkit/AutoShutdown/EventQueuePush/EventQueuePushAutoshutdown.java":
> - "37     private volatile int status = 2;" - Modifier "volatile" was
> added, since the variable is accessed from several threads.
> - "28   @author     Anton Nashatyrev : area=toolkit" - This line was added.
>
> Changes in
> "test/java/awt/Toolkit/AutoShutdown/EventQueuePush/EventQueuePushAutoshutdown.sh":
> - "SunOS", "Linux", "CYGWIN*" were combined in one case of the switch
> and "Darwin" was added also to this case.
>
> Thank you,
> Anton
>
> On 7/2/2015 10:52 PM, Sergey Bylokhov wrote:
>> Hi, Anton.
>> The fix looks fine.
>>
>> On 01.07.15 16:07, Anton Nashatyrev wrote:
>>> Hello Artem, Sergey
>>>
>>>     thanks for your help with the issue investigation!
>>>
>>>     could you please review the following fix:
>>>
>>> fix: http://cr.openjdk.java.net/~anashaty/8081485/webrev.00/
>>> <http://cr.openjdk.java.net/%7Eanashaty/8081485/webrev.00/>
>>> bug: https://bugs.openjdk.java.net/browse/JDK-8081485
>>>
>>>     Problem: EventQueue.push() at the very beginning prevents the app
>>> from shutting down automatically
>>>
>>>     Reason: the EventQueue.push() first checks if the EDT for the top
>>> EventQeue exists (to transfer it to the new EventQueue) and then
>>> implicitly starts it.
>>>
>>>     Fix: do posting of dummy event only in case if the EventQueue had
>>> the EDT
>>>
>>> Thanks!
>>> Anton.
>>
>>
>> --
>> Best regards, Sergey.
>


-- 
Best regards, Sergey.