René's URL Explorer Experiment

{"@context":"https://schema.org","@type":"DiscussionForumPosting","headline":"A (tentative) TODO list? (Generated from Claude Code plugins)","articleBody":"From https://github.com/devdanzin/cext-review-toolkit:\n\n```\n  C Extension Analysis Report\n\n  Extension: msgpack._cmsgpack\n\n  Scope: entire project\n\n  Agents Run: type-slot-checker, gil-discipline-checker, resource-lifecycle-checker,\n  module-state-checker, parity-checker, version-compat-scanner, c-complexity-analyzer,\n  stable-abi-checker, git-history-analyzer\n\n  (refcount-auditor, error-path-analyzer, null-safety-scanner, pyerr-clear-auditor\n  skipped — Cython-mode, validated zero FIX-class loss)\n\n  External Tools: cppcheck (7 .h files scanned), clang-tidy (skipped — no\n  compile_commands.json; requires `make cython \u0026\u0026 python setup.py build_ext -i` first)\n\n  ---\n  Executive Summary\n\n  The extension is broadly well-implemented for a Cython project: per-instance buffer\n  allocation, comprehensive @cython.critical_section coverage on public methods, and a\n  solid exception-handling architecture. Three areas need immediate attention. First,\n  the recent free-threading commit (#641) missed Packer.__getbuffer__ and\n  __releasebuffer__ — the two buffer-protocol methods that directly protect the C\n  buffer from concurrent reallocation — creating a use-after-free window under\n  Py_GIL_DISABLED. Second, Unpacker.__init__ has a pre-existing re-initialization leak\n  pattern (both the C buffer and the in-progress parse stack) that has never been fixed\n  upstream. Third, the pure-Python fallback diverges from the Cython implementation in\n  three user-visible ways: read_bytes() return type, object_pairs_hook contract, and\n  skip() exception translation. The git history also reveals the fork is 7+ bug-fix\n  commits behind the upstream canonical repo.\n\n  ---\n  Extension Profile\n\n  - Module: msgpack._cmsgpack (3 .pyx files, 940 lines; 7 hand-written .h files, 1,830\n  lines)\n  - Init style: Multi-phase (structural), but with process-global module state\n  (CYTHON_USE_MODULE_STATE=0, m_size=0)\n  - Python targets: ≥3.10\n  - Limited API: not claimed\n  - Code generation: Cython 3.2.1\n  - freethreading_compatible = True declared\n\n  ---\n  External Tool Baseline\n\n  cppcheck ran against all 7 hand-written .h files (pack.h, pack_template.h,\n  sysdep.h, unpack.h, unpack_container_header.h, unpack_define.h,\n  unpack_template.h). One finding:\n\n  ▎ FALSE POSITIVE — pack_template.h:256 (cppcheck: syntaxError)\n  ▎ Root cause: cppcheck's preprocessor evaluator cannot resolve the\n  ▎ three-branch portability idiom used in msgpack_pack_short (and\n  ▎ msgpack_pack_int, msgpack_pack_long, etc.):\n  ▎\n  ▎   #if defined(SIZEOF_SHORT)\n  ▎     #if SIZEOF_SHORT == 2 ... #endif\n  ▎   #elif defined(SHRT_MAX)\n  ▎     #if SHRT_MAX == 0x7fff ... #endif  ← nested #if inside #elif\n  ▎   #else\n  ▎     if(sizeof(short) == 2) { ... }    ← plain if, not #if\n  ▎   #endif\n  ▎\n  ▎ The code is valid C. cppcheck bails at line 256 and does not analyze\n  ▎ the remainder of pack_template.h (~350 lines including _PyFloat_* guards\n  ▎ and the unsigned-int packing logic). See also finding 23 (complexity).\n  ▎\n  ▎ Workaround: build the extension with `make cython \u0026\u0026 python setup.py\n  ▎ build_ext -i`, then pass `compile_commands.json` to cppcheck via\n  ▎ `--project=`. clang-tidy with compile_commands.json would also\n  ▎ resolve the macro definitions and produce richer analysis.\n\n  ---\n  Key Metrics\n\n  ┌──────────────┬────────┬─────┬──────────┬───────────────────────────────────────┐\n  │  Dimension   │ Status │ FIX │ CONSIDER │              Top Finding              │\n  ├──────────────┼────────┼─────┼──────────┼───────────────────────────────────────┤\n  │ Refcount     │ G      │ 0   │ 0        │ Cython-mode skip                      │\n  │ Safety       │        │     │          │                                       │\n  ├──────────────┼────────┼─────┼──────────┼───────────────────────────────────────┤\n  │ Error        │ G      │ 0   │ 0        │ Cython-mode skip                      │\n  │ Handling     │        │     │          │                                       │\n  ├──────────────┼────────┼─────┼──────────┼───────────────────────────────────────┤\n  │ NULL Safety  │ G      │ 0   │ 0        │ Cython-mode skip                      │\n  ├──────────────┼────────┼─────┼──────────┼───────────────────────────────────────┤\n  │ GIL          │ Y      │ 1   │ 0        │ __getbuffer__/__releasebuffer__       │\n  │ Discipline   │        │     │          │ missing @cython.critical_section      │\n  ├──────────────┼────────┼─────┼──────────┼───────────────────────────────────────┤\n  │ Resource     │ G      │ 0   │ 1        │ Partial parse-stack retained on       │\n  │ Lifecycle    │        │     │          │ _unpack error paths                   │\n  ├──────────────┼────────┼─────┼──────────┼───────────────────────────────────────┤\n  │ Module State │ Y      │ 0   │ 3        │ 5 process-global PyObject* statics;   │\n  │              │        │     │          │ PyDateTimeAPI cross-interpreter       │\n  ├──────────────┼────────┼─────┼──────────┼───────────────────────────────────────┤\n  │ Type Slots   │ Y      │ 2   │ 0        │ Unpacker.__init__ re-init leaks       │\n  │              │        │     │          │ self.buf and parse stack              │\n  ├──────────────┼────────┼─────┼──────────┼───────────────────────────────────────┤\n  │ PyErr_Clear  │ G      │ 0   │ 0        │ Cython-mode skip                      │\n  │ Safety       │        │     │          │                                       │\n  ├──────────────┼────────┼─────┼──────────┼───────────────────────────────────────┤\n  │ ABI          │ G      │ 0   │ 6        │ Not claimed; datetime C API is hard   │\n  │ Compliance   │        │     │          │ blocker                               │\n  ├──────────────┼────────┼─────┼──────────┼───────────────────────────────────────┤\n  │ Version      │ G      │ 0   │ 5        │ Dead sysdep.h guards; _PyFloat_*      │\n  │ Compat       │        │     │          │ correctly guarded to 3.10-only        │\n  ├──────────────┼────────┼─────┼──────────┼───────────────────────────────────────┤\n  │ C/Python     │ Y      │ 3   │ 5        │ read_bytes type, object_pairs_hook    │\n  │ Parity       │        │     │          │ contract, skip() exception            │\n  ├──────────────┼────────┼─────┼──────────┼───────────────────────────────────────┤\n  │ Complexity   │ G      │ 0   │ 3        │ unpack_execute CC ~60+, macro/goto    │\n  │              │        │     │          │ contract undocumented                 │\n  └──────────────┴────────┴─────┴──────────┴───────────────────────────────────────┘\n\n  G = 0 FIX | Y = 1–3 FIX | R = 4+ FIX\n\n  ---\n  Findings by Priority\n\n  Must Fix (FIX) — 6\n\n  #: 1\n  Finding: Packer.__getbuffer__ and __releasebuffer__ missing @cython.critical_section\n  —\n    concurrent pack() can call PyMem_Realloc on self.pk.buf while another thread's\n    memoryview(packer) is calling PyBuffer_FillInfo on the same pointer;\n  use-after-free.\n     Also self.exports read-modify-write is non-atomic.\n  File:Line: _packer.pyx:363-368\n  Agents: gil-discipline, git-history\n  ────────────────────────────────────────\n  #: 2\n  Finding: Unpacker.__init__ overwrites self.buf via PyMem_Malloc without first freeing\n\n    the existing buffer — any re-call to __init__ (or subclass super().__init__(...))\n    leaks the prior allocation\n  File:Line: _unpacker.pyx:373\n  Agents: type-slot, git-history\n  ────────────────────────────────────────\n  #: 3\n  Finding: Unpacker.__init__ calls unpack_init (not unpack_clear) before\n  re-initializing\n    the parse context — live PyObject* references in ctx-\u003estack[1..top] are leaked if\n    called on a mid-stream Unpacker\n  File:Line: _unpacker.pyx:385\n  Agents: type-slot, git-history\n  ────────────────────────────────────────\n  #: 4\n  Finding: fallback.Unpacker.read_bytes() returns bytearray (mutable, unhashable);\n    Cython returns bytes — dict-key use, isinstance checks, and hashability all diverge\n  File:Line: fallback.py:346-348\n  Agents: parity, git-history\n  ────────────────────────────────────────\n  #: 5\n  Finding: fallback.Unpacker passes a generator to object_pairs_hook; Cython passes a\n    fully-materialised list — len(pairs), re-iteration, and subscript access all break;\n\n    additionally, strict_map_key validation is lazy in the fallback and can be bypassed\n\n    by a hook that doesn't fully consume the generator\n  File:Line: fallback.py:520-529\n  Agents: parity, git-history\n  ────────────────────────────────────────\n  #: 6\n  Finding: fallback.Unpacker.skip() lets RecursionError propagate raw; every other\n    unpack method converts it to StackError — callers catching StackError/ValueError to\n\n    guard against adversarial nested data miss the exception on this path\n  File:Line: fallback.py:584-587\n  Agents: parity, git-history\n\n  ---\n  Should Consider (CONSIDER) — 20\n\n  #: 7\n  Finding: Unpacker._unpack error paths (FormatError, StackError, PyErr_Occurred) do\n  not\n    call unpack_clear before raising — partially-built container objects in\n    ctx-\u003estack[1..top] are retained until __dealloc__. Bounded at 1024 slots; contrast\n    with unpackb which already calls unpack_clear on failure\n  File:Line: _unpacker.pyx:484-491\n  ────────────────────────────────────────\n  #: 8\n  Finding: utc, epoch, giga, ExtType, Timestamp compile to file-scope static PyObject*\n    process globals (CYTHON_USE_MODULE_STATE=0). Re-importing in a second\n  subinterpreter\n     clobbers the shared pointer, leaving the first interpreter's live Packer/Unpacker\n    instances with stale or cross-interpreter object references\n  File:Line: _cmsgpack.pyx:8-9, _unpacker.pyx:20\n  ────────────────────────────────────────\n  #: 9\n  Finding: PyDateTimeAPI is a per-translation-unit static pointer (from datetime.h).\n  The\n    module-exec function calls import_datetime() per interpreter import, clobbering the\n\n    global with a capsule from the new interpreter. Cross-interpreter datetime\n  timestamp\n     decode (timestamp=3) will call function pointers from the wrong interpreter\n  File:Line: _cmsgpack.pyx:5, unpack.h:339-344\n  ────────────────────────────────────────\n  #: 10\n  Finding: Py_mod_multiple_interpreters: NOT_SUPPORTED slot is guarded by #if\n    CYTHON_USE_MODULE_STATE (always 0 in shipped builds) — the multi-interpreter\n  posture\n     is absent from the compiled binary, leaving CPython to apply an implicit default\n    rather than an explicit declaration\n  File:Line: generated C\n  ────────────────────────────────────────\n  #: 11\n  Finding: Fork is 7+ upstream bug-fix commits behind: missing pack_timestamp\n    return-code checking (378edc6), missing pack_ext_type autoreset (284782d), missing\n    unpack_callback_int64 null check (95c8be5), missing unpack_container_header return\n    check (156bb05)\n  File:Line: various\n  ────────────────────────────────────────\n  #: 12\n  Finding: unpackb accepts max_buffer_size and read_size silently in the fallback (via\n    **kwargs to Unpacker.__init__) but raises TypeError in Cython — a max_buffer_size\n    limit passed to unpackb is silently ignored by the Cython path, breaking any caller\n\n    relying on it as a security cap\n  File:Line: _unpacker.pyx:141-149, fallback.py:72\n  ────────────────────────────────────────\n  #: 13\n  Finding: pack_ext_type in Cython passes typecode directly to a C char — values\n  outside\n    0–127 silently overflow or encode the reserved timestamp type (-1); the fallback\n    validates and raises ValueError for out-of-range codes\n  File:Line: _packer.pyx:288-297\n  ────────────────────────────────────────\n  #: 14\n  Finding: Multi-byte buffer input raises BufferError in Cython, ValueError in fallback\n\n    — callers catching ValueError to handle all invalid input miss the exception on the\n\n    Cython path\n  File:Line: _unpacker.pyx:127, fallback.py:68\n  ────────────────────────────────────────\n  #: 15\n  Finding: read_size default is 1 MiB in Cython vs 16 KiB in fallback and docs — Cython\n\n    issues 64× larger reads than documented from file_like sources\n  File:Line: _unpacker.pyx:369\n  ────────────────────────────────────────\n  #: 16\n  Finding: datetime subclass packing: Cython uses PyDateTime_CheckExact (excludes\n    subclasses like pendulum.DateTime), fallback uses isinstance (includes them) —\n    undocumented behavioral split\n  File:Line: _packer.pyx:243\n  ────────────────────────────────────────\n  #: 17\n  Finding: Dead MSVC pre-2010 integer typedef block (_MSC_VER \u003c 1600) — unreachable\n    since VS 2010; Python ≥3.10 requires VS 2019+\n  File:Line: sysdep.h:23-31\n  ────────────────────────────────────────\n  #: 18\n  Finding: Dead GCC \u003c4.1 atomic macro branch + unused\n    _msgpack_atomic_counter_t/_msgpack_sync_* infrastructure — defined but never called\n\n    anywhere in the codebase; likely a leftover from an earlier refcount-based design\n  File:Line: sysdep.h:39-49\n  ────────────────────────────────────────\n  #: 19\n  Finding: pyproject.toml declares cython\u003e=3.2.5 with no upper bound; CI pins\n    Cython==3.2.1 via requirements.txt — developer environments can pick up Cython 4\n    before any migration review\n  File:Line: pyproject.toml:52, requirements.txt:1\n  ────────────────────────────────────────\n  #: 20\n  Finding: PyUnicode_InternInPlace on every map key from untrusted sources — in Python\n    3.12+ mortal interned strings persist in the intern table; adversarial input can\n    hold arbitrarily many unique strings in the table, a memory amplification vector\n  File:Line: unpack.h:191-192\n  ────────────────────────────────────────\n  #: 21\n  Finding: Py_SIZE(o) used on bytes/bytearray/list — soft-deprecated direct struct\n    access; stable-ABI-safe alternatives (PyBytes_Size, PyList_Size, etc.) are drop-in\n    replacements\n  File:Line: _packer.pyx:186,199,226\n  ────────────────────────────────────────\n  #: 22\n  Finding: Py_TYPE(o)-\u003etp_name direct PyTypeObject field dereference in error-path\n    format strings — not available under Limited API; use %T (3.12+) or PyType_GetName\n    (3.11+)\n  File:Line: _packer.pyx:188,254,256, unpack.h:188\n  ────────────────────────────────────────\n  #: 23\n  Finding: unpack_execute (~308 lines, CC ~60+, 10 goto targets) — the macro/goto\n    contract is undocumented; adding a new msgpack type without knowing that every case\n\n    arm must terminate with a push_*/again_* macro will silently fall through to\n    _failed. Also: MSVC and GCC code paths for SWITCH_RANGE diverge without CI coverage\n\n    of the MSVC path\n  File:Line: unpack_template.h:86-394\n  ────────────────────────────────────────\n  #: 24\n  Finding: _pack_inner (~105 lines, CC ~25) — unicode encoding and datetime arithmetic\n    are inlined; extracting _pack_unicode and _pack_datetime helpers would reduce\n    cognitive load without performance cost\n  File:Line: _packer.pyx:152-256\n  ────────────────────────────────────────\n  #: 25\n  Finding: unpack_callback_ext (~90 lines, CC ~20) — four timestamp-mode arms each\n    manage 2–5 manual refcount variables with early-return error paths; variable\n    aliasing in mode-1 (float path: a is reassigned after Py_DECREF calls); extracting\n    per-mode helpers would remove this as the highest-risk refcount surface in the\n    codebase\n  File:Line: unpack.h:292-381\n  ────────────────────────────────────────\n  #: 26\n  Finding: PyList_SET_ITEM/PyTuple_SET_ITEM are non-Limited-API macros (direct ob_item\n    access); PyList_SetItem/PyTuple_SetItem are stable-ABI drop-ins (add return-value\n    check)\n  File:Line: unpack.h:148,150,200\n\n  ---\n  Tensions\n\n  - Free-threading vs. subinterpreters: The extension declared freethreading_compatible\n  = True in the last commit but has process-global PyObject* state (findings 8–10)\n  that makes it subinterpreter-unsafe. These goals are not contradictory —\n  free-threading safety is per-instance lock discipline, subinterpreter safety is\n  per-interpreter state isolation — but shipping freethreading_compatible without the\n  matching NOT_SUPPORTED slot risks user expectation mismatch.\n  - strict_map_key security in object_pairs_hook: The Cython path enforces key\n  validation eagerly in C before the hook is called (safe). The fallback enforces it\n  lazily inside a generator (finding 5). Upstream committed a partial fix (cd81370) but\n  it still passes a generator, not a list.\n\n  ---\n  Policy Decisions (POLICY) — 3\n\n  #: P1\n  Finding: _PyFloat_Pack4/8/_PyFloat_Unpack4/8 private API usage is correctly guarded\n  to\n    Python 3.10 only via #if PY_VERSION_HEX \u003e= 0x030B00A7. When Python 3.10 reaches EOL\n\n    (Oct 2026), remove the #else branches entirely. Alternatively, replace with manual\n    IEEE 754 packing using the existing _msgpack_store32/64/_msgpack_load32/64\n  byte-swap\n     infrastructure — this eliminates the version fork and the CPython dependency from\n    the float path.\n  ────────────────────────────────────────\n  #: P2\n  Finding: Stable ABI (abi3) is not feasible at Python 3.10 without rework — the hard\n    blockers are the datetime C API (PyDelta_FromDSU not in stable ABI, PyDateTimeAPI\n    capsule not in Limited API) and _PyFloat_* on Python 3.10. Raising the minimum to\n    3.11 fixes the float issue; the datetime path requires replacing capsule calls with\n\n    PyObject_CallMethod/PyObject_IsInstance. Benefit: single abi3 wheel replacing\n    per-minor wheels. Assess when preparing the 3.10-EOL release.\n  ────────────────────────────────────────\n  #: P3\n  Finding: Add an unconditional {Py_mod_multiple_interpreters,\n    Py_MOD_MULTIPLE_INTERPRETERS_NOT_SUPPORTED} slot to the module def (not guarded by\n    CYTHON_USE_MODULE_STATE) to make the multi-interpreter posture explicit. This is a\n    near-zero-cost protective declaration that blocks CPython from loading the module\n    into a second subinterpreter until the process-global state (findings 8–9) is\n    resolved.\n\n  ---\n  Strengths\n\n  - Per-instance locking discipline is excellent: Every public Python method on Packer\n  and Unpacker carries @cython.critical_section, correctly serialising per-object state\n  for free-threading. The two missing methods (finding 1) are a narrow, fixable gap in\n  an otherwise complete implementation.\n  - C buffer management is correct on the happy path: msgpack_pack_write uses the\n  classic local-pointer-then-commit realloc pattern (pack.h:47-51) that correctly\n  preserves pk-\u003ebuf on allocation failure. All 4 tracked heap allocations across the\n  extension have matching frees.\n  - Exception propagation is thorough: unpackb and Unpacker._unpack both catch and\n  correctly convert RecursionError, OutOfData, FormatError, and re-raise existing\n  exceptions — a complete chain validated by the recent upstream sweep.\n  - No _Py* private API beyond the correctly-version-guarded _PyFloat_* fallback: The\n  extension uses only stable, documented CPython APIs in production code paths.\n  - unpack_callback_ext and the streaming FSM are functionally correct: Despite their\n  complexity, no correctness bugs were found in unpack_execute or the timestamp decode\n  path during this review.\n  - freethreading_compatible declaration is substantively justified: Module-level\n  globals are write-once at import and read-only thereafter; per-instance C buffers are\n  strictly isolated. The declaration is correct subject to fixing finding 1.\n\n  ---\n  Code Removal Opportunities\n\n  Based on data/deprecated_apis.json patterns applicable at Python ≥3.10:\n\n  - Py_SIZE → PyBytes_Size/PyList_Size/PyTuple_Size: 3 call sites\n  (_packer.pyx:186,199,226) — mechanical one-line substitution each.\n  - Py_TYPE()-\u003etp_name → %T format (Python ≥3.12): 4 sites (_packer.pyx:188,254,256,\n  unpack.h:188) — simplifies error formatting, removes struct access.\n  - Dead sysdep.h blocks (findings 17–18): Removing the MSVC pre-2010 typedefs and the\n  unused atomic counter infrastructure eliminates ~25 lines of dead code.\n  - _PyFloat_* fallback (when 3.10 EOL): Removing the #else branches in pack_template.h\n  and unpack_template.h eliminates ~8 lines and the last private-API usage.\n\n  ---\n  Recommended Action Plan\n\n  Immediate (FIX items)\n\n  1. Fix Finding 1 — Add @cython.critical_section to Packer.__getbuffer__ and\n  __releasebuffer__ (_packer.pyx:363-368). Two lines.\n  2. Fix Findings 2 \u0026 3 — At the top of Unpacker.__init__, add a guard: if self.buf !=\n  NULL: unpack_clear(\u0026self.ctx); PyMem_Free(self.buf); self.buf = NULL before the\n  PyMem_Malloc call (_unpacker.pyx:373).\n  3. Fix Finding 4 — Change fallback.Unpacker.read_bytes to wrap result in bytes(...)\n  before returning (fallback.py:348).\n  4. Fix Finding 5 — Materialise the key-value list before calling object_pairs_hook in\n  the fallback (fallback.py:520-529), and enforce strict_map_key eagerly on each key.\n  5. Fix Finding 6 — Wrap fallback.Unpacker.skip() body in try/except RecursionError:\n  raise StackError (fallback.py:584-587).\n  6. Finding 11 — Sync with upstream canonical repo to pick up the 7 post-#641 bug-fix\n  commits; priority: 378edc6 (pack_timestamp return codes), 95c8be5\n  (unpack_callback_int64 null check), 284782d (pack_ext_type autoreset).\n\n  Short-term (CONSIDER items)\n\n  7. Finding 7 — Add unpack_clear(\u0026self.ctx) before each raise in Unpacker._unpack\n  (mirroring unpackb).\n  8. Finding 13 — Add typecode range validation (0–127) to Packer.pack_ext_type in the\n  Cython path.\n  9. Finding 15 — Fix read_size default in _unpacker.pyx:369 to min(max_buffer_size,\n  16*1024).\n  10. Finding 23 — Add a block comment to unpack_execute documenting the macro/goto\n  contract (\"every case arm must terminate with a push_*/again_* macro\"). Add a CI\n  build that tests the MSVC code path for SWITCH_RANGE parity.\n  11. Findings 17–18 — Remove dead MSVC \u003c1600 typedef block and unused atomic counter\n  infrastructure from sysdep.h.\n  12. Finding 19 — Add \u003c4 upper bound to Cython dev dependency in pyproject.toml; align\n  with requirements.txt.\n  13. Finding P3 — Add unconditional Py_MOD_MULTIPLE_INTERPRETERS_NOT_SUPPORTED slot.\n\n  Longer-term (POLICY)\n\n  14. Findings 8–10 / P3 — Build with -DCYTHON_USE_MODULE_STATE=1 to activate Cython\n  3.2+'s per-interpreter module state, migrating the 5 cdef object globals into\n  __pyx_mstatetype. This is the path to genuine subinterpreter safety.\n  15. Finding 25 / P1 — When Python 3.10 reaches EOL: collapse _PyFloat_* #else\n  branches; extract unpack_callback_ext timestamp modes into per-mode helpers.\n  16. Finding P2 — When preparing the 3.10-EOL release: assess abi3 migration; the\n  datetime path rework is the main investment.\n\n  ---\n\n  ▎ Note before sharing externally: This report is an internal artifact. Before filing\n  ▎ issues or opening PRs against the upstream msgpack/msgpack-python repo, review\n  ▎ WORKING_WITH_MAINTAINERS.md (../../../WORKING_WITH_MAINTAINERS.md). The upstream\n  ▎ maintainer is actively fixing bugs (7 commits since #641); reach out first to\n  ▎ understand preferred contribution format. Security-relevant findings (finding 5's\n  ▎ strict_map_key bypass via generator) should go through responsible disclosure if\n  ▎ reported publicly.\n```\n\nFrom https://github.com/devdanzin/ft-review-toolkit:\n\n```\n  Free-Threading Analysis Report\n\n  Extension: msgpack._cmsgpack (3 .pyx files, 7 .h files, ~2,770 lines)\n\n  Agents Run: shared-state-auditor, ft-history-analyzer, lock-discipline-checker,\n  atomic-candidate-finder, unsafe-api-detector, stop-the-world-advisor\n\n  (tsan-report-analyzer skipped — no TSan report provided;\n   stw-safety-checker skipped — no _PyEval_StopTheWorld usage)\n\n  ---\n  Migration Status\n\n  - First ft commit: af45640 \"cython: freethreading_compatible (#654)\" — 2025-10-09\n  - Latest ft commit: 9de2fd9 \"Add no-GIL interpreter support (#641)\" — 2026-06-02\n  - Substantive migration commits: 2 (af45640 adds critical sections; 9de2fd9 adds FT CI)\n\n  Decorator sweep (af45640):\n  - Packer public methods decorated: __init__, pack, pack_ext_type,\n    pack_array_header, pack_map_header, pack_map_pairs, reset, bytes (8/10)\n  - Packer public methods missed: __getbuffer__, __releasebuffer__ (2/10)\n  - Unpacker public methods decorated: __init__, feed, read_bytes, unpack, skip,\n    read_array_header, read_map_header, tell, __next__ (9/9)\n\n  CI infrastructure (9de2fd9):\n  - Adds pytest-run-parallel with --parallel-threads=auto --iterations=20 for *t\n    Python versions\n  - Adds test/uneeded_test_multithreading.py (manual stress test for Packer.pack)\n    NOTE: this file is NEVER collected by pytest — filename does not match\n    test_*.py or *_test.py due to the \"uneeded\" typo. The stress test silently\n    does not run in CI.\n\n  ---\n  Executive Summary\n\n  - Readiness: Close\n  - RACE findings:  1 — one confirmed use-after-realloc race in the buffer protocol\n  - UNSAFE findings: 0\n  - PROTECT findings: 0\n  - MIGRATE findings: 1\n\n  The C extension is in strong shape. Commit af45640 correctly applied\n  @cython.critical_section to all 17 public Python-visible methods across\n  Packer and Unpacker. The only gap is two buffer-protocol slots\n  (__getbuffer__ and __releasebuffer__) that were missed in the same sweep.\n  Without that decorator, any thread calling memoryview(packer) concurrently\n  with pack() can trigger a use-after-realloc on the raw pk.buf pointer.\n\n  Verified fix: @cython.critical_section compiles on both __getbuffer__ and\n  __releasebuffer__ in Cython 3.2.5 — the fix is two decorator lines.\n\n  Two tensions are worth explicit attention: (1) freethreading_compatible = True\n  is declared on the C extension module only; the pure-Python fallback\n  (msgpack/fallback.py) carries zero synchronization, is the silent drop-in\n  on PyPy and under MSGPACK_PUREPYTHON=1, and has no thread-safety\n  documentation. (2) The CI stress test that should validate this is never\n  collected by pytest due to a filename typo, weakening the \"FT tests pass\"\n  signal.\n\n  ---\n  Findings by Priority\n\n  ───────────────────────────────────────────────────────────────────────────────\n  RACE Findings (fix immediately) — 1\n  ───────────────────────────────────────────────────────────────────────────────\n\n  #: 1\n  Finding: Packer.__getbuffer__ and __releasebuffer__ missing\n    @cython.critical_section — two independent races:\n\n    Race A (memory safety): __getbuffer__ reads self.pk.buf and self.pk.length\n    via PyBuffer_FillInfo, then increments self.exports — all without holding\n    the per-object lock. Concurrently, pack() holds that lock, reads\n    self.exports via _check_exports(), and if it sees exports == 0 (possible\n    due to Race B below), calls msgpack_pack_write → PyMem_Realloc(pk.buf).\n    The consumer of the Py_buffer now holds a pointer to freed-or-moved memory.\n\n    Race B (counter integrity): self.exports is a cdef size_t that is\n    incremented in __getbuffer__ and decremented in __releasebuffer__ without\n    any lock or atomic operation. Two concurrent memoryview(packer) calls or\n    concurrent releases race on a plain read-modify-write. A lost update\n    makes the counter negative or zero, defeating the _check_exports() guard\n    that serializes pack/reset against live buffer exports.\n\n    Sub-note: the public Packer.getbuffer() method (line 356) also lacks\n    @cython.critical_section. It calls memoryview(self) which triggers the\n    slot directly. Decorating getbuffer() alone is not sufficient (any caller\n    of memoryview() bypasses it), but it was missed in the same af45640 sweep.\n\n  File:Line: _packer.pyx:363-368 (slots); _packer.pyx:356 (getbuffer())\n  Severity: CRITICAL\n  Agents: shared-state-auditor, ft-history-analyzer, lock-discipline-checker,\n          atomic-candidate-finder, unsafe-api-detector (5-agent cross-validation)\n\n  Fix (verified to compile in Cython 3.2.5):\n\n    # _packer.pyx:362-368 — add two decorators\n    @cython.critical_section\n    def __getbuffer__(self, Py_buffer *buffer, int flags):\n        PyBuffer_FillInfo(buffer, self, self.pk.buf, self.pk.length, 1, flags)\n        self.exports += 1\n\n    @cython.critical_section\n    def __releasebuffer__(self, Py_buffer *buffer):\n        self.exports -= 1\n\n    # _packer.pyx:355 — add decorator to public method as defence-in-depth\n    @cython.critical_section\n    def getbuffer(self):\n        return memoryview(self)\n\n  Note: the decorator emits Py_BEGIN_CRITICAL_SECTION(self) /\n  Py_END_CRITICAL_SECTION() in the generated C, using the same per-object\n  mutex that pack(), reset(), etc. already hold. This makes the\n  exports-check-then-realloc in pack() atomic with respect to\n  buffer-export-then-increment in __getbuffer__. The lock will contend with\n  pack() — this is correct and intentional.\n\n  ───────────────────────────────────────────────────────────────────────────────\n  MIGRATE Findings (structural changes) — 1\n  ───────────────────────────────────────────────────────────────────────────────\n\n  #: 2\n  Finding: PyUnicode_InternInPlace called on every map key from untrusted\n    msgpack data (unpack.h:191-193). The API is deprecated since Python 3.12\n    in favour of PyUnicode_InternImmortal. Under free-threading CPython 3.13+\n    the intern table is internally locked so there is no undefined behavior,\n    but the lock contention is proportional to key throughput across all\n    concurrent unpackers. The documented replacement is available at Python\n    ≥3.10 (this project's minimum target).\n\n  File:Line: unpack.h:191-193\n  Severity: MEDIUM\n  Agent: unsafe-api-detector\n\n  Fix:\n    // unpack.h:191-193 — replace\n    if (PyUnicode_CheckExact(k)) {\n        PyUnicode_InternImmortal(\u0026k);   // was: PyUnicode_InternInPlace(\u0026k)\n    }\n\n  Note: PyUnicode_InternImmortal is explicitly documented as thread-safe and\n  avoids the intern-table lock contention under heavy parallel unpack load.\n  It marks strings as immortal, which has a slight memory-use implication\n  but matches the observed usage here (map keys are typically short-lived\n  keys not intended to be GC'd individually anyway).\n\n  ---\n  Tensions\n\n  Tension 1: freethreading_compatible on C extension, but not fallback.py\n\n  freethreading_compatible = True is declared on the C extension module only.\n  msgpack/fallback.py — the silent drop-in on PyPy and under\n  MSGPACK_PUREPYTHON=1 — uses no threading primitives at all. No Lock, no\n  RLock, no synchronization around any mutable Packer or Unpacker instance\n  state. The CI addition in 9de2fd9 does run fallback.py under\n  --parallel-threads=auto --iterations=20, but that test harness creates a\n  separate instance per test function (it parallelises tests, not shared-\n  object calls), so it does not exercise shared-instance races.\n\n  A user on PyPy who sees freethreading_compatible = True (either from the\n  wheel metadata or from msgpack's documentation) has no indication that the\n  fallback they are running is unprotected.\n\n  Options:\n    A. Add an explicit class-level or module-level note to fallback.py\n       documenting \"not thread-safe when instances are shared across threads.\"\n    B. Add threading.RLock protection to Packer and Unpacker in fallback.py,\n       mirroring the @cython.critical_section pattern from the C extension.\n    C. Raise an explicit RuntimeError or warning when fallback.py is loaded\n       under a free-threaded Python build (PYTHON_GIL=0 / sys._is_gil_enabled()\n       returns False).\n\n  Tension 2: CI stress test never runs\n\n  test/uneeded_test_multithreading.py contains a genuine multi-threaded\n  stress test for Packer.pack under concurrent threads. It is never collected\n  by pytest because its filename does not match pytest's default discovery\n  patterns (test_*.py / *_test.py). The \"uneeded\" typo is also present in\n  the filename itself.\n\n  The CI run in 9de2fd9 therefore validates that parallel *test functions*\n  (each with their own Packer/Unpacker) pass — not that a single shared\n  Packer instance is safe under concurrent writes.\n\n  Fix: rename to test_multithreading.py and add a pytest mark so it is\n  included in the FT CI matrix but skipped on non-FT builds:\n\n    # test_multithreading.py\n    import sys, pytest\n    pytestmark = pytest.mark.skipif(\n        sys.version_info \u003c (3, 13) or getattr(sys, \"_is_gil_enabled\", lambda: True)(),\n        reason=\"free-threaded Python only\"\n    )\n\n  ---\n  SAFE Patterns (confirmed thread-safe)\n\n  - Per-instance Packer state (pk.buf, pk.length, pk.buf_size, use_bin_type,\n    _default, strict_types, use_float, autoreset, datetime):\n    All 8 mutating methods carry @cython.critical_section. No cdef helper\n    acquires the lock; helpers are called only from within decorated callers.\n    SAFE — lock-discipline-checker confirmed zero mismatches across 17 methods.\n\n  - Per-instance Unpacker state (ctx, buf, buf_head, buf_tail, stream_offset,\n    file_like, all hook references):\n    All 9 public methods carry @cython.critical_section.\n    SAFE — lock-discipline-checker: all callers confirmed, no missed paths.\n\n  - stream_offset: written only from decorated methods (unpack, skip,\n    read_array_header, read_map_header, __next__, read_bytes).\n    SAFE — atomic-candidate-finder confirmed.\n\n  - Module-level globals (utc, epoch, giga, ExtType, Timestamp,\n    DEFAULT_RECURSE_LIMIT, ITEM_LIMIT):\n    Written exactly once during module exec, which runs under CPython's\n    import lock before any user thread can access the module. Thereafter\n    read-only. SAFE under free-threading.\n    (Subinterpreter isolation concern — these are process-global PyObject*\n    statics; see cext-analysis.txt findings 8–10 for that separate issue.)\n\n  - Hand-written C headers (pack.h, pack_template.h, unpack.h,\n    unpack_template.h, unpack_container_header.h, unpack_define.h, sysdep.h):\n    Zero file-scope static variables. All mutable state flows through\n    struct arguments (msgpack_packer *, unpack_context *). SAFE.\n\n  - unpackb() function: allocates its unpack_context on the stack.\n    No shared state. Fully thread-safe as a standalone function.\n    SAFE.\n\n  - No GIL-released regions: zero Py_BEGIN_ALLOW_THREADS, zero\n    with nogil:, zero PyGILState_Ensure/Release in the entire codebase.\n    The CRITICAL category \"unsafe API outside GIL\" has zero instances.\n\n  - No StopTheWorld needed: module init serialized by import lock;\n    GC traversal handled by CPython's own STW before any gc.collect();\n    no _PyEval_StopTheWorld usage added or needed.\n\n  ---\n  Recommended Action Plan\n\n  Immediate (RACE)\n\n  1. Fix Finding 1 — Add @cython.critical_section to __getbuffer__,\n     __releasebuffer__, and getbuffer() in _packer.pyx:355-368. Three\n     one-line additions. Verified to compile in Cython 3.2.5. Run make cython\n     after editing to regenerate _cmsgpack.c, then rebuild and run:\n       pytest -v test/ --parallel-threads=auto --iterations=20\n     to confirm no race is detectable.\n\n  Immediate (CI infrastructure)\n\n  2. Rename test/uneeded_test_multithreading.py → test/test_multithreading.py\n     and add the skipif mark shown above. This ensures the concurrent-pack\n     stress test actually runs in the CI matrix that 9de2fd9 added.\n\n  Short-term (MIGRATE)\n\n  3. Fix Finding 2 — Replace PyUnicode_InternInPlace with\n     PyUnicode_InternImmortal in unpack.h:191-193. One-line change.\n\n  Longer-term (Tensions)\n\n  4. Decide and document the fallback.py thread-safety posture. Either add\n     threading.RLock protection (mirroring the C extension's pattern) or add\n     explicit \"not thread-safe for shared instances\" documentation and a\n     runtime warning when loaded under a GIL-disabled interpreter.\n\n  5. Once the C extension subinterpreter migration is completed (see\n     cext-analysis.txt action 14 — CYTHON_USE_MODULE_STATE=1), the\n     freethreading_compatible + multiple_interpreters story becomes fully\n     consistent.\n\n  ---\n  Cross-reference\n\n  The subinterpreter safety findings (module-level PyObject* globals,\n  PyDateTimeAPI cross-interpreter, Py_mod_multiple_interpreters slot)\n  are covered separately in cext-analysis.txt findings 8-10 and P3.\n  They are FT-safe and are not repeated here.\n\n```","author":{"url":"https://github.com/clin1234","@type":"Person","name":"clin1234"},"datePublished":"2026-06-02T16:33:07.000Z","interactionStatistic":{"@type":"InteractionCounter","interactionType":"https://schema.org/CommentAction","userInteractionCount":1},"url":"https://github.com/683/msgpack-python/issues/683"}