Skip to content
New issue

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

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

Already on GitHub? # to your account

Default value of build_base leads to races when running tests concurrently #72

Closed
RalfJung opened this issue Aug 26, 2017 · 8 comments
Closed

Comments

@RalfJung
Copy link
Contributor

In miri, we use compiletest in multiple #[test] functions, so they are executed in parallel. We see a lot of test failures like

error: compilation failed!

status: exit code: 101

command: target/release/miri tests/run-pass/aux_test.rs -L /tmp --target=x86_64-unknown-linux-gnu -Z dump-mir=all -Z mir-opt-level=3 -Z dump-mir-dir=/tmp/aux_test -L /tmp/aux_test.stage-id.mir-opt.libaux -C prefer-dynamic -o /tmp/aux_test.stage-id -Zmir-opt-level=0 -Zmir-emit-validate=1

stdout:

------------------------------------------

------------------------------------------

stderr:

------------------------------------------

error: linking with `cc` failed: exit code: 1

  |

  = note: "cc" "-Wl,--as-needed" "-Wl,-z,noexecstack" "-m64" "-L" "/home/travis/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib" "/tmp/aux_test.0.o" "-o" "/tmp/aux_test.stage-id" "/tmp/aux_test.crate.allocator.o" "-Wl,--gc-sections" "-pie" "-Wl,-z,relro,-z,now" "-nodefaultlibs" "-L" "/tmp" "-L" "/tmp/aux_test.stage-id.mir-opt.libaux" "-L" "/home/travis/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib" "-L" "/tmp/aux_test.stage-id.mir-opt.libaux" "-l" "dep" "-L" "/home/travis/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib" "-l" "std-ab8512bb715ab82f" "-Wl,-Bstatic" "/home/travis/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib/libcompiler_builtins-53095306c4bf2e4a.rlib" "-Wl,-Bdynamic" "-l" "dl" "-l" "rt" "-l" "pthread" "-l" "pthread" "-l" "gcc_s" "-l" "c" "-l" "m" "-l" "rt" "-l" "pthread" "-l" "util"

  = note: /usr/bin/ld: cannot find /tmp/aux_test.0.o: No such file or directory

          /usr/bin/ld: cannot find /tmp/aux_test.crate.allocator.o: No such file or directory

          collect2: ld returned 1 exit status

From the behavior of the bug (it only occurs spuriously), this feels a lot like a race condition.
(It also shows that compiletest messes with /tmp really badly, rather than creating its own subdirectory, which I would have expected.)

We track this in miri as rust-lang/miri#305

@RalfJung
Copy link
Contributor Author

Ah, I think I found it. The default value for build_dir is not chosen very well; it's always /tmp. For miri, we will use the tempdir crate to fix that, but I think the default could be better -- or at least, better documented (read: mentioned at all in the docs).

@laumann
Copy link
Collaborator

laumann commented Aug 26, 2017

@RalfJung Thanks for reporting this. The Config struct is not documented well at all :-) it's a good comment to add to build_dirs documentation.

@RalfJung RalfJung changed the title Link failure in concurrent execution: cannot find file Default value of build_dir leads to races when running tests concurrently Aug 26, 2017
@laumann
Copy link
Collaborator

laumann commented Aug 28, 2017

@RalfJung Just to be sure, you meant build_base, right?

@RalfJung
Copy link
Contributor Author

Sorry, yes, I did.

@RalfJung RalfJung changed the title Default value of build_dir leads to races when running tests concurrently Default value of build_base leads to races when running tests concurrently Aug 28, 2017
@laumann
Copy link
Collaborator

laumann commented Aug 28, 2017

No worries, thanks 😄 I submitted rust-lang/rust#44126 to change the existing comments to doc comments.

@RalfJung
Copy link
Contributor Author

RalfJung commented Aug 28, 2017

Sounds like a good start!

How bad would it be for this crate to depend on tempdir? You could then provide a constructor that just does the right thing.

@laumann
Copy link
Collaborator

laumann commented Aug 28, 2017

Hmm, could it be added as an opt-in feature somehow? That would be awesome.

@RalfJung
Copy link
Contributor Author

Yeah, cargo features can totally do that.

laumann pushed a commit that referenced this issue Aug 29, 2017
Using feature "tmp" the build_base will be a temporary directory provided by the
tempdir crate. This provides a feature useful for running tests in parallel.

Fixes #72
laumann pushed a commit that referenced this issue Sep 1, 2017
Using feature "tmp" the build_base will be a temporary directory provided by the
tempdir crate. This provides a feature useful for running tests in parallel.

Fixes #72
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants