|
| 1 | +# Revisit `subint_forkserver` thread-cache constraints once msgspec PEP 684 support lands |
| 2 | + |
| 3 | +Follow-up tracker for cleanup work gated on the msgspec |
| 4 | +PEP 684 adoption upstream ([jcrist/msgspec#563](https://github.com/jcrist/msgspec/issues/563)). |
| 5 | + |
| 6 | +Context — why this exists |
| 7 | +------------------------- |
| 8 | + |
| 9 | +The `tractor.spawn._subint_forkserver` submodule currently |
| 10 | +carries two "non-trio" thread-hygiene constraints whose |
| 11 | +necessity is tangled with issues that *should* dissolve |
| 12 | +under PEP 684 isolated-mode subinterpreters: |
| 13 | + |
| 14 | +1. `fork_from_worker_thread()` / `run_subint_in_worker_thread()` |
| 15 | + internally allocate a **dedicated `threading.Thread`** |
| 16 | + rather than using `trio.to_thread.run_sync()`. |
| 17 | +2. The test helper is named |
| 18 | + `run_fork_in_non_trio_thread()` — the |
| 19 | + `non_trio` qualifier is load-bearing today. |
| 20 | + |
| 21 | +This doc catalogs *why* those constraints exist, which of |
| 22 | +them isolated-mode would fix, and what the |
| 23 | +audit-and-cleanup path looks like once msgspec #563 is |
| 24 | +resolved. |
| 25 | + |
| 26 | +The three reasons the constraints exist |
| 27 | +--------------------------------------- |
| 28 | + |
| 29 | +### 1. GIL-starvation class → fixed by PEP 684 isolated mode |
| 30 | + |
| 31 | +The class-A hang documented in |
| 32 | +`subint_sigint_starvation_issue.md` is entirely about |
| 33 | +legacy-config subints **sharing the main GIL**. Once |
| 34 | +msgspec #563 lands and tractor flips |
| 35 | +`tractor.spawn._subint` to |
| 36 | +`concurrent.interpreters.create()` (isolated config), each |
| 37 | +subint gets its own GIL. Abandoned subint threads can't |
| 38 | +contend for main's GIL → can't starve the main trio loop |
| 39 | +→ signal-wakeup-pipe drains normally → no SIGINT-drop. |
| 40 | + |
| 41 | +This class of hazard **dissolves entirely**. The |
| 42 | +non-trio-thread requirement for *this reason* disappears. |
| 43 | + |
| 44 | +### 2. Destroy race / tstate-recycling → orthogonal; unclear |
| 45 | + |
| 46 | +The `subint_proc` dedicated-thread fix (commit `26fb8206`) |
| 47 | +addressed a different issue: `_interpreters.destroy(interp_id)` |
| 48 | +was blocking on a trio-cache worker that had run an |
| 49 | +earlier `interp.exec()` for that subint. Working |
| 50 | +hypothesis at the time was "the cached thread retains the |
| 51 | +subint's tstate". |
| 52 | + |
| 53 | +But tstate-handling is **not specific to GIL mode** — |
| 54 | +`_PyXI_Enter` / `_PyXI_Exit` (the C-level machinery both |
| 55 | +configs use to enter/leave a subint from a thread) should |
| 56 | +restore the caller's tstate regardless of GIL config. So |
| 57 | +isolated mode **doesn't obviously fix this**. It might be: |
| 58 | + |
| 59 | +- A py3.13 bug fixed in later versions — we saw the race |
| 60 | + first on 3.13 and never re-tested on 3.14 after moving |
| 61 | + to dedicated threads. |
| 62 | +- A genuine CPython quirk around cached threads that |
| 63 | + exec'd into a subint, persisting across GIL modes. |
| 64 | +- Something else we misdiagnosed — the empirical fix |
| 65 | + (dedicated thread) worked but the analysis may have |
| 66 | + been incomplete. |
| 67 | + |
| 68 | +Only way to know: once we're on isolated mode, empirically |
| 69 | +retry `trio.to_thread.run_sync(interp.exec, ...)` and see |
| 70 | +if `destroy()` still blocks. If it does, keep the |
| 71 | +dedicated thread; if not, one constraint relaxed. |
| 72 | + |
| 73 | +### 3. Fork-from-main-interp-tstate (the constraint in this module's helper names) |
| 74 | + |
| 75 | +The fork-from-main-interp-tstate invariant — CPython's |
| 76 | +`PyOS_AfterFork_Child` → |
| 77 | +`_PyInterpreterState_DeleteExceptMain` gate documented in |
| 78 | +`subint_fork_blocked_by_cpython_post_fork_issue.md` — is |
| 79 | +about the calling thread's **current** tstate at the |
| 80 | +moment `os.fork()` runs. If trio's cache threads never |
| 81 | +enter subints at all, their tstate is plain main-interp, |
| 82 | +and fork from them would be fine. |
| 83 | + |
| 84 | +The reason the smoke test + |
| 85 | +`run_fork_in_non_trio_thread` test helper |
| 86 | +currently use a dedicated `threading.Thread` is narrow: |
| 87 | +**we don't want to risk a trio cache thread that has |
| 88 | +previously been used as a subint driver being the one that |
| 89 | +picks up the fork job**. If cached tstate doesn't get |
| 90 | +cleared (back to reason #2), the fork's child-side |
| 91 | +post-init would see the wrong interp and abort. |
| 92 | + |
| 93 | +In an isolated-mode world where msgspec works: |
| 94 | + |
| 95 | +- `subint_proc` would use the public |
| 96 | + `concurrent.interpreters.create()` + `Interpreter.exec()` |
| 97 | + / `Interpreter.close()` — which *should* handle tstate |
| 98 | + cleanly (they're the "blessed" API). |
| 99 | +- If so, trio's cache threads are safe to fork from |
| 100 | + regardless of whether they've previously driven subints. |
| 101 | +- → the `non_trio` qualifier in |
| 102 | + `run_fork_in_non_trio_thread` becomes |
| 103 | + *overcautious* rather than load-bearing, and the |
| 104 | + dedicated-thread primitives in `_subint_forkserver.py` |
| 105 | + can likely be replaced with straight |
| 106 | + `trio.to_thread.run_sync()` wrappers. |
| 107 | + |
| 108 | +TL;DR |
| 109 | +----- |
| 110 | + |
| 111 | +| constraint | fixed by isolated mode? | |
| 112 | +|---|---| |
| 113 | +| GIL-starvation (class A) | **yes** | |
| 114 | +| destroy race on cached worker | unclear — empirical test on py3.14 + isolated API required | |
| 115 | +| fork-from-main-tstate requirement on worker | **probably yes, conditional on the destroy-race question above** | |
| 116 | + |
| 117 | +If #2 also resolves on py3.14+ with isolated mode, |
| 118 | +tractor could drop the `non_trio` qualifier from the fork |
| 119 | +helper's name and just use `trio.to_thread.run_sync(...)` |
| 120 | +for everything. But **we shouldn't do that preemptively** |
| 121 | +— the current cautious design is cheap (one dedicated |
| 122 | +thread per fork / per subint-exec) and correct. |
| 123 | + |
| 124 | +Audit plan when msgspec #563 lands |
| 125 | +---------------------------------- |
| 126 | + |
| 127 | +Assuming msgspec grows `Py_mod_multiple_interpreters` |
| 128 | +support: |
| 129 | + |
| 130 | +1. **Flip `tractor.spawn._subint` to isolated mode.** Drop |
| 131 | + the `_interpreters.create('legacy')` call in favor of |
| 132 | + the public API (`concurrent.interpreters.create()` + |
| 133 | + `Interpreter.exec()` / `Interpreter.close()`). Run the |
| 134 | + three `ai/conc-anal/subint_*_issue.md` reproducers — |
| 135 | + class-A (`test_stale_entry_is_deleted` etc.) should |
| 136 | + pass without the `skipon_spawn_backend('subint')` marks |
| 137 | + (revisit the marker inventory). |
| 138 | + |
| 139 | +2. **Empirical destroy-race retest.** In `subint_proc`, |
| 140 | + swap the dedicated `threading.Thread` back to |
| 141 | + `trio.to_thread.run_sync(Interpreter.exec, ..., |
| 142 | + abandon_on_cancel=False)` and run the full subint test |
| 143 | + suite. If `Interpreter.close()` (or the backing |
| 144 | + destroy) blocks the same way as the legacy version |
| 145 | + did, revert and keep the dedicated thread. |
| 146 | + |
| 147 | +3. **If #2 clean**, audit `_subint_forkserver.py`: |
| 148 | + - Rename `run_fork_in_non_trio_thread` → drop the |
| 149 | + `_non_trio_` qualifier (e.g. `run_fork_in_thread`) or |
| 150 | + inline the two-line `trio.to_thread.run_sync` call at |
| 151 | + the call sites and drop the helper entirely. |
| 152 | + - Consider whether `fork_from_worker_thread` + |
| 153 | + `run_subint_in_worker_thread` still warrant being |
| 154 | + separate module-level primitives or whether they |
| 155 | + collapse into a compound |
| 156 | + `trio.to_thread.run_sync`-driven pattern inside the |
| 157 | + (future) `subint_forkserver_proc` backend. |
| 158 | + |
| 159 | +4. **Doc fallout.** `subint_sigint_starvation_issue.md` |
| 160 | + and `subint_cancel_delivery_hang_issue.md` both cite |
| 161 | + the legacy-GIL-sharing architecture as the root cause. |
| 162 | + Close them with commit-refs to the isolated-mode |
| 163 | + migration. This doc itself should get a closing |
| 164 | + post-mortem section noting which of #1/#2/#3 actually |
| 165 | + resolved vs persisted. |
| 166 | + |
| 167 | +References |
| 168 | +---------- |
| 169 | + |
| 170 | +- `tractor.spawn._subint_forkserver` — the in-tree module |
| 171 | + whose constraints this doc catalogs. |
| 172 | +- `ai/conc-anal/subint_sigint_starvation_issue.md` — the |
| 173 | + GIL-starvation class. |
| 174 | +- `ai/conc-anal/subint_cancel_delivery_hang_issue.md` — |
| 175 | + sibling Ctrl-C-able hang class. |
| 176 | +- `ai/conc-anal/subint_fork_blocked_by_cpython_post_fork_issue.md` |
| 177 | + — why fork-from-subint is blocked (this drives the |
| 178 | + forkserver-via-non-subint-thread workaround). |
| 179 | +- `ai/conc-anal/subint_fork_from_main_thread_smoketest.py` |
| 180 | + — empirical validation for the workaround. |
| 181 | +- [PEP 684 — per-interpreter GIL](https://peps.python.org/pep-0684/) |
| 182 | +- [PEP 734 — `concurrent.interpreters` public API](https://peps.python.org/pep-0734/) |
| 183 | +- [jcrist/msgspec#563 — PEP 684 support tracker](https://github.com/jcrist/msgspec/issues/563) |
| 184 | +- tractor issue #379 — subint backend tracking. |
0 commit comments