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

[beta] std: Synchronize access to global env during exec #55978

Merged
merged 2 commits into from
Nov 15, 2018

Conversation

alexcrichton
Copy link
Member

This is a beta backport of #55939

alexcrichton and others added 2 commits November 15, 2018 05:42
This commit, after reverting rust-lang#55359, applies a different fix for rust-lang#46775
while also fixing rust-lang#55775. The basic idea was to go back to pre-rust-lang#55359
libstd, and then fix rust-lang#46775 in a way that doesn't expose rust-lang#55775.

The issue described in rust-lang#46775 boils down to two problems:

* First, the global environment is reset during `exec` but, but if the
  `exec` call fails then the global environment was a dangling pointer
  into free'd memory as the block of memory was deallocated when
  `Command` is dropped. This is fixed in this commit by installing a
  `Drop` stack object which ensures that the `environ` pointer is
  preserved on a failing `exec`.

* Second, the global environment was accessed in an unsynchronized
  fashion during `exec`. This was fixed by ensuring that the
  Rust-specific environment lock is acquired for these system-level
  operations.

Thanks to Alex Gaynor for pioneering the solution here!

Closes rust-lang#55775

Co-authored-by: Alex Gaynor <alex.gaynor@gmail.com>
@rust-highfive
Copy link
Collaborator

r? @Mark-Simulacrum

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive
Copy link
Collaborator

⚠️ Warning ⚠️

  • Pull requests are usually filed against the master branch for this repo, but this one is against beta. Please double check that you specified the right target!

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 15, 2018
@alexcrichton alexcrichton added this to the Rust 2018 Release milestone Nov 15, 2018
@Mark-Simulacrum
Copy link
Member

@bors r+ p=1

@bors
Copy link
Contributor

bors commented Nov 15, 2018

📌 Commit 9609a91 has been approved by Mark-Simulacrum

@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 Nov 15, 2018
@bors
Copy link
Contributor

bors commented Nov 15, 2018

⌛ Testing commit 9609a91 with merge 51be258...

bors added a commit that referenced this pull request Nov 15, 2018
[beta] std: Synchronize access to global env during `exec`

This is a beta backport of #55939
@bors
Copy link
Contributor

bors commented Nov 15, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: Mark-Simulacrum
Pushing 51be258 to beta...

@bors bors merged commit 9609a91 into rust-lang:beta Nov 15, 2018
@pietroalbini
Copy link
Member

Please remove the beta-nominated tag after a backport is approved :)

@alexcrichton alexcrichton deleted the beta-next branch November 15, 2018 22:04
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.

5 participants