park_timeout not waking up after timeout on macs #59020
Labels
C-bug
Category: This is a bug.
O-macos
Operating system: macOS
T-libs-api
Relevant to the library API team, which will review and decide on the PR/issue.
Comments
|
This definitely seems broken. Here's dtruss output: |
sfackler
added
T-libs-api
|
It's a deadlock on the ONCE here: https://github.com/rust-lang/rust/blob/master/src/libstd/sys/unix/time.rs#L241 |
|
As a workaround, everything should work fine as long as you construct an |
jonhoo
added a commit
to jonhoo/bus
that referenced
this issue
Mar 8, 2019
|
Doesn't seem to help :( Codeuse std::thread;
use std::time::Duration;
use std::time;
fn main() {
println!("{:?}", time::Instant::now());
let t1 = thread::spawn(|| {
println!("{:?}", time::Instant::now());
let sleep = Duration::new(0,100_000);
for _ in 0..100 {
println!("Parking1");
thread::park_timeout(sleep);
}
});
let t2 = thread::spawn(|| {
println!("{:?}", time::Instant::now());
let sleep = Duration::new(0,100_000);
for _ in 0..100 {
println!("Parking2");
thread::park_timeout(sleep);
}
});
t1.join().expect("Couldn't join thread 1");
t2.join().expect("Couldn't join thread 2");
}Output |
|
Oh right sorry, you actually have to subtract two Instants to force that Once to complete. If you instead do |
jonhoo
added a commit
to jonhoo/bus
that referenced
this issue
Mar 8, 2019
|
Works perfectly now! Thanks! |
alexcrichton
added a commit
to alexcrichton/rust
that referenced
this issue
Apr 3, 2019
This commit removes usage of `Once` from the internal implementation of time utilities on OSX and Windows. It turns out that we accidentally hit a deadlock today (rust-lang#59020) via events that look like: * A thread invokes `park_timeout` * Internally, only on OSX, `park_timeout` calls `Instant::elapsed` * Inside of `Instant::elapsed` on OSX we enter a `Once` to initialize global timer data * Inside of `Once`, it attempts to `park` This means on the same stack frame, when there's contention, we're calling `park` from inside `park_timeout`, causing a deadlock! The solution implemented in this commit was to remove usage of `Once` and instead just do a small dance with atomics. There's no real need we need to guarantee that the global information is only learned once, only that it's only *stored* once. This implementation may have multiple threads invoke `mach_timebase_info`, but only one will store the global information which will amortize the cost for all other threads. A similar fix has been applied to windows to be uniform across our implementations, but looking at the code on Windows no deadlock was possible. This is purely just a consistency update for Windows and in theory a slightly leaner implementation. Closes rust-lang#59020
Centril
added a commit
to Centril/rust
that referenced
this issue
Apr 3, 2019
std: Avoid usage of `Once` in `Instant` This commit removes usage of `Once` from the internal implementation of time utilities on OSX and Windows. It turns out that we accidentally hit a deadlock today (rust-lang#59020) via events that look like: * A thread invokes `park_timeout` * Internally, only on OSX, `park_timeout` calls `Instant::elapsed` * Inside of `Instant::elapsed` on OSX we enter a `Once` to initialize global timer data * Inside of `Once`, it attempts to `park` This means on the same stack frame, when there's contention, we're calling `park` from inside `park_timeout`, causing a deadlock! The solution implemented in this commit was to remove usage of `Once` and instead just do a small dance with atomics. There's no real need we need to guarantee that the global information is only learned once, only that it's only *stored* once. This implementation may have multiple threads invoke `mach_timebase_info`, but only one will store the global information which will amortize the cost for all other threads. A similar fix has been applied to windows to be uniform across our implementations, but looking at the code on Windows no deadlock was possible. This is purely just a consistency update for Windows and in theory a slightly leaner implementation. Closes rust-lang#59020
alexcrichton
added a commit
to alexcrichton/rust
that referenced
this issue
Apr 4, 2019
This commit removes usage of `Once` from the internal implementation of time utilities on OSX and Windows. It turns out that we accidentally hit a deadlock today (rust-lang#59020) via events that look like: * A thread invokes `park_timeout` * Internally, only on OSX, `park_timeout` calls `Instant::elapsed` * Inside of `Instant::elapsed` on OSX we enter a `Once` to initialize global timer data * Inside of `Once`, it attempts to `park` This means on the same stack frame, when there's contention, we're calling `park` from inside `park_timeout`, causing a deadlock! The solution implemented in this commit was to remove usage of `Once` and instead just do a small dance with atomics. There's no real need we need to guarantee that the global information is only learned once, only that it's only *stored* once. This implementation may have multiple threads invoke `mach_timebase_info`, but only one will store the global information which will amortize the cost for all other threads. A similar fix has been applied to windows to be uniform across our implementations, but looking at the code on Windows no deadlock was possible. This is purely just a consistency update for Windows and in theory a slightly leaner implementation. Closes rust-lang#59020
bors
added a commit
that referenced
this issue
Apr 4, 2019
std: Avoid usage of `Once` in `Instant` This commit removes usage of `Once` from the internal implementation of time utilities on OSX and Windows. It turns out that we accidentally hit a deadlock today (#59020) via events that look like: * A thread invokes `park_timeout` * Internally, only on OSX, `park_timeout` calls `Instant::elapsed` * Inside of `Instant::elapsed` on OSX we enter a `Once` to initialize global timer data * Inside of `Once`, it attempts to `park` This means on the same stack frame, when there's contention, we're calling `park` from inside `park_timeout`, causing a deadlock! The solution implemented in this commit was to remove usage of `Once` and instead just do a small dance with atomics. There's no real need we need to guarantee that the global information is only learned once, only that it's only *stored* once. This implementation may have multiple threads invoke `mach_timebase_info`, but only one will store the global information which will amortize the cost for all other threads. A similar fix has been applied to windows to be uniform across our implementations, but looking at the code on Windows no deadlock was possible. This is purely just a consistency update for Windows and in theory a slightly leaner implementation. Closes #59020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
I was trying to troubleshoot this issue: jonhoo/bus#18 and eventually we figured out that on my machine park_timeout wasn't waking up. I wrote this simple test code which demonstrates the issue:
Most of the time it will only print this:
Every once in a while it will finish normally. This behavior happens on stable and nightly and I tried a handfull of stable versions all the way back to 1.12 and they all got stuck. Windows (both msvc and gnu appear to never get stuck (at least I couldn't reproduce the issue)). I was able to duplicate this an another mac as well (both running 10.14)
If only a single thread is started (or the park happens on the main thread) then I can't reproduce.
I'm on 10.14; uname:
Darwin 34363bc7dc9c 18.2.0 Darwin Kernel Version 18.2.0: Thu Dec 20 20:46:53 PST 2018; root:xnu-4903.241.1~1/RELEASE_X86_64 x86_64 i386 MacBookPro11,3 DarwinThe text was updated successfully, but these errors were encountered: