Fix primary context cleanup during Python shutdown - #2744
Conversation
|
I do not have permission to set metadata on the upstream repository. Suggested PR metadata: labels |
This comment has been minimized.
This comment has been minimized.
|
|
||
|
|
||
| @pytest.mark.agent_authored(model="gpt-5.6-sol") | ||
| def test_primary_context_cleanup_is_safe_during_python_shutdown(init_cuda, tmp_path): |
There was a problem hiding this comment.
This test will only reproduce the failure I saw if it is run with cuda-bindings <=12.9.2 or cuda-bindings >=13.0.0,<=13.0.1. However, it is good to include to prevent regression.
d0bd2ba to
56777be
Compare
seberg
left a comment
There was a problem hiding this comment.
Thanks Bradley.
Surprising to me that these are cleaned up. The pattern is common, either way.
(Whether this should PrintUnraisable is a seperate issue.)
56777be to
3694167
Compare
|
Backport failed for Please cherry-pick the changes locally and resolve any conflicts. git fetch origin 12.9.x
git worktree add -d .worktree/backport-2744-to-12.9.x origin/12.9.x
cd .worktree/backport-2744-to-12.9.x
git switch --create backport-2744-to-12.9.x
git cherry-pick -x e74d4d557d6a436d5064c71480a23c3837699f41 |
|
|
Ah, nvm. This code didn't exist at all in 12.9, so no backport. |
| [device_id](const ContextBox* b) { | ||
| context_registry.unregister_handle(b->resource); | ||
| GILReleaseGuard gil; | ||
| p_cuDevicePrimaryCtxRelease(device_id); |
There was a problem hiding this comment.
@Andy-Jost Would be nice to check in which PR we added this. I am 100% certain that this goes against cuda.core assumptions. We can't afford primary context from being destroyed or reset. Same as in cccl-rt, we cache a ton of things from the primary context, which would become dangling ptrs if the context is gone.
Summary
cuDevicePrimaryCtxReleaseduring normal thread cleanup while leaving process-exit cleanup to CUDA/the OSDevice().set_current()followed by interpreter shutdownFixes #2743.
Root cause
get_primary_context()caches aContextHandlein thread-local storage. When the main thread's TLS destructors run afterPy_Finalize(), the handle deleter callsp_cuDevicePrimaryCtxRelease. That pointer targetscuda.bindings.cydriver.__pyx_capi__["cuDevicePrimaryCtxRelease"], whose generated Cython entry path touches Python thread state. The result is a post-atexitsegfault inPyThreadState_New.The existing
GILReleaseGuardavoids manipulating the GIL after finalization, but the Cython driver wrapper invoked immediately afterward has the same Python-runtime requirement. The new guard skips that wrapper only when Python is no longer usable.Testing
pixi run --manifest-path cuda_core -e cu12 python -m pytest cuda_core/tests/test_device.py -qpython -c 'from cuda.core import Device; Device().set_current(); print("initialized")'