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

watch: fix spurious wakeup #3234

Merged
merged 4 commits into from Dec 10, 2020
Merged

watch: fix spurious wakeup #3234

merged 4 commits into from Dec 10, 2020

Conversation

Darksonn
Copy link
Contributor

@Darksonn Darksonn commented Dec 8, 2020

Closes: #3168

I will push the fix that @tijsvd suggested once I have observed the test failing on CI. It does fix the test locally.

loop {
        let notified = self.shared.notify_rx.notified();
        if let Some(ret) = maybe_changed(&self.shared, &mut self.version) {
            return ret;
        }
        notified.await;
}

command to run the test:

LOOM_MAX_PREEMPTIONS=1 RUSTFLAGS="--cfg loom" cargo test --lib --release --features full -- --test-threads=1 --nocapture watch_spurious_wakeup

@Darksonn Darksonn added C-enhancement Category: A PR with an enhancement or bugfix. A-tokio Area: The main tokio crate M-sync Module: tokio/sync labels Dec 8, 2020
@Darksonn
Copy link
Contributor Author

Darksonn commented Dec 8, 2020

The test failure has been observed in CI / loom (--skip loom_pool) (pull_request).

thread 'sync::watch::tests::watch_spurious_wakeup' panicked at '[bug] failed to observe change after notificaton.', tokio/src/sync/watch.rs:256:14
stack backtrace:
   0: rust_begin_unwind
             at /rustc/7eac88abb2e57e752f3302f02be5f3ce3d7adfb4/library/std/src/panicking.rs:483
   1: core::panicking::panic_fmt
             at /rustc/7eac88abb2e57e752f3302f02be5f3ce3d7adfb4/library/core/src/panicking.rs:85
   2: core::option::expect_failed
             at /rustc/7eac88abb2e57e752f3302f02be5f3ce3d7adfb4/library/core/src/option.rs:1226
   3: <core::future::from_generator::GenFuture<T> as core::future::future::Future>::poll
   4: futures_util::future::future::FutureExt::now_or_never
   5: core::ops::function::FnOnce::call_once{{vtable.shim}}
   6: loom::rt::scheduler::spawn_threads::{{closure}}::{{closure}}
   7: generator::stack::StackBox<F>::call_once
   8: generator::gen_impl::gen_init
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
test sync::watch::tests::watch_spurious_wakeup ... FAILED
Completed in 123 iterations

@Darksonn Darksonn merged commit f60860a into master Dec 10, 2020
@Darksonn Darksonn deleted the watch-spurious-wakeup branch December 10, 2020 08:46
Darksonn added a commit that referenced this pull request Dec 10, 2020
@Darksonn Darksonn restored the watch-spurious-wakeup branch December 10, 2020 10:31
@Darksonn Darksonn deleted the watch-spurious-wakeup branch December 10, 2020 10:31
@taiki-e taiki-e mentioned this pull request Dec 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-tokio Area: The main tokio crate C-enhancement Category: A PR with an enhancement or bugfix. M-sync Module: tokio/sync
Projects
None yet
Development

Successfully merging this pull request may close these issues.

sync::watch misbehaves in multi-threaded context
2 participants