Fix longstanding race condition around getAllOperatorsFor by swolchok · Pull Request #167860 · pytorch/pytorch

swolchok · 2025-11-14T19:23:08Z

Stack from ghstack (oldest at bottom):

-> Fix longstanding race condition around getAllOperatorsFor #167860

getAllOperatorsFor returns a const reference to internal state that is protected by a lock. Presuming that the lock is necessary in the first place (about which I offer no opinion because it's unclear to what extent the GIL should help here), this is a straightforward way to cause callers to create race conditions.

This should fix those race conditions by copying the state instead. I modified calling code to stop binding a const reference to the result for clarity.

Differential Revision: D87088731

NOTE FOR REVIEWERS: This PR has internal Meta-specific changes or comments, please review them on Phabricator!

cc @EikanWang @jgong5 @wenzhe-nrv @sanchitintel

getAllOperatorsFor returns a const reference to internal state that is protected by a lock. Presuming that the lock is necessary in the first place (about which I offer no opinion), this is a straightforward way to cause callers to create race conditions. This should fix those race conditions by copying the state instead. I modified calling code to stop binding a const reference to the result for clarity. Differential Revision: [D87088731](https://our.internmc.facebook.com/intern/diff/D87088731/) **NOTE FOR REVIEWERS**: This PR has internal Meta-specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D87088731/)! [ghstack-poisoned]

pytorch-bot · 2025-11-14T19:23:12Z

🔗 Helpful Links

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

📄 Preview Python docs built from this PR
📄 Preview C++ docs built from this PR
❓ Need help or want to give feedback on the CI? Visit the bot commands wiki

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

✅ You can merge normally! (1 Unrelated Failure)

As of commit 8d169ad with merge base 2aba180 ():

UNSTABLE - The following job is marked as unstable, possibly due to flakiness on trunk:

trunk / linux-jammy-py3-clang12-executorch / test (executorch, 1, 1, lf.linux.2xlarge, unstable) (gh) (#166072)
backends/xnnpack/test/recipes/test_xnnpack_recipes.py::TestXnnpackRecipes::test_int8_static_quant_recipe

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

getAllOperatorsFor returns a const reference to internal state that is protected by a lock. Presuming that the lock is necessary in the first place (about which I offer no opinion), this is a straightforward way to cause callers to create race conditions. This should fix those race conditions by copying the state instead. I modified calling code to stop binding a const reference to the result for clarity. Differential Revision: [D87088731](https://our.internmc.facebook.com/intern/diff/D87088731/) **NOTE FOR REVIEWERS**: This PR has internal Meta-specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D87088731/)! ghstack-source-id: 323363321 Pull Request resolved: #167860

pytorch-bot · 2025-11-14T19:23:23Z

The label oncall: jit is only applicable to issues and has been removed. Please only use this label on issues.

Skylion007 · 2025-11-15T20:17:44Z

torch/csrc/jit/runtime/operator.cpp

+    // the "first" op may change depending on registration order.
+    std::vector<std::shared_ptr<Operator>> sortedOps;
+    sortedOps.reserve(unsortedOps.size());
+    std::copy_if(


Is this really faster than a stable Boolean sort?

This PR makes no judgement on that; this is existing code that we're not changing. (however I note that this is two passes over the input whereas a sort is going to take N log N)

Skylion007 · 2025-11-15T20:20:40Z

torch/csrc/jit/runtime/operator.cpp

  }

+  const std::vector<std::shared_ptr<Operator>>& getOperatorsWithLockHeld(
+      Symbol name) {


Is symbol really trivial to take by value or should this be const ref?

it fits in a register and has no copy ctor

Skylion007 · 2025-11-15T20:22:51Z

torch/csrc/jit/runtime/operator.cpp

+    auto it = operators.find(name);
+    if (it != operators.end())
+      return it->second;
+    return empty;


Ugh this is weird. If only we had constexpr std::vector before C++20...

swolchok · 2025-11-17T17:16:52Z

@pytorchbot merge -i

pytorchmergebot · 2025-11-17T17:25:49Z

Merge started

Your change will be merged while ignoring the following 1 checks: trunk / linux-jammy-py3-clang12-executorch / test (executorch, 1, 1, lf.linux.2xlarge, unstable)

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging

Check the merge workflow status
here

…7860) getAllOperatorsFor returns a const reference to internal state that is protected by a lock. Presuming that the lock is necessary in the first place (about which I offer no opinion because it's unclear to what extent the GIL should help here), this is a straightforward way to cause callers to create race conditions. This should fix those race conditions by copying the state instead. I modified calling code to stop binding a const reference to the result for clarity. Differential Revision: [D87088731](https://our.internmc.facebook.com/intern/diff/D87088731/) **NOTE FOR REVIEWERS**: This PR has internal Meta-specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D87088731/)! Pull Request resolved: pytorch#167860 Approved by: https://github.com/zou3519

``` git revert --no-commit 567dcdb 200156e 3d801a4 2034ca9 480b4ff f570e58 ``` And Revert "[DTensor] Document fast-path dispatch (#168192)" And Revert "[DTensor] Fix deadlock after fast cache clear (#168069)" Reverts: * #167860 * #167588 * #167475 * #166808 * #166372 * #168192 * #168069 Signed-off-by: Edward Z. Yang <ezyang@meta.com> Pull Request resolved: #168264 Approved by: https://github.com/seemethere, https://github.com/malfet

This reverts commit 567dcdb. This reverts commit 0e7ccc0. ghstack-source-id: fec7bf5 Pull-Request: #169281

This reverts commit 567dcdb. This reverts commit 0e7ccc0. ghstack-source-id: 950b6d8 Pull-Request: #169281

…torch#169281 Summary: This reverts commit 567dcdb. This reverts commit 0e7ccc0. These changes break downstream builds and also are the suspected culprit of a memory corruption. Test Plan: sandcastle Differential Revision: D88394221

This reverts commit 567dcdb. This reverts commit 0e7ccc0. These changes break downstream builds and also are the suspected culprit of a memory corruption. Pull Request resolved: #169281 Approved by: https://github.com/zou3519, https://github.com/albanD

…ytorch#169281) This reverts commit 567dcdb. This reverts commit 0e7ccc0. These changes break downstream builds and also are the suspected culprit of a memory corruption. Pull Request resolved: pytorch#169281 Approved by: https://github.com/zou3519, https://github.com/albanD

``` git revert --no-commit 567dcdb 200156e 3d801a4 2034ca9 480b4ff f570e58 ``` And Revert "[DTensor] Document fast-path dispatch (#168192)" And Revert "[DTensor] Fix deadlock after fast cache clear (#168069)" Reverts: * #167860 * #167588 * #167475 * #166808 * #166372 * #168192 * #168069 Signed-off-by: Edward Z. Yang <ezyang@meta.com> Pull Request resolved: #168264 Approved by: https://github.com/seemethere, https://github.com/malfet

This reverts commit 567dcdb. This reverts commit 0e7ccc0. These changes break downstream builds and also are the suspected culprit of a memory corruption. Pull Request resolved: #169281 Approved by: https://github.com/zou3519, https://github.com/albanD

swolchok requested a review from mikaylagawarecki as a code owner November 14, 2025 19:23

pytorch-bot bot added the release notes: jit release notes category label Nov 14, 2025

facebook-github-bot added the oncall: jit Add this issue/PR to JIT oncall triage queue label Nov 14, 2025

meta-codesync bot added fb-exported meta-exported labels Nov 14, 2025

pytorch-bot bot removed the oncall: jit Add this issue/PR to JIT oncall triage queue label Nov 14, 2025

swolchok requested review from Skylion007 and zou3519 November 14, 2025 19:28

Skylion007 reviewed Nov 15, 2025

View reviewed changes

zou3519 approved these changes Nov 17, 2025

View reviewed changes

pytorch-bot bot added the ciflow/trunk Trigger trunk jobs on your pull request label Nov 17, 2025

pytorchmergebot added the merging label Nov 17, 2025

pytorchmergebot closed this in 567dcdb Nov 17, 2025

pytorchmergebot added Merged and removed merging labels Nov 17, 2025

ezyang mentioned this pull request Nov 20, 2025

Revert C++ fastpath dispatch path for DTensor #168264

Closed

ezyang added a commit that referenced this pull request Dec 1, 2025

Revert getAllOperatorsFor changes (#167860, #162218)

3dd3f3b

This reverts commit 567dcdb. This reverts commit 0e7ccc0. ghstack-source-id: fec7bf5 Pull-Request: #169281

pytorchmergebot pushed a commit that referenced this pull request Dec 1, 2025

Revert getAllOperatorsFor changes (#167860, #162218)

b725b8c

This reverts commit 567dcdb. This reverts commit 0e7ccc0. ghstack-source-id: 950b6d8 Pull-Request: #169281

github-actions bot deleted the gh/swolchok/871/head branch December 18, 2025 02:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix longstanding race condition around getAllOperatorsFor#167860

Fix longstanding race condition around getAllOperatorsFor#167860
swolchok wants to merge 1 commit intogh/swolchok/871/basefrom
gh/swolchok/871/head

swolchok commented Nov 14, 2025 •

edited

Loading

Uh oh!

pytorch-bot bot commented Nov 14, 2025 •

edited

Loading

Uh oh!

pytorch-bot bot commented Nov 14, 2025

Uh oh!

Skylion007 Nov 15, 2025

Uh oh!

swolchok Nov 17, 2025

Uh oh!

Skylion007 Nov 15, 2025

Uh oh!

swolchok Nov 17, 2025

Uh oh!

Skylion007 Nov 15, 2025

Uh oh!

swolchok commented Nov 17, 2025

Uh oh!

pytorchmergebot commented Nov 17, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

swolchok commented Nov 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pytorch-bot bot commented Nov 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🔗 Helpful Links

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

✅ You can merge normally! (1 Unrelated Failure)

Uh oh!

pytorch-bot bot commented Nov 14, 2025

Uh oh!

Skylion007 Nov 15, 2025

Choose a reason for hiding this comment

Uh oh!

swolchok Nov 17, 2025

Choose a reason for hiding this comment

Uh oh!

Skylion007 Nov 15, 2025

Choose a reason for hiding this comment

Uh oh!

swolchok Nov 17, 2025

Choose a reason for hiding this comment

Uh oh!

Skylion007 Nov 15, 2025

Choose a reason for hiding this comment

Uh oh!

swolchok commented Nov 17, 2025

Uh oh!

pytorchmergebot commented Nov 17, 2025

Merge started

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

swolchok commented Nov 14, 2025 •

edited

Loading

pytorch-bot bot commented Nov 14, 2025 •

edited

Loading