Hi, Peter,
thanks for reporting this issue, it's clearly a bug in SunToolkit. I
think it will be fixed as a part of 7159230.
See more comments below.
On 6/28/2012 11:53 AM, Peter Levart wrote:
Also, this is another spot that uses it's own synchronizer to serialize access
and calls into the PostEventQueue which is also synchronized (currently by
using synchronized methods but as I proposed in a previous message "deadlock
involving ..." could be using a global mutex) so it is prone to deadlock
"bugs".
I would rather use a ThreadLocal flag here to prevent re-entrancy from the same
thread. Multiple threads can enter flushPendingEvents() with no danger since
PostEventQueue.flush() is already synchronized.
flushPendingEvents() is not called from PostEventQueue.flush(). Instead,
it's mostly called from EventQueue to make sure all the already posted
events are processed. That's why we need synchronization in
flushPendingEvents().
For example like this:
private static final ThreadLocal<Boolean> isFlushingPendingEvents =
new ThreadLocal<Boolean>();
/*
* Flush any pending events which haven't been posted to the AWT
* EventQueue yet.
*/
public static void flushPendingEvents() {
boolean wasFlushing = isFlushingPendingEvents.get() != null;
try {
Here the thread may be interrupted, and "wasFlushing" may become stale.
// Don't call flushPendingEvents() recursively
if (!wasFlushing) {
isFlushingPendingEvents.set(true);
AppContext appContext = AppContext.getAppContext();
PostEventQueue postEventQueue =
(PostEventQueue)appContext.get(POST_EVENT_QUEUE_KEY);
if (postEventQueue != null) {
postEventQueue.flush();
}
}
} finally {
if (!wasFlushing) isFlushingPendingEvents.remove();
}
}
The easiest fix can be to check "isFlushingPendingEvents" in the
beginning of the method:
flushLock.lock();
boolean wasFlushing = isFlushingPendingEvents;
try {
// Don't call flushPendingEvents() recursively
if (!isFlushingPendingEvents) {
and rewrite the "finally" block:
} finally {
if (!wasFlushing) {
isFlushingPendingEvents = false;
}
flushLock.unlock();
}
But a better fix would be to get rid of the separate lock (flushLock)
and use the one from EventQueue (pushPopLock). It's not as trivial as it
seems to be, EventQueue and SunToolkit are in different packages, but
definitely doable.
Thanks,
Artem
Peter
On Thursday, June 28, 2012 09:36:08 AM Peter Levart wrote:
While looking at the SunToolkit code I spoted another bug. The following
code:
private static final Lock flushLock = new ReentrantLock();
private static boolean isFlushingPendingEvents = false;
/*
* Flush any pending events which haven't been posted to the AWT
* EventQueue yet.
*/
public static void flushPendingEvents() {
flushLock.lock();
try {
// Don't call flushPendingEvents() recursively
if (!isFlushingPendingEvents) {
isFlushingPendingEvents = true;
AppContext appContext = AppContext.getAppContext();
PostEventQueue postEventQueue =
(PostEventQueue)appContext.get(POST_EVENT_QUEUE_KEY);
if (postEventQueue != null) {
postEventQueue.flush();
}
}
} finally {
isFlushingPendingEvents = false;
flushLock.unlock();
}
}
... is clearly wrong. The isFlushingPendingEvents flag is reset in finally
block regardless of whether it was true or false at the entry to the try
block.
The 1st nested call to flushPendingEvents() prevents recursion but it also
resets the flag so that the second nested call is allowed...
Regards, Peter
