[Fix #5341] addressing deadlock regression in windows dispatcher threads #5421
Conversation
This file contains 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
facebook-github-bot
added
the
cla signed
|
Test pass: I also tested this against our internal configuration, we're no longer seeing the deadlock occur. We have integration tests for this server, test 7 above, and I'll investigate why it wasn't catching this before. |
shahzadlone
reviewed
Feb 6, 2019
include/osquery/dispatcher.h
Outdated
| @@ -28,6 +28,7 @@ class Status; | |||
| class Dispatcher; | |||
|
|
|||
|
|
|||
|
|
|||
muffins
force-pushed
the
win-service-zombie-fix
branch
2 times, most recently
from
1eda4bb
to
0ffb269
Compare
There was a problem hiding this comment.
@muffins has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
muffins
force-pushed
the
win-service-zombie-fix
branch
from
0ffb269
to
6ac39d0
Compare
|
@muffins has updated the pull request. Re-import the pull request |
muffins
force-pushed
the
win-service-zombie-fix
branch
from
6ac39d0
to
76ebd55
Compare
|
@muffins has updated the pull request. Re-import the pull request |
There was a problem hiding this comment.
@muffins has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
muffins
added a commit
to muffins/osquery
that referenced
this pull request
Apr 1, 2019
…#5421) Summary: This addresses a slight regression to ensure that we set `set_terminate_threads` on Windows. Without this flag being set, Windows threads will deadlock on exit as the boost managed io service threads never receive termination notifications. I'm opening this PR up against the old master as I feel we should likely cut a 3.3.3, and I'm happy to re-open this PR against the upstream experimental as well, but we'll want a fix for this released as quickly as possible to Windows deployments. Pull Request resolved: osquery#5421 Reviewed By: marekcirkos Differential Revision: D13972916 Pulled By: muffins fbshipit-source-id: 55e3b23c80091d5fb51a97d1efc043b52dc48ba3
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This addresses a slight regression to ensure that we set
set_terminate_threadson Windows. Without this flag being set, Windows threads will deadlock on exit as the boost managed io service threads never receive termination notifications.I'm opening this PR up against the old master as I feel we should likely cut a 3.3.3, and I'm happy to re-open this PR against the upstream experimental as well, but we'll want a fix for this released as quickly as possible to Windows deployments.