fix: synchronize before the pre-broadcast empty_cache - #3383
Merged
Conversation
empty_cache returns cached blocks to the driver, so a kernel still in flight that references a cached block faults with an illegal memory access once the block is freed under it. The pre-broadcast empty_cache runs right after optimizer.step(); optimizers with long kernel tails (SignSGD's per-param loop) deterministically crashed after step 1 under FSDP2 + CP, while AdamW's compact foreach step happened to finish in time. Drain pending work before releasing blocks.
samsja
approved these changes
Aug 25, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
CUDA error: an illegal memory accessthat killed every SignSGD run without full optimizer offload since feat: overlap CPU optimizer with backward #3234.torch.cuda.empty_cache()returns cached blocks to the driver; a kernel still in flight that references a cached block faults once its block is freed under it. The pre-broadcastempty_cache(added in feat: overlap CPU optimizer with backward #3234 for FP8 gather headroom) runs immediately afteroptimizer.step()— SignSGD's per-param loop leaves thousands of tiny kernels still running at that point, while AdamW's compact foreach step happens to finish in time, which is why only SignSGD crashed. Draining pending work withtorch.cuda.synchronize()before releasing blocks closes the race for every optimizer, at ~ms cost once per step.Verification
All on the same GLM-4.5-Air scaleswe config (2 trainer + 2 inference nodes,
sign_sgd+optim_cpu_offload, no full offload):CUDA_LAUNCH_BLOCKING=1Supersedes #3373 — its foreach SignSGD rewrite turned out to be unnecessary for correctness (verified by running the untouched optimizer with only this line).
🤖 Generated with Claude Code
Note
Medium Risk
Touches the per-step RL training hot path and GPU memory lifecycle before weight broadcast; the change is narrow and matches existing synchronize-before-empty_cache patterns elsewhere, but any extra sync can affect step timing.
Overview
Fixes a CUDA illegal memory access that could occur right after
optimizer.step()when the trainer frees GPU cache before broadcasting weights.The pre-broadcast
torch.cuda.empty_cache()(for FP8 gather headroom) can return cached blocks to the driver while optimizer kernels are still in flight—especially with SignSGD’s many small per-parameter updates. The change drains pending GPU work viatorch.cuda.synchronize()immediately beforeempty_cache(), with expanded comments explaining the race.This is a one-line behavioral fix in the RL training loop’s weight-broadcast path; it does not change broadcast semantics, only ordering so memory is not reclaimed under active kernels.
Reviewed by Cursor Bugbot for commit 8947aa4. Bugbot is set up for automated code reviews on this repo. Configure here.