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 <[email protected]> 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 <[email protected]> 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. >
