Update python_repl to use ignoring_sigint to... ignore SIGINT #7890
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
blorente
requested review from
stuhood,
illicitonion,
ity,
cosmicexplorer and
Eric-Arellano
illicitonion
approved these changes
Jun 18, 2019
Eric-Arellano
approved these changes
Jun 18, 2019
ity
approved these changes
Jun 18, 2019
blorente
force-pushed
the
blorente/ignore-sigint-pythonrepl-under-pantsd
branch
from
7a9357e
to
fcf89a5
Compare
blorente
changed the title
blorente
added a commit
to blorente/pants
that referenced
this pull request
Jun 21, 2019
## Problem After removing forking from pantsd, we introduced a new abstraction to ExceptionSink that allowed ignoring SIGINT without installing signal handlers (pantsbuild#7623). This was necessary because no pants runs under pantsd run in a non-main thread, and therefore crash when you install signal handlers. This PR makes use of that new functionality, to ignore SIGINT in python_repls ## Solution Instead of only ignoring sigint in the cases where we are not running from the daemon, ignore it every time using the new ExceptionSink.ignoring_sigint() contextmanager. ## Result Using Ctrl-C on a python repl is properly ignored and no longer crashes the daemon.
blorente
added a commit
that referenced
this pull request
Sep 4, 2019
**Problem** When a pants run under pantsd calls pants, it will also enable pantsd in the inner run. Since we don't allow parallel runs under pantsd, the inner run will block waiting for the outer run to complete, eventually timing out and crashing the whole run. See #7881 for context. The only real instance we've seen of this was entirely accidental. **Solution** Explicitly disable pantsd through an environment variable whenever we are serving a pailgun request. **Result** Inner runs under pantsd have the daemon turned off by default. Fixes #7881 , Depends on #7890 Commits should be independently reviewable.
blorente
added a commit
that referenced
this pull request
Sep 4, 2019
**Problem** When a pants run under pantsd calls pants, it will also enable pantsd in the inner run. Since we don't allow parallel runs under pantsd, the inner run will block waiting for the outer run to complete, eventually timing out and crashing the whole run. See #7881 for context. The only real instance we've seen of this was entirely accidental. **Solution** Explicitly disable pantsd through an environment variable whenever we are serving a pailgun request. **Result** Inner runs under pantsd have the daemon turned off by default. Fixes #7881 , Depends on #7890 Commits should be independently reviewable.
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.
Problem
After removing forking from pantsd, we introduced a new abstraction to
ExceptionSinkthat allowed ignoringSIGINTwithout installing signal handlers (#7623).This was necessary because no pants runs under pantsd run in a non-main thread, and therefore crash when you install signal handlers.
This PR makes use of that new functionality, to ignore
SIGINTinpython_replsSolution
Instead of only ignoring sigint in the cases where we are not running from the daemon, ignore it every time using the new
ExceptionSink.ignoring_sigint()contextmanager.Result
Using Ctrl-C on a python repl is properly ignored and no longer crashes the daemon.