Hi, Clemens,
On 8/26/2011 4:51 PM, Artem Ananiev wrote:
Hi, David, Clemens,
On 8/26/2011 2:02 AM, David Holmes wrote:
On 26/08/2011 2:39 AM, Clemens Eisserer wrote:
...
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:
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.
If the event dispatch thread is interrupted, it's unlikely that AWT
auto-shutdown event is in the queue and should be handled, but even if
it is, I don't see any problem here: the thread will terminate
gracefully anyway.
I am no AWT expert, but from how I interpret the old code, as soon as
interrupt() has been called, it was not intended to dispatch further
events
(I don't think the isInterrupted() call was really ment that way).
I'm considered an AWT expert, but the code was introduced before I
joined the AWT team :) Oleg (in CC) may shed some light here, but
honestly I doubt the event dispatching and auto-shutdown code was
written with a clear understanding how we should react to interrupt()
calls.
If we consider this change for JDK8, which is in the very beginning of
its development, I would vote for discarding all the enqueued events
when interrupt() or stop() is called.
we had a short offline discussion with Oleg, and we both agree there is
no need to clear the event queue explicitly. If the interrupt() call
comes from AppContext.dispose(), a new EDT will not be initialized
(because of the check for AppContext.isDisposed() in initDispatchThread()).
So your "full" version of the fix looks fine. As a sanity check, please
run all the tests from test/java/awt/EQ and test/java/awt/EDT with your
changes, it shouldn't take much time.
Thanks,
Artem
However, I wonder why AppContext.stopEventDispatchThreads() is never
used in
AppContext.dispose(), as it seems to provide a cleaner way for shutdown?
Yes, it sounds like a good idea. However, we can't rely upon this this
mechanism only: the thread may be busy running user code, and the
shutdown event may be not handled, so threadGroup.stop() should still be
called from AppContext.dispose().
I'm no AWT expert either, it just concerns me when there is an
introduced change in behaviour without a full understanding of the
implications. We really need some input from an AWT event processing
guru.
Cheers,
David
Thanks, Clemens
Thanks,
Artem
