Merged
Conversation
It was originally used to communicate the death of a single thread, but we now only use it when the whole process has died.
We inject a SHD_SHIM_EVENT_PROCESS_DEATH event into each individual managed thread when we detect the process has died. In shadow#2674, this ended up causing the memory write inside Process::reap_thread to fail. Since the process has died, we don't need to do that write at all; marking the process as dead correctly skips this step. I tried to write a regression test but was unable to reproduce the issue. As in the mongodb case, I set it up so that a child thread called `abort` while the thread group leader was blocked. In both cases, we end up trying to use the thread group leader to do the write, but it is in a zombie state since we already natively raised SIGABRT, which kills the whole process. In my test that write *works*, even though the thread group leader is in a zombie state. I did verify that this change fixes the problem in the mongodb example, though.
Codecov ReportBase: 68.15% // Head: 68.17% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #2680 +/- ##
==========================================
+ Coverage 68.15% 68.17% +0.01%
==========================================
Files 202 202
Lines 30326 30326
Branches 5923 5923
==========================================
+ Hits 20668 20674 +6
+ Misses 4975 4972 -3
+ Partials 4683 4680 -3
Flags with carried forward coverage won't be shown. Click here to find out more.
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
45d5086 to
c0f5e1b
Compare
stevenengler
approved these changes
Jan 20, 2023
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.
We inject a SHD_SHIM_EVENT_PROCESS_DEATH event into each individual
managed thread when we detect the process has died.
In #2674, this ended up causing
the memory write inside Process::reap_thread to fail. Since the process
has died, we don't need to do that write at all; marking the process as
dead correctly skips this step.
I tried to write a regression test but was unable to reproduce the
issue. As in the mongodb case, I set it up so that a child thread
called
abortwhile the thread group leader was blocked. In both cases,we end up trying to use the thread group leader to do the write, but it
is in a zombie state since we already natively raised SIGABRT, which
kills the whole process. In my test that write works, even though the
thread group leader is in a zombie state.
I did verify that this change fixes the problem in the mongodb example,
though.