Skip to content

Fix primary context cleanup during Python shutdown - #2744

Merged
seberg merged 1 commit into
NVIDIA:mainfrom
bdice:fix-primary-context-shutdown-segfault
Sep 18, 2026
Merged

seberg merged 1 commit into
NVIDIA:mainfrom
bdice:fix-primary-context-shutdown-segfault

Conversation

@bdice

@bdice bdice commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Summary

  • avoid calling the CUDA Bindings Cython wrapper from the primary-context TLS destructor after CPython has finalized
  • preserve cuDevicePrimaryCtxRelease during normal thread cleanup while leaving process-exit cleanup to CUDA/the OS
  • add a subprocess regression test covering Device().set_current() followed by interpreter shutdown

Fixes #2743.

Root cause

get_primary_context() caches a ContextHandle in thread-local storage. When the main thread's TLS destructors run after Py_Finalize(), the handle deleter calls p_cuDevicePrimaryCtxRelease. That pointer targets cuda.bindings.cydriver.__pyx_capi__["cuDevicePrimaryCtxRelease"], whose generated Cython entry path touches Python thread state. The result is a post-atexit segfault in PyThreadState_New.

The existing GILReleaseGuard avoids 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 -q
    • 166 passed, 2 skipped
  • minimal shutdown probe exits 0:
    • python -c 'from cuda.core import Device; Device().set_current(); print("initialized")'
  • repository pre-commit hooks pass

@github-actions github-actions Bot added the cuda.core Everything related to the cuda.core module label Aug 31, 2026
@bdice

bdice commented Aug 31, 2026

Copy link
Copy Markdown
Contributor Author

I do not have permission to set metadata on the upstream repository. Suggested PR metadata: labels bug and cuda.core; milestone cuda.core 1.2.0.

@github-actions

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):

@bdice bdice Sep 1, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@seberg seberg left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Bradley.

Surprising to me that these are cleaned up. The pattern is common, either way.

(Whether this should PrintUnraisable is a seperate issue.)

@seberg
seberg force-pushed the fix-primary-context-shutdown-segfault branch from 56777be to 3694167 Compare September 18, 2026 10:08
@seberg
seberg enabled auto-merge (squash) September 18, 2026 10:32
@seberg seberg added the to-be-backported Trigger the bot to raise a backport PR upon merge label Sep 18, 2026
@seberg seberg added this to the cuda.core 1.3.0 milestone Sep 18, 2026
@seberg seberg added the P0 High priority - Must do! label Sep 18, 2026
@seberg
seberg merged commit e74d4d5 into NVIDIA:main Sep 18, 2026
119 of 121 checks passed
@github-actions

Copy link
Copy Markdown
Contributor

Backport failed for 12.9.x, because it was unable to cherry-pick the commit(s).

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

@github-actions

Copy link
Copy Markdown
Contributor
Doc Preview CI
Preview removed because the pull request was closed or merged.

@seberg

seberg commented Sep 18, 2026

Copy link
Copy Markdown
Contributor

Ah, nvm. This code didn't exist at all in 12.9, so no backport.

@seberg seberg removed the to-be-backported Trigger the bot to raise a backport PR upon merge label Sep 18, 2026
[device_id](const ContextBox* b) {
context_registry.unregister_handle(b->resource);
GILReleaseGuard gil;
p_cuDevicePrimaryCtxRelease(device_id);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working cuda.core Everything related to the cuda.core module P0 High priority - Must do!

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG]: Primary-context TLS destructor segfaults after Python finalization

3 participants