<AWT Dev> Endless loop in EventDispatchThread - proposed solution

[David Holmes]

On 22/08/2011 8:17 PM, Clemens Eisserer wrote:
> Thanks again for taking a look at my comments and for your patience :)

Threads are my core interest :)

>
>     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)

Ah I see. So this would again be solved by purging the event queue. I'm no 
expert on the event system per se so it may be that events can continue to 
be generated after the decision to dispose of the app context has been made 
- otherwise I would expect no further events and so clearing the queue would 
allow the interrupt (plus variable setting) to act as the termination 
indicator for the EDT.

David

> 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