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

std: Move a process test out of libstd #62183

Merged
merged 1 commit into from
Jul 3, 2019
Merged

Conversation

alexcrichton
Copy link
Member

This commit moves a test out of libstd which is causing deadlocks on
musl on CI. Looks like the recent update in musl versions brings in some
internal updates to musl which makes setgid and setuid invalid to
call after a fork in a multithreaded program. The issue seen here is
that the child thread was attempting to grab a lock held by a
nonexistent thread, meaning that the child process simply deadlocked
causing the whole test to deadlock.

This commit moves the test to its own file with no threads which should
work.

This commit moves a test out of libstd which is causing deadlocks on
musl on CI. Looks like the recent update in musl versions brings in some
internal updates to musl which makes `setgid` and `setuid` invalid to
call after a `fork` in a multithreaded program. The issue seen here is
that the child thread was attempting to grab a lock held by a
nonexistent thread, meaning that the child process simply deadlocked
causing the whole test to deadlock.

This commit moves the test to its own file with no threads which should
work.
@rust-highfive
Copy link
Collaborator

r? @nikomatsakis

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

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 27, 2019
@alexcrichton
Copy link
Member Author

For reference, this is the commit that I believe is the change of behavior in musl.

@RalfJung
Copy link
Member

However this also means Command is buggy on that target, right? Seems like one can trigger that from safe code with something like .uid(1000).gid(1000).

@alexcrichton
Copy link
Member Author

That is true, yes, but I don't really want to get in the business of fixing all platforms everywhere. At some point we just need our CI to work, and that's what this does.

@RalfJung
Copy link
Member

Fair. Is there an issue tracking this? Or don't we track such issues in the rustc repo?

@alexcrichton
Copy link
Member Author

There is not currently, no, since it appears musl doesn't have an issue tracker, just a mailing list.

@smaeul
Copy link
Contributor

smaeul commented Jul 1, 2019

@alexcrichton musl patch sent upstream: https://www.openwall.com/lists/musl/2019/07/01/2. I think a new release is planned to be out soon, or you can take the patch.

@alexcrichton
Copy link
Member Author

r? @pietroalbini

@smaeul we can update when a new release comes out, yes

@pietroalbini
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Jul 1, 2019

📌 Commit 2a37582 has been approved by pietroalbini

@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 Jul 1, 2019
@nikomatsakis
Copy link
Contributor

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Jul 1, 2019

💡 This pull request was already approved, no need to approve it again.

@bors
Copy link
Contributor

bors commented Jul 1, 2019

📌 Commit 2a37582 has been approved by nikomatsakis

@nikomatsakis
Copy link
Contributor

@bors rollup-

just saw this was reassigned to @pietroalbini =) I'll leave it as they had it

Manishearth added a commit to Manishearth/rust that referenced this pull request Jul 2, 2019
std: Move a process test out of libstd

This commit moves a test out of libstd which is causing deadlocks on
musl on CI. Looks like the recent update in musl versions brings in some
internal updates to musl which makes `setgid` and `setuid` invalid to
call after a `fork` in a multithreaded program. The issue seen here is
that the child thread was attempting to grab a lock held by a
nonexistent thread, meaning that the child process simply deadlocked
causing the whole test to deadlock.

This commit moves the test to its own file with no threads which should
work.
Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this pull request Jul 3, 2019
std: Move a process test out of libstd

This commit moves a test out of libstd which is causing deadlocks on
musl on CI. Looks like the recent update in musl versions brings in some
internal updates to musl which makes `setgid` and `setuid` invalid to
call after a `fork` in a multithreaded program. The issue seen here is
that the child thread was attempting to grab a lock held by a
nonexistent thread, meaning that the child process simply deadlocked
causing the whole test to deadlock.

This commit moves the test to its own file with no threads which should
work.
Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this pull request Jul 3, 2019
std: Move a process test out of libstd

This commit moves a test out of libstd which is causing deadlocks on
musl on CI. Looks like the recent update in musl versions brings in some
internal updates to musl which makes `setgid` and `setuid` invalid to
call after a `fork` in a multithreaded program. The issue seen here is
that the child thread was attempting to grab a lock held by a
nonexistent thread, meaning that the child process simply deadlocked
causing the whole test to deadlock.

This commit moves the test to its own file with no threads which should
work.
bors added a commit that referenced this pull request Jul 3, 2019
Rollup of 15 pull requests

Successful merges:

 - #62021 (MSVC link output improve)
 - #62064 (nth_back for chunks_exact)
 - #62128 (Adjust warning of -C extra-filename with -o.)
 - #62161 (Add missing links for TryFrom docs)
 - #62183 (std: Move a process test out of libstd)
 - #62186 (Add missing type urls in Into trait)
 - #62196 (Add Vec::leak)
 - #62199 (import gdb for explicit access to gdb.current_objfile())
 - #62229 (Enable intptrcast for explicit casts)
 - #62250 (Improve box clone doctests to ensure the documentation is valid)
 - #62255 (Switch tracking issue for `#![feature(slice_patterns)]`)
 - #62285 (Fix michaelwoerister's mailmap)
 - #62304 (HashMap is UnwindSafe)
 - #62319 (Fix mismatching Kleene operators)
 - #62327 (Fixed document bug, those replaced each other)

Failed merges:

r? @ghost
@bors bors merged commit 2a37582 into rust-lang:master Jul 3, 2019
@alexcrichton alexcrichton deleted the fix-tests branch July 8, 2019 20:33
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.

7 participants