Hi Clemens,
On 21/08/2011 9:25 PM, Clemens Eisserer wrote:
On our caciocavallo-web demo server I sometimes experience endless spinning
in an EventDispatchThread (in run()) which should have been shut down long
ago by AppContext.dispose() - causing our server to consume 100% cpu.
I tried to anylze the situation using a remote debugger, this is what I've
found:
The thread is interrupted, therefor no events are dispatched anymore by
pumpEventsForFilter:
while (doDispatch && cond.evaluate()) {
if (isInterrupted() || !pumpOneEventForFilters(id)) {
doDispatch = false;
}
}
however, detaching the thread from the EventQueue fails, because there are
still events left for dispatching,
which in turn won't be dispatched, because the thread is interrupted.
1.) A fix would be to check for isInterruped also in the thread's run loop:
public void run() {
while (true) {
try {
pumpEvents(new Conditional() {
public boolean evaluate() {
return true;
}
});
} finally {
EventQueue eq = getEventQueue();
if (eq.detachDispatchThread(this) || threadDeathCaught
|| isInterrupted()) {
break;
}
}
}
}
What do you think about the proposed solutions?
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.
2.) A big mystery for me was, why the thread hasn't been killed by
Thread.stop(), called by AppContext.dispose() - and why the
"threadDeathCaught"-variable was still false (as this would have caused the
thread to exit anyway).
The reason was, ThreadDeath is swollowed by the IllegalMonitorStateException
caused by Thread.stop():
Basically this is what is going on, when waiting for new events:
pushPopLock.lock();
try {
cond.await(); //Throws ThreadDeath
}finally {
pushPopLock.unlock(); //Throws IllegalMonitorStateException,
"swollows" ThreadDeath
}
So ThreadDeath is never catched.
This bug has been introduced when converting to java.util.concurrent, as
java monitors don't throw an exception in this situation.
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?
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.
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.
David
Thanks, Clemens
