Skip to content

Umbrella issues with free-threading and Cython bindings #2416

Description

@clin1234

(Global, non-restarting numbering. "Agents/methods" shows cross-validation: SS=shared-state,
AT=atomics, UA=unsafe-apis, LD=lock-discipline, MAN=manual review, HIST=git history.)

RACE Findings (fix immediately) — 2

# Finding File:Line Severity Agents/methods
1 Global std::map m_feeders mutated with no lockfeed() calls .find() while populate_feeders() calls operator[]. Two threads launching distinct kernels concurrently corrupt the map (rehash during concurrent insert → heap corruption). Violates the "distinct objects cannot corrupt the interpreter" guarantee. param_packer.h:62 (decl), :135 (find), :138 (insert) CRITICAL SS, AT, UA, MAN
2 Unsynchronized lazy-init of 18 ctypes globalsfeed() guard if (ctypes_c_int==nullptr) fetch_ctypes(); lets two threads both run fetch_ctypes(), tearing writes to ctypes_module + 17 ctypes_c_* pointers and leaking a strong ref from PyImport_ImportModule. Also violates the codebase's own documented static-init/GIL deadlock rule if "fixed" naively with a magic static. param_packer.h:133 (guard), :11-29 (globals), :31-58 (fetch_ctypes) HIGH SS, AT, MAN

Reachability (verified — Findings 1 & 2 are live, not theoretical): param_packer.pxd:7 exposes
int feed(...); it is called from the kernel-launch parameter-packing loop at
cuda_bindings/cuda/bindings/_lib/utils.pxi:89
(size = param_packer.feed(self._ckernelParams[idx], value, ctype)); the module is Cythonized with
freethreading_compatible=True (cuda_bindings/build_hooks.py:227Py_MOD_GIL_NOT_USED), and no
lock is held around feed(). Launching kernels from multiple host threads (a normal CUDA multi-stream
pattern) therefore drives concurrent feed() calls with no GIL serialization → the races are live in
the shipped …-cpython-315t wheels. Structural aggravator: param_packer.pxd states the header "gets
compiled into every Cython extension module that depends on param_packer.pxd," so its static state
exists as independent racy copies in each consuming module (e.g. the driver/runtime kernel-launch
extensions), not one shared instance.

Dedup note (Finding 2): the shared-state scanner reported the 17 ctypes_c_* pointers as 19
static_type_object / MIGRATE items. That label is imprecise — these are cached pointers to
ctypes' type objects
, not user-defined static PyTypeObject instances needing heap-type conversion.
The real defect is unsynchronized lazy-init of shared state, so all 19 are collapsed here into
Finding 2 (and the separate MIGRATE section is consequently empty).

UNSAFE Findings (fix before declaring free-threading support) — 1

# Finding File:Line Severity
3 Borrowed references cached long-term + module ref leaked. PyDict_GetItemString (:41-57) returns borrowed refs cast to PyTypeObject* and cached for process lifetime; PyImport_ImportModule (:33) stores a strong ref in ctypes_module that is never released. Safe only while ctypes stays imported; under free-threading this compounds with Finding 2 (the racy lazy path can leak/duplicate the module ref). param_packer.h:33, :41-57 MEDIUM

PROTECT Findings (add synchronization / verify) — 3

# Finding File:Line Severity
4 mr_dealloc_cb is a non-atomic global function pointer — set by register_mr_dealloc_callback (:1036), read/called from shared_ptr deleters on arbitrary threads (:1054-1055). Safe iff registered exactly once at import (before any MR-backed device pointer exists) — verify. Best practice under FT: make it std::atomic<MRDeallocCallback> for a well-formed happens-before. resource_handles.cpp:1033 LOW–MEDIUM
5 GraphNodeBox::slot_table is a mutable, lazily-created cache (:1313, populated :1358). If any read-path accessor triggers the lazy creation, two concurrent reads of the same graph node race on it — breaking the "concurrent reads are safe" guarantee. If it is only populated while mutating the graph (adding nodes — explicitly out-of-contract), it is policy-permitted. Verify the trigger path; if read-reachable, guard with once-init/atomic. resource_handles.cpp:1313 LOW–MEDIUM
6 initialize_deferred_cleanup uses check-then-act, not CASif (queue.load(acquire)) return; … queue.store(release) (memory ordering is correct). Safe iff invoked once at module init; if reachable concurrently, two threads each allocate a DeferredCleanupQueue (one leaks). Verify single-init at import. resource_handles.cpp:334-340 LOW

