When SendContinuePacketAndWaitForResponse returns eStateInvalid, don'… #2927
Conversation
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
…t fetch more packets. This looks like just an oversight in the AsyncThread function. It gets a result of eStateInvalid, and then marks the process as exited, but doesn't set "done" to true, so we go to fetch another event. That is not safe, since you don't know when that extra packet is going to arrive. If it arrives while you are tearing down the process, the internal-state-thread might try to handle it when the process in not in a good state. Rather than put more effort into checking all the shutdown paths to make sure this extra packet doesn't cause problems, just don't fetch it. We weren't going to do anything useful with it anyway. The main part of the patch is setting "done = true" when we get the eStateInvalid. I also added a check at the beginning of the while(done) loop to prevent another error from getting us to fetch packets for an exited process. I added a test case to ensure that if an Interrupt fails, we call the process exited. I can't test exactly the error I'm fixing, there's no good way to know that the stop reply for the failed interrupt wasn't fetched. But at least this asserts that the overall behavior is correct. Differential Revision: https://reviews.llvm.org/D101933 (cherry picked from commit 72ba78c)
|
@swift-ci please test |
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.
…t fetch more packets.
This looks like just an oversight in the AsyncThread function. It gets a result of
eStateInvalid, and then marks the process as exited, but doesn't set "done" to true,
so we go to fetch another event. That is not safe, since you don't know when that
extra packet is going to arrive. If it arrives while you are tearing down the
process, the internal-state-thread might try to handle it when the process in not
in a good state.
Rather than put more effort into checking all the shutdown paths to make sure this
extra packet doesn't cause problems, just don't fetch it. We weren't going to do
anything useful with it anyway.
The main part of the patch is setting "done = true" when we get the eStateInvalid.
I also added a check at the beginning of the while(done) loop to prevent another error
from getting us to fetch packets for an exited process.
I added a test case to ensure that if an Interrupt fails, we call the process
exited. I can't test exactly the error I'm fixing, there's no good way to know
that the stop reply for the failed interrupt wasn't fetched. But at least this
asserts that the overall behavior is correct.
Differential Revision: https://reviews.llvm.org/D101933
(cherry picked from commit 72ba78c)