Fixes various problems revealed by Windows CI runs#702
Merged
SylvainCorlay merged 23 commits intojupyter-xeus:mainfrom Mar 27, 2026
Merged
Fixes various problems revealed by Windows CI runs#702SylvainCorlay merged 23 commits intojupyter-xeus:mainfrom
SylvainCorlay merged 23 commits intojupyter-xeus:mainfrom
Conversation
This was referenced Mar 26, 2026
SylvainCorlay
requested changes
Mar 26, 2026
…on and report instead
| { | ||
| // release/destroy the debugger python object while GIL is acquired | ||
| pybind11::gil_scoped_acquire gil_lock; | ||
| auto local_debug = std::move(m_pydebugger); |
Member
There was a problem hiding this comment.
Maybe use the macro for acquiring the GIL and calling decref on the object?
Contributor
Author
There was a problem hiding this comment.
I looked for such macro but couldnt find it in pybind11's doc, if you have the name at hand I can try it.
Meanwhile I tried to use ref-decrement instead of moving-locally-and-destroy as it does seem to make more sense. It seems to work as expected locally at least.
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
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.
This PR fixes various issues which are either specific to Windows (python path issue) or timing issues randomly revealed by Windows tests runs:
xpythonprocess startup timing, more pronounced on Windows, lead to random tests failing because of zmq's heartbeat check failing and throwing an exception, leading to the test executable crash;std::systemwas used in tests to launch thexpythonproject and using&to run it in parallel and continue, but this only works with bash and on Windows the&is not interpreted the same way by cmd, resulting to tests waiting forxpythonto complete on Windows and ultimately leading to timeouts;the python executable path deduction algorithm was incorrect when I tried locally on Windows (it was looking for the exe in thethis is not considered here as the issue is actually the user not specifying or wrongly specifyingmyenv/Library/directory instead ofmyenv/) - note however that I couldnt reproduce that specific problem on the CI which uses explicit env variable to specify this path, it's only visible when there is no such specificationPYTHONHOME;xpythonrunning without me noticing while re-running the test, which lead to other errors.Issues 2, 3, 5 are fixed in this PR by using
boost.process(only as a dependency of the test executable) toxpythonsubprocess in a cross-platform way and in parallel,xpython's I/O to wait for it to be ready after launch before proceeding to any testing,Issue 5 is also fixed in this PR by tracking the stilll-running
xpythonsub-processes until aftermain()is finished and if some are still running (usually because of errors inxpython) will terminate the remaining sub-process. This works with aborts (resulting from uncaught exceptions or not) but not with quick-exits.Note that only 4 is specific to Windows, the others could theorically happen on other platforms (speed/timing and UB differences) and the fixes reflect that.
Most of the changes here were first experimented and confirmed working through #685, #686, and experimented with the incoming async changes in #697.