<AWT Dev> Endless loop in EventDispatchThread - proposed solution

[Clemens Eisserer]

Hi Dave,

Thanks again for taking a look at my comments and for your patience :)


It seems to me that when the app context is being shutdown in this manner
> that part of that should be to purge/clear the event queue. To me that would
> be a more appropriate fix for this non-terminating situation.

Could be, I have to admit I am not an EventQueue expert :/


Right. The ThreadDeath gets seen while trying to acquire the Lock to return
> from await() and so the lock is not reacquired and so the subsequent
> unlock() throws IMSE. There's an open bug on this (6627356), against
> Condition.await, but like anything to do with async exceptions there are no
> easy solutions. I'd prefer if AppContext could stop using a method that's
> been deprecated for a decade (and which I'd love to physically remove in
> Java 8).


> BTW the interrupt() should really be causing the await() to complete, but
> there must be contention on the Lock causing the stop() to be issued before
> the lock is acquired. Maybe that contention could be removed to allow the
> interrupt() to do all the work?


There is at least one case when Thread.stop is required to halt the EDT:
If there are events added to the queue, after await() threw the
InterruptedException. In this case the EDT won't terminate and tries to
dispatch the remaining events, which it does until the queue is empty, and
then again it waits in await().
(the isInterrupted()-checks are a bit meaningless, because after an
InterruptedException is thrown in getNextEvent, isInterrupted() returns
false again)
There is no second Thread.interrupt(), so ThreadDeath is thrown when
Thread.stop() is called. (which currently leads to spinning, as
inInterrupted() returns true when Thread.stop has been called and
threadDeathCaught remains false forever)

I experienced this issue only on the highly loaded (and swapping, but not
OOM) server, that would explain the timing issues.



 My proposal for this bug would be to do away with the
> threadDeathCaught-variable completly, as the isInterrupted()-check proposed
> in 1.) will stop the thread anyway.
>

Seems to me that there should be a simple variable that the run() method can
> check which gets set when the thread is expected to terminate. That way it
> doesn't matter whether it gets hit by the interrupt() or the stop(), it will
> terminate based on the flag.
>

Jep, I agree. Additionally, when pending events are dispatched during
shut-down, the queue should not block waiting for new events.
That async exceptions could even be consumed by bad-behaving user-code (e.g.
some Thread.sleep wrapped in a catch-block), a variable seems to be a much
cleaner solution.



> But any changes to this code need deep scrutiny - I know many eyes have
> looked at it over the years. And there's no doubt a fair amount of
> historical baggage as well.
>
I'll prepare two patches:
- A simple one, which just restores pre-java7 behaviour, working arround
that IllegalMonitorStateException
- A more in-deptch change, trying not to rely on AsynExeptions for thread
termination.


Thanks again, Clemens
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/awt-dev/attachments/20110822/1ec8a7c2/attachment.html