Hi, Petr.
A few comments:
case RESULT_TIMEOUT:
412 if (EventQueue.isDispatchThread()) {
413 // Deadlocked. Have to return and let EDT process
some events
Comment is not accurate. Most of the time syncNativeQueue is called on EDT and
deadlock is possible but not necessary.
414 return false;
415 } else {
416 // The timeout is too low, because deadlock is
impossible.
417 // Make another attempt to flush with bigger
timeout.
418 waitTime += 100;
What happens if syncNativeQueue is called on Appkit?
419 }
420 break;
On 17.01.2014 17:52, Petr Pchelko wrote:
Hello, AWT Team.
Let's reveal this discussion.
The bug: https://bugs.openjdk.java.net/browse/JDK-7185258
The fix: http://cr.openjdk.java.net/~pchelko/9/7185258/webrev.01/
This review was left because there was an idea to use a different approach to
this problem: just use the timeout, detect a deadlock and return false from
syncNativeQueue.
I've implemented this approach and played with it, but it appears to work not
really well. The problem is the timeout: if we set it too little we would have
too many false positive deadlocks
which would result in bad synchronization and tremendously increasing
probability of the InfiniteLoop exception. If the timeout is too big tests slow
down badly, but what's even worse - if we wait
for a deadlock too long some events could get posted to EDT (repaints, timer
updates etc..) resulting in an InfiniteLoop exception. So, I've decided to
return back to my original approach.
The idea of this fix is to interrupt waiting as soon as we detect that we've
got into the nested Appkit loop. This happens in DnD and in
LWCToolkit.invokeAndWait.
With this fix several tests start to pass. Tested on DnD-related regression
tests.
2. To dispatch some DropTarget events on EDT synchronously we start our own
nested loop in doAWTRunLoop using the ToolkitThreadBlockedHandler. In this case
we do process the events in a nested runLoop.
This case might actually be a bug, because we are short-circuiting normal event
processing and could "steal" some events from Cocoa. But I would not address
this it in this fix.
Probably it is a good time to realize how it should work? Since the fix adds
complexity which can not be necessary in the future.
I agree. But this would be a long investigation, so let's leave this review
until I find out how we should really work.
This question arises during the review. It's not related, but anyway:
Our nested loop is indeed quite dangerous, because it could really still events
from Cocoa, so it should be used with great cation. But we need to process the
events in this nested loop, because DropTarget event handlers are called
synchronously and opening a modal dialog in such a handler would completely
block the application if we do not process events in nested loop. So,
everything works fine right now.
With best regards. Petr.
On 24.12.2013, at 12:30, Petr Pchelko <[email protected]> wrote:
Hello, Sergey.
As I remember doAWTRunLoop process events and selectors in case of drags/DnD
otherwise how we got mouse events? Or I have missed something?
In case of DnD we have several nested loops in place:
1. The native Cocoa DnD nested loop. It's started within the NSView
dragImage:... call. It processes only the DnD-related events - some mouse
events, some key events and flag changed events. This is for the DragSource.
And we cannot emulate such event in postDummyevent?
We could post something like a dummy MouseUp event and get rid of it in the
NSApplication sendEvent:..., but I don't like such a solution.
May be Cocoa has more nested event loops we do not know about yet which do not
accept MouseUp. The same would be valid for any event type.
2. To dispatch some DropTarget events on EDT synchronously we start our own
nested loop in doAWTRunLoop using the ToolkitThreadBlockedHandler. In this case
we do process the events in a nested runLoop.
This case might actually be a bug, because we are short-circuiting normal event
processing and could "steal" some events from Cocoa. But I would not address
this it in this fix.
Probably it is a good time to realize how it should work? Since the fix adds
complexity which can not be necessary in the future.
I agree. But this would be a long investigation, so let's leave this review
until I find out how we should really work.
With best regards. Petr.
On 24.12.2013, at 12:21, Sergey Bylokhov <[email protected]> wrote:
On 12/24/13 11:57 AM, Petr Pchelko wrote:
Hello, Sergey.
As I remember doAWTRunLoop process events and selectors in case of drags/DnD
otherwise how we got mouse events? Or I have missed something?
In case of DnD we have several nested loops in place:
1. The native Cocoa DnD nested loop. It's started within the NSView
dragImage:... call. It processes only the DnD-related events - some mouse
events, some key events and flag changed events. This is for the DragSource.
And we cannot emulate such event in postDummyevent?
2. To dispatch some DropTarget events on EDT synchronously we start our own
nested loop in doAWTRunLoop using the ToolkitThreadBlockedHandler. In this case
we do process the events in a nested runLoop.
This case might actually be a bug, because we are short-circuiting normal event
processing and could "steal" some events from Cocoa. But I would not address
this it in this fix.
Probably it is a good time to realize how it should work? Since the fix adds
complexity which can not be necessary in the future.
Without the DnD we use a doAWTRunLoop which does not process events. (Except
the case with Single-Threaded Interop with FX).
Actually, after some more thinking I believe I should reimplement the fix and
follow Anthony's suggestion with a timeout, because my fix is too complex. I'll
send a new webrev later today.
With best regards. Petr.
On 24.12.2013, at 1:41, Sergey Bylokhov <[email protected]> wrote:
Hi, Petr.
On 23.12.2013 16:54, Petr Pchelko wrote:
The problem:
nativeSyncQueue synchronizes the native event queue. However, there are several
situations when a dummy event posted to the native queue is not dispatched.
These are our own nested loop in doAWTRunLoop and Cocoa's internal nested loop
in NSView:dragImage.
As I remember doAWTRunLoop process events and selectors in case of drags/DnD
otherwise how we got mouse events? Or I have missed something?
Solution:
1. The interruptNativeSyncQueue was introduced. In case we are waiting for the
native event queue this method interrupts waiting, otherwise it's a no-op. This
is needed for the following reason: suppose the nativeSyncQueue is called on
EDT. While the queue is flushed some event was processed which caused us to
call doAWTRunLoop. As EDT is blocked we would have got a deadlock. Interrupting
the wait lets EDT flush it's events and lets AppKit exit a nested loop. When
the nativeSyncQueue is interrupted we do not immediately exit realSync, but
flush EDT and try to sync native queue again. Most likely in this case we would
not be in a nested loop and will successfully sync a native queue.
2. A lightweight version of nativeSyncQueue was introduced. This one does not
flush the event queue, it only flushes a selector queue. This is needed to not
deadlock when realSync was called during DnD. Suppose nativeSyncQueue is called
while the app is in the native nested dragging loop. Until dragging operation
finishes only dragging-related events will be processed. So we have no
opportunity to flush the queue as our dummy event will be blocked by a dragging
nested loop.
I've tested it by running almost all awt and swing tests. No new failures, some
tests start to pass after the fix.
With best regards. Petr.
--
Best regards, Sergey.
--
Best regards, Sergey.
--
Best regards, Sergey.
