This repository has been archived by the owner on Nov 18, 2019. It is now read-only.
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.
So you know how Rust is all good and stuff? Well it doesn't fix all bugs. In this case, it's still possible to deadlock if a thread grabs a mutex's lock and never lets go. In this case, the game thread was meant to release the lock on the nose-goes mutex before sleeping, but it never did. The thread held the mutex lock during its entire sleep, and would only release it briefly before returning to the top of the loop, where it would almost immediately re-acquire the lock. As a result, the thread handling the nose-goes request would never be able to acquire the lock, and would never manage to remove the player from the nose-goes event. This would result in players who successfully tapped the poison pill to still be killed in a nose-goes event.
I've fixed this by adding an explicit scope to the game loop to ensure the mutex guard for the nose-goes state is dropped at the right time. I've also added a note to remind others to be wary of the same issue in the future. If we keep running into this issue, we can further abstract the game loop such that these issues can't happen anymore, but I haven't bothered at this point.