René's URL Explorer Experiment

Title: Protect the container walks in dumps() against concurrent mutation (#234) by espressolee · Pull Request #235 · python-rapidjson/python-rapidjson · GitHub

Open Graph Title: Protect the container walks in dumps() against concurrent mutation (#234) by espressolee · Pull Request #235 · python-rapidjson/python-rapidjson

X Title: Protect the container walks in dumps() against concurrent mutation (#234) by espressolee · Pull Request #235 · python-rapidjson/python-rapidjson

Description: Following up on #234 with the fix, since you said help would be welcome. The PR targets free-threaded, where Py_MOD_GIL_NOT_USED makes the concurrent paths reachable. The list hunk also fixes a pre-existing single-threaded callback re-entrancy crash on GIL builds, including released 1.23. Current head: 7eb5586a477535afbff4c356f9dc104cb6854d80. What it changes Four container walks in dumps() read the caller's object without holding anything: site fix why this one all_keys_are_string() critical section on the dict the loop calls no Python, so a section is enough and cheap. Restructured to a single exit — returning from inside a critical section would leak it the list walk in dumps_internal() re-read the length each iteration, PyList_GetItemRef() the unchecked macro over a size captured once reads past the end after a concurrent or re-entrant shrink; PyList_GetItemRef is bounds-checked and returns a strong reference the two dict walks in dumps_internal() on free-threaded builds, iterate a snapshot from PyDict_Items(); GIL builds retain PyDict_Next streaming a critical section is not sufficient on free-threaded builds: these bodies call back into Python (RECURSE, PyObject_Str), and a critical section is suspended when the thread blocks. A small RAII guard owns the snapshot so the eight existing early returns cannot leak it. The #ifdef Py_GIL_DISABLED boundary avoids charging default builds for a safety mechanism they do not need In rapidjson.cpp, the loop bodies are unchanged — only the headers and item acquisition move. Measured python3.14.0rc1t, module built from this branch, no PYTHON_GIL override, 10 runs per arm: reproducer before after mutate the dict being dumped (#234) 10/10 SIGSEGV 0/10 mutate the list being dumped 10/10 SIGSEGV 0/10 control — no mutator clean clean control — mutate a different object clean clean control — same test, PYTHON_GIL=1 clean clean The list bug also reproduces without concurrency: on released python-rapidjson 1.23, a single-threaded default= callback that shrinks the list being encoded crashed 5/5 on stock CPython 3.11, 3.12, and 3.13, and 10/10 on 3.14.6. Same-object controls were positive while no-mutation, different-object, append-only, and tuple controls were clean. This branch was 0/10 in both GIL and free-threaded modes for the same reproducer. pytest tests/ on the patched free-threaded build: 914 passed, 26 skipped, 2 xfailed. Snapshot cost follow-up Same interpreters, same data, nine timed samples per case, median ns/dump: build flat 16 flat 1024 nested 128×8 peak delta, flat 1024 GIL, conditional patch vs base -1.1% +0.4% +0.9% 0 bytes no-GIL, snapshot patch vs base +3.7% +1.9% +24.2% +65,592 bytes Compatibility/test matrix after the follow-up: Python 3.12.13 and 3.14.6 each 923 passed, 17 skipped, 2 xfailed; Python 3.14.0rc1t 914 passed, 26 skipped, 2 xfailed. Intermediate measurement, in case it is useful: with only the first two fixes applied the dict reproducer still crashed 8/10, which is what pinned the remaining crashes to the two dumps_internal dict walks rather than to all_keys_are_string. CI / exact head Exact-head workflow run 32349040222 completed successfully for 7eb5586a477535afbff4c356f9dc104cb6854d80: all 9 build/test jobs passed, including CPython 3.14t, debug leak tests, Linux x86_64, aarch64 and ppc64le, macOS arm64 and Intel, and Windows; the PyPI upload job was skipped on the pull request as intended; six wheel artifacts were produced. What I did not do No change to the Py_MOD_GIL_NOT_USED declaration — it is already on this branch and I did not touch it. With these four sites protected the ordering is now the safe way round; before them, the declaration was what removed the interpreter-level protection. The snapshot is free-threaded-only. A controlled microbenchmark found the uniform implementation cost 11.9–25.1% on GIL builds, with +65,592 bytes peak allocation for a 1,024-entry dict. The two PyDict_Items walks are therefore behind #ifdef Py_GIL_DISABLED; GIL builds retain PyDict_Next streaming. After that change, GIL timing was within -1.1% to +0.9% of base and peak allocation was identical. The free-threaded build still pays the snapshot cost, where it is the safety mechanism. No general dict re-entrancy claim. The list case above is a measured positive and is fixed by this branch. Five single-threaded dict callback/coercion/sorting mutation shapes stayed fault-free on patched and unpatched builds in both GIL modes; that bounds the claim to the tested inputs rather than proving no dict re-entrancy bug exists. Reshape any of this however you prefer — including rejecting the snapshot approach for the dict walks, which is the one real design choice in here. (Fix developed with AI assistance; the measurements are mine and reproducible.)

Open Graph Description: Following up on #234 with the fix, since you said help would be welcome. The PR targets free-threaded, where Py_MOD_GIL_NOT_USED makes the concurrent paths reachable. The list hunk also fixes a pre...

X Description: Following up on #234 with the fix, since you said help would be welcome. The PR targets free-threaded, where Py_MOD_GIL_NOT_USED makes the concurrent paths reachable. The list hunk also fixes a pre...

Opengraph URL: https://github.com/python-rapidjson/python-rapidjson/pull/235

X: @github

direct link

Domain: github.com

Links:

Viewport: width=device-width