Handle case where output buffer is closed during shutdown #365
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
- Prevent crash during launch shutdown when a process IO event happens
after the buffers have been closed
- Use unbuffered output in that case so IO still has a chance of being seen
Signed-off-by: Pete Baughman <[email protected]>
|
@ivanpauno Some code duplication, but fixes the crash on shutdown in the nightly docker image |
|
@wjwwood fyi and ask if we can get this in a little faster. |
hidmic
reviewed
Dec 16, 2019
| self.__stdout_buffer.write(last_line) | ||
| except ValueError: | ||
| # __stdout_buffer was probably closed by __flush_buffers on shutdown. Output without | ||
| # buffering. |
There was a problem hiding this comment.
@pbaughman why not checking if self.__stdout_buffer.closed instead of hoping ValueError was raised for the reasons you expect?
| self.__stderr_buffer.write(last_line) | ||
| except ValueError: | ||
| # __stderr buffer was probably closed by __flush_buffers on shutdown. Output without | ||
| # buffering. |
There was a problem hiding this comment.
@hidmic Yeah, I was coming at this from the approach of "this is a race condition" but this method is synchronized with the one that closes the buffers, right? They're not going to get closed out from under us while the method is running?
There was a problem hiding this comment.
It's asyncio, so no, that won't happen. There're no guarantees regarding execution order, and that's what probably caused this issue in the first place.
Signed-off-by: Pete Baughman <[email protected]>
|
@hidmic I've addressed your comments |
|
CI (up to
|
|
@hidmic The windows failure looks like a problem with building, not testing. |
|
Alright, going in! |
ivanpauno
pushed a commit
that referenced
this pull request
Jan 17, 2020
* Handle case where output buffer is closed during shutdown
- Prevent crash during launch shutdown when a process IO event happens
after the buffers have been closed
- Use unbuffered output in that case so IO still has a chance of being seen
Signed-off-by: Pete Baughman <[email protected]>
* Address MR feedback
Signed-off-by: Pete Baughman <[email protected]>
ivanpauno
pushed a commit
that referenced
this pull request
Jan 20, 2020
* Handle case where output buffer is closed during shutdown
- Prevent crash during launch shutdown when a process IO event happens
after the buffers have been closed
- Use unbuffered output in that case so IO still has a chance of being seen
Signed-off-by: Pete Baughman <[email protected]>
* Address MR feedback
Signed-off-by: Pete Baughman <[email protected]>
Signed-off-by: Ivan Santiago Paunovic <[email protected]>
ivanpauno
added a commit
that referenced
this pull request
Jan 21, 2020
* Handle case where output buffer is closed during shutdown (#365) * Handle case where output buffer is closed during shutdown - Prevent crash during launch shutdown when a process IO event happens after the buffers have been closed - Use unbuffered output in that case so IO still has a chance of being seen Signed-off-by: Pete Baughman <[email protected]> * Address MR feedback Signed-off-by: Pete Baughman <[email protected]> Signed-off-by: Ivan Santiago Paunovic <[email protected]> * Import test file without contaminating sys.modules (#360) Signed-off-by: Pete Baughman <[email protected]> Signed-off-by: Ivan Santiago Paunovic <[email protected]> * Release loop lock before waiting for it to do work (#369) Main thread can be blocked trying to acquire __loop_from_run_thread_lock while emit_event() in another thread is holding that lock and waiting for the main thread to emit the event. This change releases the lock before blocking. Signed-off-by: Shane Loretz <[email protected]> Signed-off-by: Ivan Santiago Paunovic <[email protected]> Co-authored-by: Peter Baughman <[email protected]> Co-authored-by: Shane Loretz <[email protected]>
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.
Prevent crash during launch shutdown when a process IO event happens
after the buffers have been closed
Use unbuffered output in that case so IO still has a chance of being seen
Fixes #364
Signed-off-by: Pete Baughman [email protected]