Add threading condition back to events daemon

[danlamanna]

Partially revert "97bdf57886d0a847c"

This fixes an issue where the event daemon was never actually being
terminated. Coverage shows while self.terminate gets set to True, the
log message printing the stopped message never actually gets run. This
is due to a bug where we indefinitely block for another item off the
queue in the middle of our 'while not terminate' clause.

Looking at this now, I suppose the daemon would actually get terminated but it would require a single event get processed after stop was called.

Random architectural thoughts about the events daemon:

• Maybe the daemon should /have/ a thread and not /be/ a thread
  Since python threads can't be "restarted" but an events daemon could and arguably should.
• No other thread is joining this thread, so I think it may be hanging around until CherryPy stops
  This isn't a big deal in the case of running Girder under CherryPy, but if the events daemon were to be used elsewhere it might be nice to enclose the logic (or maybe not).

@zachmullen You originally wrote the threading condition logic I want to restore, and I went through a bit of weeds to find this bug so I may have gotten a thing or two wrong, mind taking a look?

[zachmullen]

This looks like a good fix (once I read through the docs and discovered that the default lock for Condition can be acquired multiple times on the same thread).

Was this the cause of all those random error messages during testing? Or did the underlying problem present in some other way?

[danlamanna]

Was this the cause of all those random error messages during testing? Or did the underlying problem present in some other way?

I'm not sure what error messages you're talking about, do you have a link? I ran into this by trying to get pytest to serially run our tests in a single process. CherryPy doesn't remove subscribed events from the bus on shutdown, which was causing the start listener to try to revive the event daemon. I fixed this locally with hacky scoping on our 'stop' event and discovered this in the process. I plan to submit a bug report upstream to CherryPy.

…

On Wed, Oct 4, 2017 at 21:48 PM, Zach Mullen ***@***.***> wrote: This looks like a good fix (once I read through the docs and discovered that the [default lock](https://docs.python.org/2/library/threading.html#threading.RLock) for `Condition` can be acquired multiple times on the same thread). Was this the cause of all those random error messages during testing? Or did the underlying problem present in some other way? -- You are receiving this because you authored the thread. Reply to this email directly or view it on GitHub: #2379 (comment)

[jbeezley]

I think he's talking about this #642. It's before your time :P.

[zachmullen]

@jbeezley that's a bingo!