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

[Alexander Potochkin]

Hello Anton

The test looks good and the EventQueue fix look good to me.

Thanks
alexp

On 9/3/2015 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.
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/awt-dev/attachments/20150907/47db7cb0/attachment.html>