From https://gist.github.com/clin1234/6cdb5150ba3298576ca54fa07c017517#file-cuda-python-md

Must Fix (FIX) — 2

# Finding File:Line Agents/Evidence
1 C++ exception across a non-except+ Cython boundary → std::terminate. param_packer.pxd declares int feed(void*, object, object) with no except +; Cython docs (verified via context7): extern cdef functions are implicitly noexcept — no try/catch is generated. Yet fetch_ctypes() (called from feed() on first use) deliberately throws std::runtime_error when import ctypes or its dict lookup fails (param_packer.h:35,39), and m_feeders[...]/std::function construction can throw std::bad_alloc. Any such throw unwinds into Cython-generated frames with no handler → std::terminate() → hard interpreter abort (during e.g. interpreter finalization, embedded/restricted envs, or OOM) instead of a Python exception. The project's own _resource_handles.pxd:173,225 shows the correct in-repo idiom (except+ nogil). Fix (1 line): declare feed as except + — or make feed noexcept by catching in C++ and returning -1. Note: on the throw path the pending ImportError from PyImport_ImportModule is left set; with except + translation it would be replaced by RuntimeError — prefer catching in C++ and letting the original Python error propagate. param_packer.pxd:7; param_packer.h:33-39,62,70 error-path scanner (adjacent hit) + manual + context7 /cython/cython
2 param_packer.h shared-state races under free-threading (live in shipped 3.15t wheels): unsynchronized std::map m_feeders read/write and racy lazy-init of 18 static ctypes globals, plus the never-released ctypes_module strong ref and long-term caching of PyDict_GetItemString borrowed refs. This run's scanners re-confirmed via 37 scan_module_state hits + 3 scan_gil_usage hits + 1 scan_refcounts potential_leak (:33) — all deduplicated into this one finding. Full analysis, interleavings, and the PyMutex-vs-std::mutex deadlock discussion: FT report Findings 1–3. Fix: populate the ctypes pointers and the (finite, 6-pair) feeder table once at module import; feed()'s existing size == 0 ctypes fallback (utils.pxi:91-95) makes prepopulation behavior-neutral. param_packer.h:11-29,62,133-138 module-state (37) + gil (3) + refcounts (1) scanners + FT report

Should Consider (CONSIDER) — 4

