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

Hang in utp-0.6.3 test suite in Rust 1.15 #38720

Closed
brson opened this issue Dec 30, 2016 · 4 comments
Closed

Hang in utp-0.6.3 test suite in Rust 1.15 #38720

brson opened this issue Dec 30, 2016 · 4 comments
Labels
regression-from-stable-to-beta Performance or correctness regression from stable to beta. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@brson
Copy link
Contributor

brson commented Dec 30, 2016

https://github.com/meqif/rust-utp at e9bf831c41714f545762f482a1d464f188faa1a9

Happens consistently on beta, not on stable.

130 brian@ip-10-145-43-250:~/dev/rust-utp⟫ rustc +beta -Vv
rustc 1.15.0-beta.1 (d9a0f0df7 2016-12-19)
binary: rustc
commit-hash: d9a0f0df7051c603011d6b60fbdd155318fc47f3
commit-date: 2016-12-19
host: x86_64-unknown-linux-gnu
release: 1.15.0-beta.1
LLVM version: 3.9
130 brian@ip-10-145-43-250:~/dev/rust-utp⟫ cargo +beta test
    Finished debug [unoptimized + debuginfo] target(s) in 0.0 secs
     Running target/debug/deps/stream-bbce74275fd70236

running 6 tests
test test_local_addr ... ok
thread 'thread '<unnamed><unnamed>' panicked at '' panicked at 'thread 'The packet is too smallThe packet is too small', tests/stream.rs<unnamed>', ' panicked at 'tests/stream.rsThe packet is too small::72
32note: Run with `RUST_BACKTRACE=1` for a backtrace.
',
tests/stream.rs:49
thread '<unnamed>' panicked at 'The packet is too small', tests/stream.rs:97
test test_stream_open_and_close has been running for over 60 seconds
test test_stream_large_data has been running for over 60 seconds
test test_stream_open_and_close_ipv6 has been running for over 60 seconds
test test_stream_small_data has been running for over 60 seconds

cc @meqif

@brson brson added regression-from-stable-to-beta Performance or correctness regression from stable to beta. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Dec 30, 2016
@meqif
Copy link
Contributor

meqif commented Dec 30, 2016

I can confirm it hangs on beta:

rustc 1.15.0-beta.1 (d9a0f0df7 2016-12-19)
binary: rustc
commit-hash: d9a0f0df7051c603011d6b60fbdd155318fc47f3
commit-date: 2016-12-19
host: x86_64-apple-darwin
release: 1.15.0-beta.1
LLVM version: 3.9

And that it does not hang on stable:

rustc 1.14.0 (e8a012324 2016-12-16)
binary: rustc
commit-hash: e8a0123241f0d397d39cd18fcc4e5e7edde22730
commit-date: 2016-12-16
host: x86_64-apple-darwin
release: 1.14.0
LLVM version: 3.9

Curiously enough, it does not hang on the latest nightly either:

rustc 1.16.0-nightly (4ecc85beb 2016-12-28)
binary: rustc
commit-hash: 4ecc85beb339aa8089d936e450b0d800bdf580ae
commit-date: 2016-12-28
host: x86_64-apple-darwin
release: 1.16.0-nightly
LLVM version: 3.9

@meqif
Copy link
Contributor

meqif commented Dec 30, 2016

I managed to track down the issue to a missing #[repr(C)] on a struct I was casting into an array of bytes with:

unsafe { &*(self as *const PacketHeader as *const [u8; HEADER_SIZE]) }

After adding it, the hanging issues went away and all tests passed. Needless to say, I got what I deserved for using unsafe code. :)

My guess is that something about how structs are laid out changed in the beta and was reverted later on (the nightly doesn't hang and passes all tests), but I'm just shooting in the dark.

Is there anything else I can do to help?

@alexcrichton
Copy link
Member

Ah yes that'd do it! Right now beta contains #37429 which exercises the compiler's right to reorder struct fields which aren't tagged with #[repr(C)]. That was disabled, however, in #38523 and backported to beta today in #38716.

Let's leave this open to verify the test suite passes with the new beta, but my guess is that it will!

@alexcrichton
Copy link
Member

Confirmed working on most recent beta, so closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
regression-from-stable-to-beta Performance or correctness regression from stable to beta. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

3 participants