Found by:claude-jitfields-to-fastfields, during the #5 CUDA mesh work.
Every CUDA launcher in include/fastfields/impl/cuda/ frees its device buffers immediately after an asynchronous kernel launch, and sdt / sdt_naive additionally do a synchronous default-stream D2H copy of a buffer written by a kernel running on the caller's stream. Nothing orders either operation explicitly.
Both are correct today only by accident of implementation: plain cudaFree / cudaFreeHost synchronise the entire device, which happens to drain the caller's stream first.
Why this is fragile rather than merely ugly
The correctness rests on two assumptions that are already false in one important consumer:
- Legacy default-stream synchronisation. The implicit ordering between the caller's stream and the default stream holds for the legacy default stream. PyTorch creates its streams with
cudaStreamNonBlocking, which by design does not synchronise against the legacy default stream. A caller passing such a stream can observe the D2H copy reading a buffer the kernel has not finished writing. cudaFree being a full-device barrier. Any move to cudaFreeAsync / stream-ordered allocation — a natural optimisation, and what a memory pool would want — silently removes the barrier that is currently doing the ordering. The frees would then race the kernels that are still reading those buffers.
Neither failure is loud. Both produce intermittently wrong numbers, not crashes.
Why CI cannot catch it
Same structural blind spot as the vertex-normal bug fixed in #86: there is no GPU in CI, so the CUDA path is validated by compile+link only, and stream ordering is invisible to a compiler. The standing argument that "the voxelwise math is shared with the CPU backend and is covered by the CPU suite" does not reach here at all — stream and allocation lifetime management is CUDA-only glue with no CPU counterpart to inherit correctness from.
Suggested direction
- Make the ordering explicit rather than incidental: synchronise on the caller's stream (or use stream-ordered copies) before freeing, and use
cudaMemcpyAsync on the caller's stream plus an explicit sync instead of a synchronous default-stream copy. - Add a test that passes a
cudaStreamNonBlocking stream, for whenever real hardware is available — that is the configuration that would expose it.
Related
Found by:
claude-jitfields-to-fastfields, during the #5 CUDA mesh work.Every CUDA launcher in
include/fastfields/impl/cuda/frees its device buffers immediately after an asynchronous kernel launch, andsdt/sdt_naiveadditionally do a synchronous default-stream D2H copy of a buffer written by a kernel running on the caller's stream. Nothing orders either operation explicitly.Both are correct today only by accident of implementation: plain
cudaFree/cudaFreeHostsynchronise the entire device, which happens to drain the caller's stream first.Why this is fragile rather than merely ugly
The correctness rests on two assumptions that are already false in one important consumer:
cudaStreamNonBlocking, which by design does not synchronise against the legacy default stream. A caller passing such a stream can observe the D2H copy reading a buffer the kernel has not finished writing.cudaFreebeing a full-device barrier. Any move tocudaFreeAsync/ stream-ordered allocation — a natural optimisation, and what a memory pool would want — silently removes the barrier that is currently doing the ordering. The frees would then race the kernels that are still reading those buffers.Neither failure is loud. Both produce intermittently wrong numbers, not crashes.
Why CI cannot catch it
Same structural blind spot as the vertex-normal bug fixed in #86: there is no GPU in CI, so the CUDA path is validated by compile+link only, and stream ordering is invisible to a compiler. The standing argument that "the voxelwise math is shared with the CPU backend and is covered by the CPU suite" does not reach here at all — stream and allocation lifetime management is CUDA-only glue with no CPU counterpart to inherit correctness from.
Suggested direction
cudaMemcpyAsyncon the caller's stream plus an explicit sync instead of a synchronous default-stream copy.cudaStreamNonBlockingstream, for whenever real hardware is available — that is the configuration that would expose it.Related
sdt) #5 remains open for the precomputed-treesdtoverload and real-hardware validation.