Enable ProactorEventLoop on windows for ipykernel#1469
Conversation
|
Hello @NewUserHa, Thanks for your work here! Dear @ianthomas23, Is it possible to get this merged on the next release? It is really important for Windows users, as current solution creates incompatibilities between libraries. Thank you! |
|
@ZupoLlask It is not passing CI, so of course it will not be merged. |
|
@NewUserHa Can you please have a look at this? TY |
|
I have no idea how this occur. this modification works in previous versions of ipython |
37bad2c to
903b031
Compare
|
the |
|
@NewUserHa There were some problems with the CI that I think I have now fixed, so can you rebase this on |
|
PR modified:
current:
|
|
Thank you @NewUserHa! Let's wait and see if @ianthomas23 can help a bit here... 🙏 |
I can, but it will be next week now. |
|
I've taken a look but I am not there is anything I can add here. I notice that most of the eventloops testing is disabled on windows, e.g. ipykernel/tests/test_eventloop.py Lines 86 to 87 in 770afbe so this has evidently been a problematic area in the past. |
|
ipykernel/tests/test_eventloop.py Lines 86 to 87 in 770afbe it should have no problem, since the difference between proactor and selector is add_reader() only mostly.
However, the debugpy can't debug ipyknerel and the test, reporting And the ipykernel's class IOPub starts a thread for tornado's class IOloop, while tornado >6.1 version starts a thread for proactor on windows as well, while pyzmq uses tornado's AsyncIOloop to start a thread also. |
|
@NewUserHa I dug into the Root cause — ipykernel's read side, not debugpyOn a failing run debugpy does hit the breakpoint and write the
(Proven on 3.13 with FixRun the ipykernel service loops (control, IOPub, shell channel, subshells) on a Branch (ready to pull): https://github.com/BoykoNeov/ipykernel/tree/pr-1469 — single commit Result (Windows, local): full One separate, minor thing (not fixed by the above — flagging it)While bisecting I found |
…rect test_attach_debug #1469 batch (this session): - Fix A committed b142cae on pr-1469 (= #1469 head + 1 commit) and pushed to fork; root-cause+fix comment posted on ipython/ipykernel#1469 (offers a PR into patch-2). Standalone PR held: current main is Selector-everywhere, so a PR-to-main is a near-no-op; the #1469 comment is the load-bearing artifact (NewUserHa had asked for exactly this help). - CORRECTION: test_attach_debug was wrongly logged as "NOT loop-related / debugpy-version artifact". Bisected same-box (origin/main Selector PASS vs pr-1469 Proactor FAIL, pure-#1469 too): it IS a second #1469 Proactor regression, but COSMETIC not functional — a stateful repl probe shows namespace/computation work under Proactor; only the first repl evaluate's DAP result field differs (empty Selector / value Proactor). Out of Fix A scope (main stays Proactor). Carried forward (uncommitted from prior batches): - upstream-pr-filed: #1529 stream-send fix + regression test pushed, reply posted, title fixed. - yield-case-depth-inversion-built: minor touch-up. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Thanks for your effort. Your commit changes all The
there is still a test failed, can you help take a look at that |
|
Thanks @NewUserHa — good questions, taking them in turn. 1. You're right that it flows through # tornado/platform/asyncio.py — AsyncIOLoop.initialize
...
if "asyncio_loop" not in kwargs:
kwargs["asyncio_loop"] = loop = asyncio.new_event_loop()
...
super().initialize(**kwargs)i.e. it's designed to let the caller supply the loop and only creates its own when you don't. It's been public API since tornado 6.2:
ipykernel's floor is 2. Reusing We already are — # BaseAsyncIOLoop.initialize
self.selector_loop = asyncio_loop
if hasattr(asyncio, "ProactorEventLoop") and isinstance(
asyncio_loop, asyncio.ProactorEventLoop
):
self.selector_loop = AddThreadSelectorEventLoop(asyncio_loop)That 3. The remaining failed test That's # tests/test_debugger.py
assert reply["body"]["result"] == ""which encodes the Selector timing. Since this PR makes the main loop Proactor, that assertion should move with it — editing it against current |
…on windows Removed the _init_asyncio_patch method and its calls.
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
Thanks. good fix and analysis. Your fix repleaces all service loops (including IOPub/control/iostream/etc.) to The #1532 also relates to this, please take a look with this pr. Please open it as a PR into patch-2 includes covering the last remaining failed test |
When the kernel runs under a ProactorEventLoop on Windows (enabled by ipythongh-1469 so the main loop can spawn asyncio subprocesses, ipythongh-1468), debugging deadlocks on Python >= 3.12. ProactorEventLoop has no native add_reader, so tornado drives a Proactor loop's zmq sockets via a helper "Tornado selector" thread that does select() then call_soon_threadsafe() to wake the loop. ipykernel exempts its own service threads from the debugger but not tornado's helper. When debugpy suspends every thread at a breakpoint under sys.monitoring (3.12+, interpreter-global), that un-exempt helper freezes mid-wake and the control/debug read path never advances -- the loop sits in the IOCP poll forever. On 3.11 (sys.settrace, per-thread) the helper is not frozen, so it does not reproduce there. The ipykernel service loops -- control, IOPub, the shell channel and subshells -- never need Proactor's subprocess support, so run them on a SelectorEventLoop instead: it implements add_reader natively and needs no helper thread. Only the main/user-code loop stays on Proactor. Off Windows the default loop is already selector-based, so this is a no-op there. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Opened it as a PR into On your questions: Does moving Keep as much Proactor as possible? You could narrow it to just the control loop (the one on the debug read path) — but I'd lean against it. The other service loops only carry low-volume zmq traffic with no subprocess I/O, so Proactor buys them nothing measurable, and any loop left on Proactor keeps a freezable helper thread around under the debugger. Uniform selector for the service loops keeps the rule simple ("only the main loop is Proactor") at basically no cost. The last failing test — Heads-up on testing: the debugger suite can't actually hit this deadlock right now — On #1532 — related but separate (dropping the dead |
Run service-thread event loops on a selector loop on Windows (fix debugger deadlock under Proactor)
|
Fixs: #1468 The PR passes all checks now. may you give it a review? @ianthomas23 After the PR, we now would be able to use |
…on both PRs GitHub status re-check 2026-06-21: - NewUserHa/ipykernel#1 (Fix A) MERGED into patch-2 (self-merge 3638154); NewUserHa then reverted dead %asyncio (the #1532 cleanup we'd kept out). - ipython/ipykernel#1469 now 40/40 green; NewUserHa endorsed + pinged ianthomas23 for review. Latest comment is an endorsement, not a question. - ipython/ipykernel#1529 unchanged: awaiting ianthomas23 re-review, green except the known macOS-pypy test_run_concurrently_sequence timeout flake. Nothing owed from us on either PR; both await maintainer review. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
see #1468