Fix RMM race conditions from deferred H2D copies in pylibcudf tests - #23789
Conversation
|
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. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan provides up to 12 included reviews per hour; 11 remain after this review. 📝 WalkthroughSummary by CodeRabbit
WalkthroughThe tests now materialize host byte sources, retain them during asynchronous host-to-device copies, and synchronize streams before dependent device-buffer use. ChangesAsynchronous host-to-device copy lifetime
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to This localized test-only change keeps host data alive during asynchronous device copies; no actionable merge-blocking risk remains beyond normal checks and review. Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
/merge |
Description
Split out from #23517.
DeviceBuffer.to_device(bytes, stream)reads host memory asynchronously (rapidsai/rmm#2511). If the bytes object isn't kept alive past the copy, Python can free it before the GPU reads it.The PR keeps those bytes alive through the copy by making an explicit reference. This fixes the immediate problem but it not the end of this problem. Let's use rapidsai/rmm#2521 to discuss a more general solution or at minimum document the current behavior.
I tested these changes in #23517 (which reliably reproduced the bug). And the failing tests fixed in this PR are now passing.
Checklist