Repository navigation
fix(python): don't crash when the interpreter exits during a result delivery - #607
Conversation
…elivery pyo3-async-runtimes delivers results from a tokio thread via `call_soon_threadsafe`, which releases the GIL in `socket.send`. If the awaiting coroutine was the last thing the program did, the interpreter can start finalizing before that thread gets the GIL back, and CPython < 3.13.8 kills it with `pthread_exit`. The forced unwind through Rust frames aborts the process. Deliveries now go through a custom pyo3-async-runtimes runtime that counts them. An atexit hook (which runs before the interpreter is marked as finalizing) stops new deliveries and waits for the in-flight ones to leave Python. Closes #606
…pit's The exit gate stayed closed after impit's atexit hook, so results of calls made from hooks registered before `import impit` were dropped and never arrived. Creating a future now reopens the gate, and the hook is only registered on Python < 3.13.8, which is the only range affected by the crash.
apify-service-account
added
the
tested
apify-service-account
added
the
t-tooling
Pijukatel
left a comment
Pijukatel
left a comment
There was a problem hiding this comment.
Just comments about the tests.
Is this tested on Python version where this can actually happen(< 3.13.8)? Could we run the tests (at least for Linux build) for both lower and upper bound Python version?
…tests The exit tests only exercised the gate on Python < 3.13.8, which CI doesn't run, and wouldn't catch a new call site that skips the gate. A clippy ban on the ungated `future_into_py` does, on every Python version.
|
Thank you for the review @Pijukatel , you're right that this is rather internal behaviour and it's overall tricky to test this explicitly. I thought this through again and dropped the tests completely. There's a new |
vdusek
left a comment
vdusek
left a comment
There was a problem hiding this comment.
Two low-severity findings from staff-review. Probably only trade-off notes, and an idea for a possible follow-up. Up to you whether/and how to address them. Otherwise LGTM.
| { | ||
| // Python code is still running, so the interpreter isn't finalizing yet. This reopens the gate | ||
| // for atexit hooks that run after ours, whose results would never arrive otherwise. | ||
| STATE.lock().unwrap().closed = false; |
There was a problem hiding this comment.
Note: After this reopen, nothing closes the gate again. If an atexit hook that runs after ours awaits an impit call as its last step, the #606 race can come back. A delivery for a future dropped earlier can also land in that window. The comment covers the trade-off, and atexit has no later drain point, so I think this is acceptable. Flagging it so the limit is known.
✍️ Drafted by Claude Code
| } | ||
| }; | ||
| match chunk_result { | ||
| Some(Ok(chunk)) => Ok(Python::attach(|py| PyBytes::new(py, &chunk).unbind())), |
There was a problem hiding this comment.
Note: This Python::attach (and the ones in aread below) runs inside the future body on a tokio thread, so the exit gate doesn't cover it. A future still pending at finalization, for example an abandoned task on a loop stopped without cancellation, can still hit pthread_exit. It's out of scope for a result-delivery fix, but it may be worth a follow-up issue.
✍️ Drafted by Claude Code
Waits for in-flight async results in an atexit hook so tokio threads don't get killed by interpreter finalization on Python < 3.13.8 (closes #606).