Refactor graphics interop around RAII-mapped buffers

[rparolin]

Summary

• Reworks graphics interop around the existing handle/RAII pattern instead of making GraphicsResource inherit from Buffer
• Keeps GraphicsResource responsible for registration/unregistration only; map() now returns a mapped Buffer
• Adds a mapped-graphics device pointer handle whose deleter captures both the GraphicsResourceHandle and StreamHandle and calls cuGraphicsUnmapResources(...) on release
• Preserves with resource.map(stream=s) as buf: by giving Buffer context-manager support
• Supports both with GraphicsResource.from_gl_buffer(vbo) as resource: for automatic resource cleanup and with GraphicsResource.from_gl_buffer(vbo, stream=s) as buf: for register-and-map-in-one-step usage
• Restores default-stream compatibility for map() / unmap() and keeps GraphicsResource.handle as the raw CUgraphicsResource handle

Motivation

Review feedback pointed toward Andy Jost's resource_handles direction rather than turning GraphicsResource into a mutable Buffer.

This version follows the same ownership model used elsewhere in cuda_core: the mapped pointer owns unmap lifetime, and the resource registration owns unregister lifetime. That keeps stream-ordered cleanup in the handle layer, avoids changing GraphicsResource.handle semantics based on mapping state, and removes the need to treat an unmapped resource as a partially valid Buffer.

Key changes

• Handle-based mapped buffers: map() creates a real Buffer from a new mapped-graphics device pointer handle. The handle deleter captures the graphics resource and mapping stream, and unmaps on release.
• GraphicsResource stays a resource owner: GraphicsResource no longer inherits from Buffer; it tracks registration state, forwards unmap() / close() to the active mapped buffer, and keeps a strong reference to that buffer so is_mapped, unmap(), and double-map protection remain correct even if callers drop their own Python reference.
• Context-manager behavior:
  • with resource.map(stream=s) as buf: still works and automatically unmaps on exit.
  • with GraphicsResource.from_gl_buffer(vbo, stream=s) as buf: registers, maps on entry, and unregisters on exit.
  • with GraphicsResource.from_gl_buffer(vbo) as resource: scopes the registration lifetime and closes automatically on exit, avoiding a manual resource.close().
• Compatibility cleanup: map() / unmap() again accept stream=None and use the default stream, and GraphicsResource.handle once again exposes the raw graphics-resource handle.

[copy-pr-bot]

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

[copy-pr-bot]

Auto-sync is disabled for ready for review pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

[rparolin]

/ok to test

[github-actions]

Doc Preview CI
🚀 View preview at https://nvidia.github.io/cuda-python/pr-preview/pr-1701/
https://nvidia.github.io/cuda-python/pr-preview/pr-1701/cuda-core/
https://nvidia.github.io/cuda-python/pr-preview/pr-1701/cuda-bindings/
https://nvidia.github.io/cuda-python/pr-preview/pr-1701/cuda-pathfinder/
Preview will be ready when the GitHub Pages deployment is complete.

[rparolin]

/ok to test

[cpcloud]

Thanks for the refactor here — the GraphicsResource(Buffer) direction looks good overall.

Requesting changes for two behavior issues called out inline:

1. close() currently unmaps on stream 0 even when mapping happened on a non-default stream, which can break stream-ordering guarantees.
2. __enter__ returns self even when unmapped, allowing context-manager usage where handle/size are invalid.

Please address these and I’m happy to re-review.

[cpcloud]

ping @rparolin. happy to fix this up if you want.

[leofang]

Thanks for the refactor here — the GraphicsResource(Buffer) direction looks good overall.

Yeah and thinking about it more, I suspect this is a must:

• If GL buffer registration is context-independent: This PR makes the UX nicer
• If GL buffer registration is locked to the current CUDA context: We need to track the underlying device/context, so we either inherit from Buffer (this PR) or add Buffer.from_gl_buffer() constructor (and keep GraphicsResource internal).
  • From the viewpoint of staying consistent across cuda.core, the latter might be worth considering too, but one challenge is that Buffer today does not have the map/unmap concept.
  • I would be happy for you @rparolin @Andy-Jost @cpcloud to brainstorm and I can review later 🙂

I will ask internally if it's context-independent or not, but it's not a blocker.

[Andy-Jost]

Thanks for the discussion here. I'd like to propose an alternative approach that I think addresses both of Phillip's concerns and Leo's observation about Buffer not having map/unmap.

Proposal: Use the resource_handles module instead of modifying Buffer

We already have a C++ handle layer (cuda/core/_cpp/resource_handles.hpp) that provides RAII-based lifetime management with std::shared_ptr custom deleters. This pattern already solves the exact problems raised here:

1. Stream capture: DevicePtrHandle already captures the allocation stream in its deleter and uses it for cuMemFreeAsync. We can do the same for graphics resources - capture the map stream and use it for unmap.

2. Dependency tracking: Handles can hold references to other handles, preventing out-of-order destruction. A mapped buffer handle can hold a reference to the GraphicsResourceHandle, ensuring the graphics resource outlives the mapped pointer.

3. Arbitrary release actions: The shared_ptr deleter can chain actions - e.g., unmap then unregister.

Concrete design:

• GraphicsResource.map(stream) returns a Buffer (or just the existing DevicePtrHandle under the hood)
• The returned handle's deleter captures:
  • The GraphicsResourceHandle (keeps it alive)
  • The map StreamHandle (for stream-ordered unmap)
• When the mapped buffer handle is released, the deleter calls cuGraphicsUnmapResources on the captured stream
• GraphicsResource itself doesn't need to track _mapped state - the existence of mapped buffer handles implies mapped state

Benefits:

• Buffer stays unchanged (no map/unmap concept needed)
• Stream ordering is correct by construction (cpcloud's concern 1)
• No invalid __enter__ state possible (cpcloud's concern 2) - you either have a valid mapped handle or you don't
• Dependency tracking prevents use-after-free
• Consistent with how we handle DeviceMemoryResource allocations

This is the same pattern we use for memory pool allocations, where the pointer handle captures the pool handle and stream for proper cleanup ordering.

Thoughts?

[cpcloud]

+1 to @Andy-Jost's design. Looks solid.

[leofang]

FYI

I will ask internally if it's context-independent or not

it is context-dependent.

Proposal: Use the resource_handles module instead of modifying Buffer

Modifying Buffer in what way? Sorry it's not clear to me.

Can we sketch a bit how the user code would look like with this proposal? The changes in this PR were motivated by the sketch in here: #1608 (comment). Rob asked the right question there. Inheritance is probably not necessary, as long as the same UX can be delivered.

[cpcloud]

The GraphicsResource(Buffer) direction still looks good overall, but I can’t approve this yet because two blocking behavior issues remain in the cleanup/context-manager paths called out inline: close() still ignores the mapping stream, and __enter__() still allows unmapped context-manager usage. I also reran pixi run pytest tests/test_graphics.py -q locally; it passes, but the current test coverage does not catch either case, so please add targeted regressions when you fix them.

[cpcloud]

@rparolin Do you want to fire off an agent to implement @Andy-Jost's design? Probably could make pretty short work of it.

[rparolin]

@cpcloud made the requested changes and aligned with @Andy-Jost recommendations. Please re-review. Thanks!

[cpcloud]

Re-reviewed the latest graphics follow-up changes. The earlier context-manager and stream-ordering concerns look addressed, and passes in my environment.

[rparolin]

Re-reviewed the latest graphics follow-up changes. The earlier context-manager and stream-ordering concerns look addressed, and passes in my environment.

Thanks!