Skip to content

fix(data): order GPU representation reads after writers - #182

Open
kevkrist wants to merge 1 commit into
NVIDIA:mainfrom
kevkrist:agent/hybrid-zero-copy-hash-partition
Open

kevkrist wants to merge 1 commit into
NVIDIA:mainfrom
kevkrist:agent/hybrid-zero-copy-hash-partition

Conversation

@kevkrist

@kevkrist kevkrist commented Aug 12, 2026

Copy link
Copy Markdown

Summary

This change makes GPU-table cloning safe without turning the read-only clone path into a host-blocking operation.

  • A clone stream waits for the source representation's recorded writer event before reading.
  • The source batch records the clone stream as a reader, including when cloning throws after enqueueing work, so mutation or destruction cannot overtake the copy.
  • The cloned representation records its own writer event; a consumer on another stream uses wait_until_ready() before reading it.
  • GPU representations record writer events even for the default stream and create, wait on, and destroy events under the owning CUDA device.
  • The ambiguous mutable clone() API is removed. Read-only clone() remains asynchronous, while the existing same-device mutable clone_to() compatibility path still returns a ready copy.
  • View-backed release_table() waits for producer work, materializes with the representation's allocator, and keeps the view owner alive until all queued reads finish. Its exception path drains any work that may have been enqueued before rethrowing.

Failure example: clone()

Assume a source allocation initially contains zeroes:

  1. Producer stream P is paused, with a write of 0x5a and source writer event Wsrc queued behind the pause.
  2. Clone stream C calls clone() while P is still paused.
  3. Before this fix, C could enqueue its copy without waiting for Wsrc. The copy was therefore free to read the old zeroes instead of 0x5a.
  4. Because the asynchronous copy was not registered as a source reader, releasing the read-only accessor could also let a mutable accessor overwrite or destroy the source while C was still reading it.

The fixed dependency chain is:

P: source write -> Wsrc
                    |
                    v
C:              source-to-clone copy -> Rsrc
                                          |
                                          v
                              source mutation/destruction

C: source-to-clone copy -> Wclone -> consumer-stream read

clone() can return while C is pending. Rsrc protects the source lifetime, and Wclone makes destination readiness explicit to consumers.

Failure example: release_table()

For a view-backed representation, the variant stores both a cudf::table_view and the sole shared owner of the buffers. The old implementation effectively did this:

auto view = std::get<owning_table_view>(_table).view
_table = std::make_unique<cudf::table>(view, release_stream, allocator);

Constructing the new table can enqueue asynchronous reads of view. Once the right-hand side finishes constructing, assigning it to _table destroys the variant's old view-backed alternative—and therefore its sole owner—even though release_stream may not have executed those reads yet. This is a use-after-free race.

The fixed path waits for the source writer, starts materialization, synchronizes release_stream, and only then replaces the variant alternative. If construction throws after enqueueing work, it drains the stream before unwinding so the retained owner cannot be destroyed under in-flight reads.

@copy-pr-bot

copy-pr-bot Bot commented Aug 12, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@kevkrist
kevkrist marked this pull request as ready for review August 13, 2026 14:48
@kevkrist kevkrist changed the title Order GPU representation reads after writers fix(data): order GPU representation reads after writers Aug 14, 2026
@mbrobbel

This comment was marked as resolved.

@kevkrist
kevkrist force-pushed the agent/hybrid-zero-copy-hash-partition branch from 410d697 to 24496d8 Compare September 17, 2026 16:28
@mbrobbel

Copy link
Copy Markdown
Member

/ok to test 24496d8

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants