Skip to content

Commit ae8799c

Browse files
Transurgeonclaude
andcommitted
Bump engine to d8cb9b8 so the GIL-free declaration actually holds
The audit in 2e19844 was performed against engine d8cb9b8, but the submodule pointer was left at 4172c5e from #21, nine commits behind. The two revisions differ in exactly the way the audit depended on. Engine 6e64401 ("Make peak-memory tracking a compile-time option (SP_TRACK_MEMORY)", merged one day before this branch) puts the g_allocated_bytes / g_peak_bytes counters behind an option that defaults to OFF, precisely so the library can be called from several threads at once. At 4172c5e those counters are unconditional, and every sp_malloc / sp_free updates them non-atomically. So the audit's "no writable data/bss symbols" finding was true of the engine it inspected and false of the engine this branch ships: nm on the cp313t extension built from 4172c5e lists _g_allocated_bytes and _g_peak_bytes as its only external writable data. That race is reachable from the usage the README blesses, since two threads building or evaluating *distinct* problems both allocate. Bumping the pointer removes it: nm on the rebuilt extension shows no mutable globals at all, and the engine's own ctest suite passes at d8cb9b8. Verified on cpython-3.13.5+freethreaded, 8 threads: * independent problem per thread, 300 evaluations each, every result bit-identical to a single-threaded reference: 10/10 runs clean. * sharing one expression capsule across threads, which the README forbids: 10/10 runs die with SIGSEGV / SIGBUS / SIGABRT / SIGTRAP. The second number is why the README and the bindings.c comment no longer describe that contract as "the same contract as with the GIL". expr::refcount is a plain int updated non-atomically by expr_retain() / free_expr(), and capsule destructors run on whichever thread drops the last Python reference, so breaking the rule now corrupts the heap instead of interleaving calls. The same misuse is harmless under the GIL. Making that count atomic upstream would turn it back into ordinary unsupported usage; until then the docs say plainly how sharp the edge is. Both documents also note that SP_TRACK_MEMORY must stay off in a wheel build, since turning it on silently reintroduces the global-counter race. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 8406c34 commit ae8799c

3 files changed

Lines changed: 48 additions & 19 deletions

File tree

‎README.md‎

Lines changed: 28 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -17,18 +17,34 @@ from sparsediffpy import _sparsediffengine
1717
## Free-threaded Python
1818

1919
Wheels are published for free-threaded CPython 3.14 (`cp314t`) as well as the
20-
default builds. There is no 3.13t wheel because NumPy 2.5 and later ship no
21-
`cp313t` wheels. The extension declares that it does not need the GIL:
22-
the engine keeps no global mutable state, every problem and expression owns its
23-
own buffers, and all inputs and outputs are copied at the boundary. Distinct
24-
problems can therefore be built and evaluated concurrently from different
25-
threads.
26-
27-
A single problem or expression capsule is not thread-safe. Do not call into the
28-
same problem from two threads at once, and do not share expression capsules
29-
between problems that are evaluated concurrently. This is the same contract as
30-
under the GIL, which only serialized individual calls and never protected
31-
against interleaved use of one object.
20+
default builds. There is no `cp313t` wheel: NumPy ships none from 2.5 onwards
21+
(2.4.6 is the last release with one), so a 3.13t build would have to compile
22+
NumPy from source.
23+
24+
The extension declares that it does not need the GIL, so importing it leaves
25+
free threading enabled. Distinct problems can be built and evaluated
26+
concurrently from different threads: the engine keeps no mutable global state,
27+
every problem and expression owns its own buffers, and all inputs and outputs
28+
are copied at the boundary.
29+
30+
This depends on the engine being built without `SP_TRACK_MEMORY`, which is the
31+
default. That option makes every allocation update the `g_allocated_bytes` and
32+
`g_peak_bytes` process globals non-atomically, so two threads that merely
33+
allocate race on them. It is a development switch; do not turn it on for a
34+
wheel build.
35+
36+
A single problem or expression capsule is **not** thread-safe. Do not call into
37+
the same problem from two threads at once, and do not share expression capsules
38+
between problems that are evaluated concurrently.
39+
40+
Without the GIL, breaking that rule is worse than it used to be. `expr::refcount`
41+
is a plain `int` that `expr_retain()` and `free_expr()` update non-atomically,
42+
and capsule destructors run on whichever thread drops the last Python reference.
43+
So sharing one capsule across threads corrupts the heap instead of merely
44+
interleaving calls: an eight-thread loop building and dropping `exp()` nodes over
45+
one shared variable node crashes on every run here, while the same loop is
46+
harmless under the GIL. Making that reference count atomic upstream would turn
47+
this back into an ordinary "unsupported usage" rather than a crash.
3248

3349
## License
3450

‎SparseDiffEngine‎

Submodule SparseDiffEngine updated 136 files

‎sparsediffpy/_bindings/bindings.c‎

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -191,12 +191,25 @@ PyMODINIT_FUNC PyInit__sparsediffengine(void)
191191
if (!module) return NULL;
192192
#ifdef Py_GIL_DISABLED
193193
/* Free-threaded CPython (3.13t+): declare that this module does not need
194-
the GIL. The engine keeps no global mutable state -- every problem and
195-
expression owns its own buffers, and the wrappers copy inputs and
196-
outputs -- so distinct problems may be used concurrently from different
197-
threads. A single problem or expression capsule is not thread-safe and
198-
must not be used from two threads at once (same contract as with the
199-
GIL, which never protected against interleaved calls on one object). */
194+
the GIL.
195+
196+
This holds only for an engine built WITHOUT SP_TRACK_MEMORY, which is
197+
the default. With that option on, every sp_malloc / sp_free updates the
198+
g_allocated_bytes and g_peak_bytes process globals non-atomically, so
199+
two threads that merely allocate would race. Never enable it for a
200+
wheel build.
201+
202+
Otherwise the engine keeps no mutable global state: every problem and
203+
expression owns its buffers, and the wrappers copy inputs and outputs,
204+
so distinct problems may be built and evaluated concurrently.
205+
206+
A single problem or expression capsule remains NOT thread-safe, and
207+
without the GIL the consequence is harsher than it used to be.
208+
expr::refcount is a plain int that expr_retain() and free_expr() update
209+
non-atomically, and capsule destructors run on whichever thread drops
210+
the last Python reference. Sharing one capsule across threads therefore
211+
corrupts the heap rather than merely interleaving calls. Making that
212+
count atomic upstream would remove the sharp edge. */
200213
PyUnstable_Module_SetGIL(module, Py_MOD_GIL_NOT_USED);
201214
#endif
202215
return module;

0 commit comments

Comments
 (0)