From 5351395106f6d2866b73608b71bbc32e5e9d2aef Mon Sep 17 00:00:00 2001 From: David Renshaw Date: Tue, 1 Nov 2016 08:20:39 -0400 Subject: [PATCH] Add a comment explaining that we depend on SeqCst ordering. --- src/lock.rs | 2 +- src/oneshot.rs | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/src/lock.rs b/src/lock.rs index 0e233e19e47..4455c79995f 100644 --- a/src/lock.rs +++ b/src/lock.rs @@ -8,7 +8,7 @@ extern crate core; use self::core::cell::UnsafeCell; use self::core::ops::{Deref, DerefMut}; -use self::core::sync::atomic::Ordering::{SeqCst}; +use self::core::sync::atomic::Ordering::SeqCst; use self::core::sync::atomic::AtomicBool; /// A "mutex" around a value, similar to `std::sync::Mutex`. diff --git a/src/oneshot.rs b/src/oneshot.rs index ab6c37d8c81..50e38656d3f 100644 --- a/src/oneshot.rs +++ b/src/oneshot.rs @@ -194,6 +194,12 @@ impl Drop for Complete { // blocking to see if it succeeded. In the latter case we don't need to // wake up anyone anyway. So in both cases it's ok to ignore the `None` // case of `try_lock` and bail out. + // + // The first case crucially depends on `Lock` using `SeqCst` ordering + // under the hood. If it instead used `Release` / `Acquire` ordering, + // then it would not necessarily synchronize with `inner.complete` + // and deadlock might be possible, as was observed in + // https://github.com/alexcrichton/futures-rs/pull/219. self.inner.complete.store(true, SeqCst); if let Some(mut slot) = self.inner.rx_task.try_lock() { if let Some(task) = slot.take() {