Hi Petr,
The fix looks fine to me.
--
best regards,
Anthony
On 10/09/2013 03:31 PM, 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 <[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.
