BrightcoveOS/Diamond — pull request #350 by lambdafu (2013-04-20 14:47:29+00:00)
Title: Fix race condition where diamond hangs at shutdown.

I run diamond from circusd with a default interval of 10 seconds and found that sometimes circusd spins on waiting for diamond to shut down after sending a TERM signal. After some digging, I found that there is a race condition in the scheduler when using such short intervals, because the __delay function does not check the running flag for intervals shorter than 10 seconds.

Under normal conditions, the scheduler queue is emptied in the diamond.scheduler.Scheduler.stop() function, and this is caught in sched.scheduler.run(). However, if the stop() function happens to run while an interval task is executing in the other thread, the queue is emptied first and then the interval task re-adds itself to the queue. This will then cause the scheduler to continue to run. For larger intervals, the running flag is checked in __delay function eventually, but for intervals less than 10 seconds, this is not done, so the scheduler runs indefinitely, and the main thread hangs in the join() call in ThreadScheduler.stop().

My patch makes sure that the running flag is checked on all paths through the __delay function. Instead of using an empty queue to indicate to the sched.scheduler.run() function that we want to end the scheduler, I use a dedicated exception. Furthermore, to preserve the existing stop() behavior, I empty the scheduler queue after making sure that no interval task can run anymore (when receiving the exception in _run()).

This fixes the problem for me with standard python that uses a GIL. I think there are several more issues with the code (for example, the running flag is not set in the start() function, the running flag is not lock-protected, the logic when to use locks and when not should not depend on the running flag, etc), but these are probably not encountered in actual use for now. My patch is conservative in that it only fixes what needs fixing for the bug I am actually seeing.

BTW, from review, the same bug seems to exist in the tgscheduler kronos.