Skip to content

Reticulate: Recover tests#11232

Merged
dfalbel merged 2 commits into
mainfrom
reticulate-recover
Jan 7, 2026
Merged

Reticulate: Recover tests#11232
dfalbel merged 2 commits into
mainfrom
reticulate-recover

Conversation

@dfalbel

@dfalbel dfalbel commented Jan 5, 2026

Copy link
Copy Markdown
Contributor

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.
  • 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

New Features

  • N/A

Bug Fixes

  • N/A

QA Notes

@:reticulate @:web

@github-actions

github-actions Bot commented Jan 5, 2026

Copy link
Copy Markdown

E2E Tests 🚀
This PR will run tests tagged with: @:critical @:reticulate @:web

readme  valid tags

@dfalbel dfalbel force-pushed the reticulate-recover branch from 90b97e1 to 49b583f Compare January 6, 2026 12:31
@dfalbel dfalbel requested a review from seeM January 6, 2026 14:55
@dfalbel dfalbel marked this pull request as ready for review January 6, 2026 14:55

@seeM seeM left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice fix, looks like it was a tough one!

@dfalbel dfalbel requested a review from midleman January 7, 2026 12:23
@midleman

midleman commented Jan 7, 2026

Copy link
Copy Markdown
Contributor

Oh wow! Nice to see this PR. If you don't mind I'm going to add the @:web tag as well and kick off again. Just to be doubly sure...

@midleman midleman left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome! Will keep an eye on how these tests perform over time since they used to give us so much trouble.

@dfalbel dfalbel merged commit 1729d7d into main Jan 7, 2026
41 checks passed
@dfalbel dfalbel deleted the reticulate-recover branch January 7, 2026 17:08
@github-actions github-actions Bot locked and limited conversation to collaborators Jan 7, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants