Abstract signal handler changes in hooks #49
Merged
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
minrk
added a commit
that referenced
this pull request
Sep 4, 2015
Abstract signal handler changes in hooks
dfalbel
added a commit
to posit-dev/positron
that referenced
this pull request
Jan 7, 2026
<!-- Thank you for submitting a pull request. If this is your first pull request you can find information about contributing here: * https://github.com/posit-dev/positron/blob/main/CONTRIBUTING.md We recommend synchronizing your branch with the latest changes in the main branch by either pulling or rebasing. --> <!-- Describe briefly what problem this pull request resolves, or what new feature it introduces. Include screenshots of any new or altered UI. Link to any GitHub issues but avoid "magic" keywords that will automatically close the issue. If there are any details about your approach that are unintuitive or you want to draw attention to, please describe them here. --> This PR overrides the default `post_handler_hook` and `pre_handler_hook` in the IPyKernel instance, so that exceptions from executing it are not raised with `exc_info=True`. Those hooks were added in ipython/ipykernel#49 to allow customizing the signal behaviors. Later, they started being executed in a try context and exceptions: ipython/ipykernel#360 Their code hasn't changed in the last 10 years, so it seems safe to override them like this. Addresses #10953 The problem we are trying to solve described in #10953 is that in Reticulate sessions on Linux, some execute requests hang, causing the kernel to be completely unresponsive. I'm not entirely sure why this fix works, but here's the hypothesis: - The kernel receives an execute_request. - The shell thread starts processing it and runs the `pre_handler_hook` which fails in reticulate (because you can't signal from the main thread) - This causes a log message to be written with `self.log.debug("Unable to signal in pre_handler_hook:", exc_info=True)` - Log messages are written to an `OutStream`, which will eventually [`flush`](https://github.com/ipython/ipykernel/blob/327589f5e0ed3759751daebb724b8f7d1a0e0c52/ipykernel/iostream.py#L587-L610). - Flushing needs to re-schedule to the IO thread and is a locking operation with a 10s timeout. This can cause hangs eg, we delay responding to rpc status messages, etc. The hypothesis is that larger tracebacks, makes the IO thread queue larger, which makes more likely that flushing times out. ### Release Notes <!-- Optionally, replace `N/A` with text to be included in the next release notes. The `N/A` bullets are ignored. If you refer to one or more Positron issues, these issues are used to collect information about the feature or bugfix, such as the relevant language pack as determined by Github labels of type `lang: `. The note will automatically be tagged with the language. These notes are typically filled by the Positron team. If you are an external contributor, you may ignore this section. --> #### New Features - N/A #### Bug Fixes - N/A ### QA Notes <!-- Positron team members: please add relevant e2e test tags, so the tests can be run when you open this pull request. - Instructions: https://github.com/posit-dev/positron/blob/main/test/e2e/README.md#pull-requests-and-test-tags - Available tags: https://github.com/posit-dev/positron/blob/main/test/e2e/infra/test-runner/test-tags.ts --> <!-- Add additional information for QA on how to validate the change, paying special attention to the level of risk, adjacent areas that could be affected by the change, and any important contextual information not present in the linked issues. --> @:reticulate @:web
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.
The kernel should not assume that we want the default
SIGINTbehaviour. SageMath for example uses a customSIGINThandler and it should be possible to use that handler within Jupyter too.This is done by abstracting away the signal handler changes in new hook methods of the
Kernelwhich can be changed if needed. These hook methods could also be useful for other purposes.