<AWT Dev> [8] Review Request: JDK-8025588 [macosx] Frozen AppKit thread in 7u40

[Sergey Bylokhov]

Hi, Petr.
New version looks good as well.

On 09.10.2013 15:31, Petr Pchelko wrote:
> Hello.
>
> Thank you for the review, but I have a new version.
> http://cr.openjdk.java.net/~pchelko/8025588/webrev.02/
>
> In LWCToolkit we never throw an InterruptedException, so I've removed it from the method signature. But I forgot to remove it from the catch clauses in the places where the method is used.
> This version fixes the problem.
>
> With best regards. Petr.
>
> On 09.10.2013, at 15:17, Sergey Bylokhov <Sergey.Bylokhov at oracle.com> wrote:
>
>> Hi, Petr.
>> The fix looks good.
>>
>> On 09.10.2013 14:48, Petr Pchelko wrote:
>>> Hello, Anthony.
>>>
>>>> 1. Can InvocationEvent.notifier be final instead of volatile?
>>> It's protected, so it's a part of the public API. It is extremely unlikely someone would be broken by making this field final, but who knows..
>>>
>>>> 2. InvocationEvent now has 4 ctors, 3 public and 1 private. Can all the public ctors call the private one directly? Right now there is an extra call from one public ctor to another, which then call the private one.
>>> I've update the fix.
>>>
>>>> 3. LWCToolkit.java: for backwards compatibility I suggest to replace "true" with "false" at line 562, so all the uncaught exceptions are thrown to the calling code, as it was before the fix.
>>> Actually I think the behaviour before the fix was a bug. If you look to the end of the invokeAndWait method (LWCToolkit:577-583) we are taking the exceptions from the InvocationEvent and rethrowing them as the IvocationTargetException.
>>> So originally the code was written with the intention to catch exceptions, but this was lost during the refactorings of this code (by me in JDK-8006634)
>>>
>>> The updated fix is available here:
>>> http://cr.openjdk.java.net/~pchelko/8025588/webrev.01/
>>>
>>> With best regards. Petr.
>>>
>>> On 09.10.2013, at 14:31, Artem Ananiev <artem.ananiev at oracle.com> wrote:
>>>
>>>> Hi, Petr,
>>>>
>>>> a few comments:
>>>>
>>>> 1. Can InvocationEvent.notifier be final instead of volatile?
>>>>
>>>> 2. InvocationEvent now has 4 ctors, 3 public and 1 private. Can all the public ctors call the private one directly? Right now there is an extra call from one public ctor to another, which then call the private one.
>>>>
>>>> 3. LWCToolkit.java: for backwards compatibility I suggest to replace "true" with "false" at line 562, so all the uncaught exceptions are thrown to the calling code, as it was before the fix.
>>>>
>>>> Thanks,
>>>>
>>>> Artem
>>>>
>>>> On 10/9/2013 12:54 PM, Petr Pchelko wrote:
>>>>> Hello, AWT Team.
>>>>>
>>>>> Please review the fix for the issue:
>>>>> https://bugs.openjdk.java.net/browse/JDK-8025588
>>>>> The fix is available at:
>>>>> http://cr.openjdk.java.net/~pchelko/8025588/webrev.00/
>>>>> The CCC request for public API changes was approved:
>>>>> http://ccc.us.oracle.com/8025588
>>>>>
>>>>> The problem:
>>>>> Is some cases InvocationEvent was deleted from the EventQueue when it's source was disposed. If this InvocationEvent was created by the LWCToolkit.invokeAnWait, it never exited from the nested event loop, so the application frizzed.
>>>>>
>>>>> The solution:
>>>>> Now when the InvocationEvent is removed it's disposed and the invokeAndWait mechanism gets notified and exits the nested event loop.
>>>>>
>>>>> It's impossible to create a regression test here as the issue is very hard to reproduce.
>>>>>
>>>>> With best regards. Petr.
>>>>>
>>>>>
>>
>> -- 
>> Best regards, Sergey.
>>


-- 
Best regards, Sergey.