On 25/08/2011 2:20 AM, Clemens Eisserer wrote:
Hi again,
Please find the latest version of the "full" patch at:
http://cr.openjdk.java.net/~ceisserer/7081670/webrev_full.02/
Because the return value of pumpOneEventForFilters(int id) redundant as it
always equals shutdown, I now just set shutdown and return nothing.
You changed from Lock to ReentrantLock so that you could use
isHeldByCurrentThread(), but that locks in (pardon the pun) the kind of Lock
that AppContext must use.
504 pushPopLock.lock();
505 try {
506 AWTEvent event = getNextEventPrivate();
507 if (event != null) {
508 return event;
509 }
510
AWTAutoShutdown.getInstance().notifyThreadFree(dispatchThread);
511 pushPopCond.await();
512 } finally {
513 if(pushPopLock.isHeldByCurrentThread()) {
514 pushPopLock.unlock();
515 }
516 }
You should add a comment explaining why the check for isHeldByCurrentThread
is needed - and that if things are done right at a higher-level we should
never need the stop() to break out of await().
Ditto for line 570
Overall I'm still concerned that there is an issue in the overall design
that permits events to be queued even after a "shutdown" has been logically
initiated. With this patch those events won't get processed and not knowing
what they are I can't say whether this will be a problem or not. It is a
concern that the current code in detachDispatchThread says:
1045 /*
1046 * Don't detach the thread if any events are pending. Not
1047 * sure if it's a possible scenario, though.
1048 *
1049 * Fix for 4648733. Check both the associated java event
1050 * queue and the PostEventQueue.
1051 */
as it seems to indicate that the exact conditions for detachment are
unclear. Based on reading 4648733 I'm assuming that we have to keep the
event queue receiving events so that the shutdown event can be posted (as
part of AWT auto-shutdown), and that then allows other events in. The
question remains as to whether those events should be processed even when
shutdown has been initiated.
David
-----
Thanks, Clemens
