Skip to content
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

Fix race condition when allocating source files in SourceMap #69266

Merged
merged 1 commit into from
Feb 20, 2020

Conversation

Zoxc
Copy link
Contributor

@Zoxc Zoxc commented Feb 18, 2020

This makes allocating address space in the source map an atomic operation. rustc does not currently do this in parallel, so this bug can't trigger, but parsing files in parallel could trigger it, and that is something we want to do.

Fixes #69261.

r? @wesleywiser

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 18, 2020
@Zoxc
Copy link
Contributor Author

Zoxc commented Feb 18, 2020

cc @petrochenkov

Copy link
Member

@wesleywiser wesleywiser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One small comment. If you don't think it's worthwhile, feel free to r=me this.

src/librustc_span/source_map.rs Show resolved Hide resolved
@petrochenkov
Copy link
Contributor

cc @matklad

Copy link
Contributor

@mlodato517 mlodato517 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Found a minor typo and am interested in trying to learn a little more about rust if you have the time to answer some questions :-) Also, I don't see any notes in CONTRIBUTING.md about reviewing PRs so I hope I'm not out of line in reviewing this. If there are some docs regarding when/how to review PRs - please link me to them so I don't make the mistake again.

src/librustc_span/source_map.rs Outdated Show resolved Hide resolved
src/librustc_span/source_map.rs Show resolved Hide resolved
src/librustc_span/source_map.rs Show resolved Hide resolved
src/librustc_span/lib.rs Show resolved Hide resolved
@wesleywiser
Copy link
Member

@mlodato517 If you have additional questions feel free to ask! Otherwise I'm going to mark those parts of your review resolved to keep this PR clean.

@Zoxc
Copy link
Contributor Author

Zoxc commented Feb 18, 2020

@bors r=wesleywiser

@bors
Copy link
Contributor

bors commented Feb 18, 2020

📌 Commit 437f56e has been approved by wesleywiser

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 18, 2020
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Feb 20, 2020
…iser

Fix race condition when allocating source files in SourceMap

This makes allocating address space in the source map an atomic operation. `rustc` does not currently do this in parallel, so this bug can't trigger, but parsing files in parallel could trigger it, and that is something we want to do.

Fixes rust-lang#69261.

r? @wesleywiser
This was referenced Feb 20, 2020
bors added a commit that referenced this pull request Feb 20, 2020
Rollup of 5 pull requests

Successful merges:

 - #68705 (Add LinkedList::remove())
 - #68945 (Stabilize Once::is_completed)
 - #68978 (Make integer exponentiation methods unstably const)
 - #69266 (Fix race condition when allocating source files in SourceMap)
 - #69287 (Clean up E0317 explanation)

Failed merges:

r? @ghost
@bors bors merged commit 5d285dc into rust-lang:master Feb 20, 2020
@Zoxc Zoxc deleted the fix-source-map-race branch February 20, 2020 15:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

librustc_span has a concurrency bug
6 participants