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

Always run rustc in a thread #56813

Merged
merged 2 commits into from Dec 21, 2018

Conversation

Projects
None yet
9 participants
@oli-obk
Copy link
Contributor

oli-obk commented Dec 14, 2018

cc @ishitatsuyuki @eddyb

r? @pnkfelix

Previously we moved to only producing threads when absolutely necessary. Even before we opted to only create threads in some cases, which is unsound due to the way we use thread local storage.

@ishitatsuyuki

This comment has been minimized.

Copy link
Member

ishitatsuyuki commented Dec 14, 2018

Please remove std::rt::update_stack_guard in the case, since we won't be maintaining that functionality.

@rust-highfive

This comment was marked as resolved.

Copy link
Collaborator

rust-highfive commented Dec 14, 2018

The job x86_64-gnu-llvm-5.0 of your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.
travis_time:end:0a06bba8:start=1544804643926193860,finish=1544804698271149349,duration=54344955489
$ git checkout -qf FETCH_HEAD
travis_fold:end:git.checkout

Encrypted environment variables have been removed for security reasons.
See https://docs.travis-ci.com/user/pull-requests/#Pull-Requests-and-Security-Restrictions
$ export SCCACHE_BUCKET=rust-lang-ci-sccache2
$ export SCCACHE_REGION=us-west-1
Setting environment variables from .travis.yml
$ export IMAGE=x86_64-gnu-llvm-5.0
---
[00:03:44]    Compiling panic_unwind v0.0.0 (/checkout/src/libpanic_unwind)
[00:03:49] warning: function is never used: `deinit`
[00:03:49]    --> src/libstd/sys/unix/thread.rs:358:5
[00:03:49]     |
[00:03:49] 358 |     pub unsafe fn deinit() {
[00:03:49]     |
[00:03:49]     = note: #[warn(dead_code)] on by default
[00:03:49] 
[00:04:00]     Finished release [optimized] target(s) in 52.73s

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

@oli-obk oli-obk force-pushed the oli-obk:main_ branch from e658c9b to 88ec87f Dec 14, 2018

@oli-obk oli-obk force-pushed the oli-obk:main_ branch from 88ec87f to 6b96827 Dec 14, 2018

if spawn_thread || env::var_os("RUST_MIN_STACK").is_some() {
// We need a thread for soundness of thread local storage in rustc. For debugging purposes
// we allow an escape hatch where everything runs on the main thread.
if env::var_os("RUSTC_UNSTABLE_NO_MAIN_THREAD").is_none() {

This comment has been minimized.

@nagisa

nagisa Dec 15, 2018

Contributor

I find the idea of using an environment variable here so undiscoverable that the condition might as well be false for all intents and purposes…

The manual page for rustc technically has a section on environment variables. Perhaps it would be a good place to document this envvar there as well?

This comment has been minimized.

@pnkfelix

pnkfelix Dec 17, 2018

Member

Also, does the compiler have a part of its --help -v output dedicated to environment variables? That might be nice too.

This comment has been minimized.

@pnkfelix

pnkfelix Dec 17, 2018

Member

(having said that, such additions can wait for a followup PR... lets get this PR in now.)

@nagisa

This comment has been minimized.

Copy link
Contributor

nagisa commented Dec 15, 2018

The implementation looks good to me.

@pnkfelix

This comment has been minimized.

Copy link
Member

pnkfelix commented Dec 17, 2018

@bors r+

@bors

This comment has been minimized.

Copy link
Contributor

bors commented Dec 17, 2018

📌 Commit 6b96827 has been approved by pnkfelix

@bors

This comment has been minimized.

Copy link
Contributor

bors commented Dec 20, 2018

⌛️ Testing commit 6b96827 with merge 890597e...

bors added a commit that referenced this pull request Dec 20, 2018

Auto merge of #56813 - oli-obk:main_🧶, r=pnkfelix
Always run rustc in a thread

cc @ishitatsuyuki @eddyb

r? @pnkfelix

[Previously](#48575) we moved to only producing threads when absolutely necessary. Even before we opted to only create threads in some cases, which [is unsound](#48575 (comment)) due to the way we use thread local storage.
@bors

This comment has been minimized.

Copy link
Contributor

bors commented Dec 20, 2018

💔 Test failed - status-travis

@rust-highfive

This comment has been minimized.

Copy link
Collaborator

rust-highfive commented Dec 20, 2018

The job dist-x86_64-linux of your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.
[01:36:29]    Compiling cargo v0.32.0
[01:36:35] error[E0308]: mismatched types
[01:36:35]  --> /cargo/registry/src/github.com-1ecc6299db9ec823/cargo-0.32.0/src/cargo/util/sha256.rs:9:34
[01:36:35]   |
[01:36:35] 9 |         let hasher = Hasher::new(Algorithm::SHA256);
[01:36:35]   |                                  |
[01:36:35]   |                                  |
[01:36:35]   |                                  expected reference, found enum `util::sha256::crypto_hash::Algorithm`
[01:36:35]   |                                  help: consider borrowing here: `&Algorithm::SHA256`
[01:36:35]   |
[01:36:35]   = note: expected type `&util::sha256::crypto_hash::Algorithm`
[01:36:35]              found type `util::sha256::crypto_hash::Algorithm`
[01:36:35] error: aborting due to previous error
[01:36:35] 
[01:36:35] For more information about this error, try `rustc --explain E0308`.
[01:36:35] For more information about this error, try `rustc --explain E0308`.
[01:36:35] error: failed to compile `cargo-vendor v0.1.22`, intermediate artifacts can be found at `/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-bootstrap-tools`
[01:36:35] Caused by:
[01:36:35]   Could not compile `cargo`.
[01:36:35] 
[01:36:35] To learn more, run the command again with --verbose.
[01:36:35] To learn more, run the command again with --verbose.
[01:36:35] 
[01:36:35] 
[01:36:35] command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "install" "-j" "4" "--locked" "--color" "always" "--force" "--debug" "--vers" "0.1.22" "cargo-vendor"
[01:36:35] 
[01:36:35] 
[01:36:35] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap dist --host x86_64-unknown-linux-gnu --target x86_64-unknown-linux-gnu
[01:36:35] Build completed unsuccessfully in 1:30:43
---
travis_time:end:0fd00d0c:start=1545331532947154253,finish=1545331532959197945,duration=12043692
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:05e5c405
$ ln -s . checkout && for CORE in obj/cores/core.*; do EXE=$(echo $CORE | sed 's|obj/cores/core\.[0-9]*\.!checkout!\(.*\)|\1|;y|!|/|'); if [ -f "$EXE" ]; then printf travis_fold":start:crashlog\n\033[31;1m%s\033[0m\n" "$CORE"; gdb --batch -q -c "$CORE" "$EXE" -iex 'set auto-load off' -iex 'dir src/' -iex 'set sysroot .' -ex bt -ex q; echo travis_fold":"end:crashlog; fi; done || true
travis_fold:end:after_failure.4
travis_fold:start:after_failure.5
travis_time:start:0eb80e5e
travis_time:start:0eb80e5e
$ cat ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers || true
cat: ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers: No such file or directory
travis_fold:end:after_failure.5
travis_fold:start:after_failure.6
travis_time:start:0df16a48
$ dmesg | grep -i kill

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

@alexcrichton

This comment has been minimized.

Copy link
Member

alexcrichton commented Dec 20, 2018

@bors

This comment has been minimized.

Copy link
Contributor

bors commented Dec 21, 2018

⌛️ Testing commit 6b96827 with merge 01c6ea2...

bors added a commit that referenced this pull request Dec 21, 2018

Auto merge of #56813 - oli-obk:main_🧶, r=pnkfelix
Always run rustc in a thread

cc @ishitatsuyuki @eddyb

r? @pnkfelix

[Previously](#48575) we moved to only producing threads when absolutely necessary. Even before we opted to only create threads in some cases, which [is unsound](#48575 (comment)) due to the way we use thread local storage.
@bors

This comment has been minimized.

Copy link
Contributor

bors commented Dec 21, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: pnkfelix
Pushing 01c6ea2 to master...

@bors bors merged commit 6b96827 into rust-lang:master Dec 21, 2018

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
@nikomatsakis

This comment has been minimized.

Copy link
Contributor

nikomatsakis commented Jan 10, 2019

In the @rust-lang/compiler meeting today, we decided against backporting this because the problem just doesn't seem that urgent. The bug is in stable already and not many folks have commented on it (it's quite challenging to reproduce). So it can ride the trains as usual, lower risk. (discussion)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment