fix(python): leftover jsii-kernel-* directories in TMPDIR #2100
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
The Python runtime did not properly signal shutdown to the `node` process hosting the *jsii kernel*, causing it to be abruptly killed on Python interpreter exit. This means the *jsii kernel*'s own clean-up code was not allowed to run. This adds an `atexit` hook which will properly close the `node` process' `STDIN` file descriptor, effectively signaling it should shut down. This then gives the process 5 seconds to clean up after itself and finish before sending it `SIGTERM` (at which point it is given 5 seconds to terminate after which it will be terminated).
RomainMuller
added
bug
|
@all-contributors add @edsenabr for bug since he's the one who mentioned this one to me! |
|
I've put up a pull request to add @edsenabr! 🎉 |
nija-at
approved these changes
Oct 13, 2020
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
RomainMuller
added a commit
that referenced
this pull request
Nov 12, 2020
This change adds shutdown hook registrations in the .NET and Java runtimes so that the `node` process hosting the JSII runtime is gracefully terminated upon program end. This allows any "on exit" hooks registered in the JavaScript process to properly run when the host program ends. This is similar to what was done in #2100 for Python. Additionally, removed the code that surrounded handling of the `STDERR` stream from the Kernel process: there is no need to `PIPE` it, and simply inheriting it (i.e: subprocess `STDERR` is the same stream as the parent's) results in a more intuitive behavior, as `STDERR` output from child processes surfaces "immediately" even if the parent process somehow cannot forward it anymore (e.g: if it crashed or deadlocked). This also means the `STDERR` forwarding thread is no longer needed.
mergify bot
pushed a commit
that referenced
this pull request
Nov 16, 2020
This change adds shutdown hook registrations in the .NET and Java runtimes so that the `node` process hosting the JSII runtime is gracefully terminated upon program end. This allows any "on exit" hooks registered in the JavaScript process to properly run when the host program ends. This is similar to what was done in #2100 for Python. --- By submitting this pull request, I confirm that my contribution is made under the terms of the [Apache 2.0 license]. [Apache 2.0 license]: https://www.apache.org/licenses/LICENSE-2.0
1 task
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.
The Python runtime did not properly signal shutdown to the
nodeprocess hosting the jsii kernel, causing it to be abruptly killed on
Python interpreter exit. This means the jsii kernel's own clean-up
code was not allowed to run.
This adds an
atexithook which will properly close thenodeprocess'STDINfile descriptor, effectively signaling it should shut down. Thisthen gives the process 5 seconds to clean up after itself and finish
before sending it
SIGTERM(at which point it is given 5 seconds toterminate after which it will be terminated).
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.