[shoumikhin]

What is wrong today

When a CUDA program returns the same tensor more than once, for example y.detach() twice, AOTI (AOTInductor, PyTorch's ahead-of-time compiler that generates the CUDA code) puts one output handle in each of those output slots:

output_handles[0] = buf0.release();
output_handles[1] = output_handles[0];

The CUDA backend copied each output into its ExecuTorch tensor and then deleted that slot's handle. So it deleted the shared handle twice, and the process crashed, for example with:

SlimTensor storage_offset must not be negative, got -9007650542493611503
Failed to D2D copy GPU output 1 into ETensor
free(): double free detected in tcache 2

What this change does

A handle is now deleted only at the last output slot that holds it, as PyTorch's own AOTI runner does. The cleanup on error paths follows the same rule. With CUDA graphs on, a successful capture keeps every output handle for the graph and deletes none of them, so nothing changes there. If ending the capture or instantiating the graph fails, the cleanup deletes the handles by the same rule.

A comment in ReplaceViewCopyWithViewPass said that a view would make two outputs share one handle. That was misleading, because two alias_copy outputs of one tensor share one handle too, so the comment no longer says it.

What was tested

test_same_tensor_returned_twice returns one tensor in two output slots that are not next to each other, with CUDA graphs on: three eager calls, a graph capture and a replay, each compared with eager. With torch 2.14 on an A100 it passes, and it crashes without this change. The other backends/cuda tests that CI runs in the same job still pass.

[pytorch-bot]

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/24420

• 📄 Preview Python docs built from this PR

Note: Links to docs will display an error until the docs builds have been completed.

⏳ 1 Pending, 1 Unrelated Failure

As of commit dc628cc with merge base f2564fb ():

BROKEN TRUNK - The following job failed but were present on the merge base:

👉 Rebase onto the `viable/strict` branch to avoid these failures

• Test CUDA Builds / unittest-cuda (system) / linux-job (gh) (trunk failure)
  backends/cuda/tests/test_antialiased_resize.py::TestAntialiasedResizeDecomposition::test_align_corners_compiles_with_dynamic_sizes

This comment was automatically generated by Dr. CI and updates every 15 minutes.

[shoumikhin]

@claude /pr-review

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.

[claude]

Claude finished @shoumikhin's task in 15m 28s —— View job

---

PR Review: #24420

I found no blockers: the change to delete each output handle at its last slot is correct on every path I traced, and the review below has five minor follow-ups, the most notable being that the Metal backend still deletes a repeated output handle once per slot.

Detailed review

Summary

The PR makes the CUDA backend delete an output handle only at the last output slot that holds it, so a tensor that AOTI returns in several slots is no longer freed twice. I found no correctness problems in the new logic, including on the error and CUDA graph paths. The findings below are minor. They come from reading the code; no tests were run, because the runner has no GPU.

Code Quality

• The Metal backend still deletes a repeated output handle once per slot.

  • MetalBackend also derives from AotiBackend (backends/apple/metal/metal_backend.py:21). So it runs the same ReplaceViewCopyWithViewPass (backends/aoti/aoti_backend.py:303) and gets the same AOTI output codegen.
  • Its cleanup guard (backends/apple/metal/runtime/metal_backend.cpp:505-512) deletes gpu_outputs[i] for every non-null slot. For the Twice model, the second aoti_torch_delete_tensor_object call on the shared handle fails the tensors.find lookup (backends/apple/metal/runtime/shims/memory.cpp:261-263), so every execute logs Didn't find tensor.
  • It doesn't crash, because the outputs are copied back to CPU (lines 603-613) before the guard runs and the guard ignores the error code.
  • The same last-slot rule would fix it, here or in a tracked follow-up.
  • I inferred, but did not observe, that the codegen for MPS also emits output_handles[j] = output_handles[i]. The basis is that the C++ wrapper codegen does not depend on the device.

  Fix this →

• backends/aoti/passes/replace_view_copy_with_view.py:34-39: the detach_copy entry no longer says why it maps to a copy op.

  • The class docstring (line 44) says the pass replaces *_copy ops with views, and every other _VIEW_TARGETS entry does so. This entry maps to alias_copy instead.
  • The sentence this PR removes was the only stated reason not to use the view aten.alias. The comment that remains explains why the entry uses neither detach_copy nor clone, but not why it doesn't use alias.
  • If shared handles were the whole reason, map it to aten.alias.default / ops.edge.aten.alias.default like the neighbouring entries. Otherwise, write down the real reason.
  • Low confidence: I did not check whether alias passes Edge verification and lowers through Inductor.

  Fix this →

• The description's claim about the capture step is wrong on two failure paths.

  • These two returns come before slim_outputs is cleared at 1115-1120:
    • cudaStreamEndCapture failing (backends/cuda/runtime/cuda_backend.cpp:1062-1066)
    • cudaGraphInstantiate failing (1104-1108)
  • On those paths the cleanup guard does delete the capture-step outputs. With this PR it deletes each handle once instead of twice. So "the capture step … deletes none of them, so nothing changes there" doesn't hold for them. The description will likely become the commit message, so it is worth correcting.
  • Unrelated to this PR, and I'm not confident it matters: the returns at 1074 and 1089 clear every slot first, because "the outputs belong to the captured graph" (1069). It isn't clear why these two returns don't do the same.

• backends/cuda/tests/test_cuda_export.py:786-797 copies the CUDA graph setup sequence.

  • The set_option / try / finally sequence, including the comment "The option is process wide.", already exists in TestStridedOutputWithCudaGraph._run_with_cuda_graph (backends/cuda/tests/test_output_stride_rearrange.py:316-330). The other CUDA graph runtime tests are in that class.
  • That helper keeps only output 0 (:333). If it returned every output, this test could move into that class, and the process-wide option would be set and reset in one place.

  Fix this →

Testing

• No test sends a repeated handle through the cleanup guard. This one is optional.
  • The description says the error paths follow the same rule, but no test reaches the guard (cuda_backend.cpp:957-961) while it holds a repeated handle. An example: an earlier slot that shares a handle is skipped and cleared, then the D2D copy fails at a later slot holding the same handle.
  • delete_output is a lambda inside execute(), so it can't be unit-tested in its current form.
  • backends/cuda/runtime/test/test_cuda_output_copy.cpp already unit-tests helpers in backends/cuda/runtime/utils.h. If the rule moved there, next to delete_slimtensor_vector (utils.h:454), a test could cover a vector with repeated and cleared slots.

Recommendation

Approve: nothing blocks the merge. The Metal item can go in a follow-up PR. The rest are small edits to a comment, the PR description, and the test.

[shoumikhin]

@pytorchbot cherry-pick --onto release/1.6 -c critical

[pytorchbot]

Cherry picking #24420

The cherry pick PR is at #24442 and it is recommended to link a critical cherry pick PR with an issue.

Details for Dev Infra team

Raised by workflow job

[shoumikhin]

@pytorchbot cherry-pick --onto cherry-pick-24314-by-pytorch_bot_bot_ -c critical

[pytorchbot]

Cherry picking #24420

Command git -C /home/runner/work/executorch/executorch rebase origin/cherry-pick-24420-by-pytorch_bot_bot_ returned non-zero exit code 1

Rebasing (1/21)
Rebasing (2/21)
Auto-merging backends/aoti/common_shims_slim.cpp
Auto-merging backends/aoti/common_shims_slim.h
Auto-merging backends/cuda/cuda_backend.py
warning: Cannot merge binary files: backends/cuda/runtime/aoti_cuda_shims.lib (HEAD vs. a8ccc45f6a (Fix CUDA mask indexing, CUDA graph outputs and the runtime output-buffer check (#24299)))
Auto-merging backends/cuda/runtime/aoti_cuda_shims.lib
CONFLICT (content): Merge conflict in backends/cuda/runtime/aoti_cuda_shims.lib
Auto-merging backends/cuda/runtime/aoti_cuda_shims.lib.md
Auto-merging backends/cuda/runtime/cuda_backend.cpp
Auto-merging backends/cuda/tests/test_cuda_export.py
CONFLICT (content): Merge conflict in backends/cuda/tests/test_cuda_export.py
error: could not apply a8ccc45f6a... Fix CUDA mask indexing, CUDA graph outputs and the runtime output-buffer check (#24299)
hint: Resolve all conflicts manually, mark them as resolved with
hint: "git add/rm <conflicted_files>", then run "git rebase --continue".
hint: You can instead skip this commit: run "git rebase --skip".
hint: To abort and get back to the state before "git rebase", run "git rebase --abort".
hint: Disable this message with "git config set advice.mergeConflict false"
Could not apply a8ccc45f6a... # Fix CUDA mask indexing, CUDA graph outputs and the runtime output-buffer check (#24299)

Details for Dev Infra team

Raised by workflow job

[shoumikhin]

@pytorchbot cherry-pick --onto cherry-pick-24314-by-pytorch_bot_bot_ -c critical

[pytorchbot]

Cherry picking #24420

The cherry pick PR is at #24448 and it is recommended to link a critical cherry pick PR with an issue.

Details for Dev Infra team

Raised by workflow job