# Finding File:Line
3 _ref creator asymmetry breaks the registry design contract on the miss path. create_context_handle_ref and create_stream_handle_ref register their boxes (with unregistering deleters); create_event_handle_ref and create_kernel_handle_ref look up but never register, and their miss-path boxes are dependency-less: KernelBox{kernel, {}} drops the library dependency REGISTRY_DESIGN.md says KernelBox exists to carry; EventBox{event, false,false,false,-1, {}} drops the context dependency and zeroes timing/IPC metadata. Reachable from graph round-trips (graph/_subclasses.pyx:121,511,549): after the original owned handle dies, a round-tripped event misreports timing_enabled=false, and a round-tripped kernel no longer keeps its library loaded. Mitigations found: registry-hit path preserves everything; Kernel.from_handle (_module.pyx:538-556) explicitly accepts mod to re-supply library lifetime — the API anticipates the gap. Recommendation: align the four creators (either register miss-boxes for identity dedup, or document why events/kernels deliberately don't cache degraded boxes) and state the degraded-metadata contract in REGISTRY_DESIGN.md. resource_handles.cpp:769-775, 1262-1268 vs :431-447, :622-635
4 Tautological PyCapsule validation in the driver-API capsule extraction. PyCapsule_GetPointer(capsule, PyCapsule_GetName(capsule)) passes the capsule's own name as the expected name — the check can never fail, so a cuda_core wheel paired with an ABI-drifted cuda_bindings wheel casts a mismatched function pointer silently (call-time UB instead of import-time error). DESIGN.md documents the convenience rationale ("query it rather than hardcoding signatures"), so this is a known trade-off — but comparing against the expected signature string (even just in debug builds) would turn silent drift into a clean import failure. Same pattern at the cynvrtc/cynvvm/cynvjitlink extraction sites. _resource_handles.pyx:352-353, 358-361, 472-473, 489-490, 506+
5 Advertised-but-unimplemented surfaces (parity). Buffer.__buffer__ / __release_buffer__ raise NotImplementedError("WIP…") — defining the slots makes memoryview(buf) fail with NotImplementedError rather than TypeError, and stubs/docs imply buffer support; also _memoryview.copy_from (:446), a texture path (texture/_texture.pyx:521), managed-memory ops on unsupported platforms, and win32 virtual memory (_virtual_memory_resource.py:135). Track as explicit WIP or remove the slots until implemented. _memory/_buffer.pyx:436,440 et al.
6 Maintainer-flagged open threads from #2162 (commit message, Sebastian Berg): (a) SMResource split-mutex — fix added self._split_mutex but the author records residual uncertainty ("This is thread-unsafe. But I am honestly not sure…") about CUDA-internal safety; (b) explicit TSan gap: "Testing with thread-sanitizer might flush out some…"; (c) cuda-pathfinder's load_nvidia_dynamic_lib functools.cache flagged for review. These are the maintainers' own TODOs — carrying them here so they don't get lost. _device_resources.pyx:498-505; pathfinder

Tensions

  • Register-on-miss vs. don't-cache-degraded-boxes (Finding 3): registering event/kernel miss-boxes would restore identity-map dedup but would cache wrong metadata that later lookups would trust; not registering (current behavior) creates a fresh degraded box per round-trip but leaves the registry clean for a future owned registration (map_[key] = h overwrites). Neither is strictly better — which is exactly why the choice should be written down; today the code silently disagrees with REGISTRY_DESIGN.md's stated purpose.
  • Capsule convenience vs. ABI safety (Finding 4): the tautological name check is what lets cuda_core avoid hardcoding cydriver's signature strings; fixing it costs a maintained signature table. Debug-only validation is the compromise.
  • cppcheck style vs. C++ idiom: cppcheck wants constructors on EventBox (4 warnings); the codebase's aggregate-init-only style is safer than a constructor here (verified all 3 sites fully initialize). Dismissed rather than "fixed."

Policy Decisions (POLICY) — 3

# Finding
7 abi3/Limited API is not claimed and should not be pursued now. The Cython layer + per-version wheels (incl. cp315t) are incompatible with today's limited API anyway (free-threaded builds have no stable ABI yet); revisit only if wheel-matrix cost forces it.
8 Subinterpreters are unsupported by design. Process-wide C++ singletons (six HandleRegistry instances, DeferredCleanupQueue, driver fn-pointer capsules) are shared across interpreters; per-interpreter isolation would require moving them into per-module state. No action unless subinterpreter support becomes a goal.
9 Code-removal opportunities: none at the current >=3.10 floor. When the floor reaches 3.13/3.14: PyDict_GetItemRef (3.13) and PyImport_ImportModuleAttrString (3.14) replace exactly param_packer.h's PyImport_ImportModule + PyDict_GetItemString-borrowed-ref chain (~5-8 lines saved and the borrowed-ref hazard of Finding 2 eliminated structurally). PyModule_AddType (3.10) is already available but the project defines no hand-written types.

From https://gist.github.com/clin1234/2469758160721f4f991110167017e4d3#file-cuda-python-md

Metadata

Metadata

Assignees

Labels

triageNeeds the team's attention

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions