<AWT Dev> Endless loop in EventDispatchThread - proposed solution

[Artem Ananiev]

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