Fix StatHat::Reporter to allow #finish to avoid deadlocks waiting for worker threads#5
Open
bodhi wants to merge 1 commit intopatrickxb:masterfrom
Open
Conversation
… worker threads Worker threads would previously block waiting for content to be pushed onto the shared queue. So even though the thread that calls #finish marks the shared @running variable as false, the worker threads will never wake up to notice that they should quit. Avoid this situation by pushing the process control in-band with the queued content, to signal to the worker threads that they should wake up *and* stop processing.
|
@bodhi what's the scenario where this happens? |
|
(your description makes is sound like it always happens) |
Author
|
Honestly, I can't remember. We stopped using StatHat a while ago on our project for a couple of reasons, so I haven't looked at it basically since I submitted the pull request. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Worker threads would previously block waiting for content to be pushed
onto the shared queue. So even though the thread that calls #finish marks the
shared @running variable as false, the worker threads will never wake up to
notice that they should quit, thus causing a deadlock when the calling thread
#joins the worker threads.Avoid this situation by pushing the process control in-band with the queued
content, to signal to the worker threads that they should wake up and stop
processing.
This is a minimal change to allow
#finishto work as expected. What would probably be better is creating the threads on demand as the events are pushed into the API. This would eliminate the need for the queue and process control, at the expense of creating 1 thread per event. I haven't measured the impact of this style on GC or performance (eg. thread startup cost), but I suspect it would be negligible in the operation of a larger project.