Skip to content

Fix race condition in Function::optimized_graph(). #27012

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from

Conversation

ZolotukhinM
Copy link

@ZolotukhinM ZolotukhinM commented Sep 28, 2019

Stack from ghstack:

The current logic is buggy, and will fail in the following situation:

Thread A: check optimized_graph_, it is empty.
Thread A: claim the mutex in order to initialize optimized_graph_.
Thread A: copy graph_ into optimized_graph_.
Thread A: start running optimizations on optimized_graph_.
Thread B: check optimized_graph_, it is not empty.
Thread B: start using optimized_graph_.

BUG: Thread B is using the graph while it's still being mutated by
Thread A.

Differential Revision: D17649149

The current logic is buggy, and will fail in the following situation:

Thread A: check optimized_graph_, it is empty.
Thread A: claim the mutex in order to initialize optimized_graph_.
Thread A: copy graph_ into optimized_graph_.
Thread A: start running optimizations on optimized_graph_.
Thread B: check optimized_graph_, it is not empty.
Thread B: start using optimized_graph_.

BUG: Thread B is using the graph while it's still being mutated by
Thread A.

[ghstack-poisoned]
@pytorchbot pytorchbot added the oncall: jit Add this issue/PR to JIT oncall triage queue label Sep 28, 2019
ZolotukhinM pushed a commit that referenced this pull request Sep 28, 2019
The current logic is buggy, and will fail in the following situation:

Thread A: check optimized_graph_, it is empty.
Thread A: claim the mutex in order to initialize optimized_graph_.
Thread A: copy graph_ into optimized_graph_.
Thread A: start running optimizations on optimized_graph_.
Thread B: check optimized_graph_, it is not empty.
Thread B: start using optimized_graph_.

BUG: Thread B is using the graph while it's still being mutated by
Thread A.

ghstack-source-id: a8bb36e
Pull Request resolved: #27012
@facebook-github-bot
Copy link
Contributor

@ZolotukhinM merged this pull request in 310ebb4.

@dzhulgakov dzhulgakov added this to the 1.3 milestone Sep 30, 2019
@ZolotukhinM
Copy link
Author

Pull Request for v1.3.0: #27323

@facebook-github-bot facebook-github-bot deleted the gh/ZolotukhinM/125/head branch October 28, 2019 22:07
pdlive215 pushed a commit to pdlive215/pytorch that referenced this pull request Nov 27, 2019
Summary:
Pull Request resolved: pytorch#27012

The current logic is buggy, and will fail in the following situation:

Thread A: check optimized_graph_, it is empty.
Thread A: claim the mutex in order to initialize optimized_graph_.
Thread A: copy graph_ into optimized_graph_.
Thread A: start running optimizations on optimized_graph_.
Thread B: check optimized_graph_, it is not empty.
Thread B: start using optimized_graph_.

BUG: Thread B is using the graph while it's still being mutated by
Thread A.

Test Plan: Imported from OSS

Differential Revision: D17649149

Pulled By: ZolotukhinM

fbshipit-source-id: ab82a4e76003b88733d219ecd47a42d38be2269c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Merged oncall: jit Add this issue/PR to JIT oncall triage queue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants