On 13/07/2012 8:13 AM, Oleg Pekhovskiy wrote:
Hi David,
the condition you mentioned:
if (!forceDetach && (peekEvent() != null) ||
!SunToolkit.isPostEventQueueEmpty()) {
return false;
}
could lead to understanding problem (braces are the best way to clarify).
&& has higher priority than ||, thus we could rewrite that condition
like this:
( !forceDetach && (peekEvent() != null) ) ||
!SunToolkit.isPostEventQueueEmpty()
Sorry - need to clean my glasses. I was seeing !f && ( X || Y)
:(
That means SunToolkit.isPostEventQueueEmpty() is called when:
forceDetach == true
OR
peekEvent() == null
OR
both above.
Anthony has pushed the fix that changes (and simplifies) that place:
http://cr.openjdk.java.net/~anthony/7u6-16-missingAWTThread-7162144.0/
So maybe we should discuss the new code - the new behavior.
Yes I saw that patch too. All clear now.
Thanks,
David
Thanks,
Oleg
7/13/2012 1:10 AM, David Holmes wrote:
On 12/07/2012 10:18 PM, Anthony Petrov wrote:
On 07/12/12 05:41, David Holmes wrote:
<snip>
It also seems that with this fix the interrupted EDT will not detach
due
to the flush being in progress. The EDT will resume the pumpEvents loop
in its run() method. If the interrupt state of the EDT has not been
maintained then the fact it was interrupted (and should detach?) is now
lost. If it is maintained then presumably we will try to
detachDispatchThread again, in which case the EDT will enter a busy
polling loop trying, but failing, to detach, until the flush() has
actually completed.
When interrupt() is called on EDT, the shutdown flag is set, which is
subsequently passed as the forceDetach parameter to
detachDispatchingThread(). If the parameter is true, the detaching
happens unconditionally. So this shouldn't be an issue.
I don't quite follow this part. The logic is:
if (!forceDetach && (peekEvent() != null) ||
!SunToolkit.isPostEventQueueEmpty()) {
return false;
}
If forceDetach is true then we won't execute peekEvent() and
isPostEventQueueEmpty(), which means we would not have hit the
original deadlock.
Oleg's response seems to indicate that we will indeed keep looping but
because of the values of various flags we won't do anything "useful".
But that is the kind of "busy wait" that I was referring to.
Cheers,
David
--
best regards,
Anthony
Cheers,
David
Thanks,
Oleg
7/11/2012 7:35 PM, Anthony Petrov wrote:
Hi Oleg,
1. I suggest to rename isPending to isFlushing, as it reflects the
current state of the PostEventQueue more precisely.
2. I suggest to use the try{}finally{} pattern to set/reset the new
flag in the flush() method to ensure the flag is reset even if the
while() loop throws an exception.
Otherwise the fix looks good.
--
best regards,
Anthony
On 7/11/2012 7:24 PM, Oleg Pekhovskiy wrote:
Hi!
Please review the fix for CR:
http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7177040
Webrev:
http://cr.openjdk.java.net/~bagiras/7u6/7177040.1
The idea of the fix is that there are two concurrent threads that
try
to get two synchronization objects EventQueue.pushPopLock and
PostEventQueue itself.
For NetBeans these threads are (EDT & WarmUp) or (EDT & TimerQueue).
Problem happens when EDT is interrupted and goes to
EventQueue.detachDispatchThread() where EventQueue.pushPopLock is
owned and
EDT is waiting to own PostEventQueue when calling
SunToolkit.isPostEventQueueEmpty() -> PostEventQueue.noEvents().
At the same time another thread calls EventQueue.postEvent() that
calls EventQueue.flushPendingEvents() -> PostEventQueue.flush()
where
PostEventQueue is owned
and another thread is waiting to own EventQueue.pushPopLock when
calling EventQueue.postEvent() -> EventQueue.postEventPrivate().
To avoid potential deadlock I moved synchronization out of
postEvent()'s cycle in PostEventQueue.flush(),
but to be clear about the existence of Events that are not posted to
EventQueue yet I added PostEventQueue.isPending flag that is true
until the end of the cycle.
There are only two classes that utilize PostEventQueue.flush()
method: SunToolkit.flushPendingEvents() and
SunToolkitSubclass.flushPendingEvents().
They are both synchronized by static SunToolkit.flushLock. That
eliminates the situation when we have overlapped calls of
PostEventQueue.flush().
Thanks,
Oleg
