From 426e07a992aa85983f9825e7d297a0bb8a82ddb7 Mon Sep 17 00:00:00 2001 From: Eliza Weisman Date: Fri, 1 Oct 2021 09:44:27 -0700 Subject: [PATCH 1/3] core: additional deprecation fixes --- tracing-core/src/spin/once.rs | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/tracing-core/src/spin/once.rs b/tracing-core/src/spin/once.rs index 219eb31540..fe4b53f183 100644 --- a/tracing-core/src/spin/once.rs +++ b/tracing-core/src/spin/once.rs @@ -76,10 +76,11 @@ impl Once { let mut status = self.state.load(Ordering::SeqCst); if status == INCOMPLETE { - status = self + if self .state - .compare_and_swap(INCOMPLETE, RUNNING, Ordering::SeqCst); - if status == INCOMPLETE { + .compare_exchange(INCOMPLETE, RUNNING, Ordering::SeqCst, Ordering::SeqCst) + .is_ok() + { // We init // We use a guard (Finish) to catch panics caused by builder let mut finish = Finish { @@ -101,6 +102,8 @@ impl Once { match status { INCOMPLETE => unreachable!(), RUNNING => { + // TODO(eliza): replace with `core::hint::spin_loop` once our MSRV supports it. + #[allow(deprecated)] // We spin cpu_relax(); status = self.state.load(Ordering::SeqCst) @@ -126,7 +129,12 @@ impl Once { loop { match self.state.load(Ordering::SeqCst) { INCOMPLETE => return None, - RUNNING => cpu_relax(), // We spin + + RUNNING => { + // TODO(eliza): replace with `core::hint::spin_loop` once our MSRV supports it. + #[allow(deprecated)] + cpu_relax() // We spin + } COMPLETE => return Some(self.force_get()), PANICKED => panic!("Once has panicked"), _ => unsafe { unreachable() }, From 22c7ea6c5a6bd20648439cf1e68b814e10472804 Mon Sep 17 00:00:00 2001 From: Eliza Weisman Date: Fri, 1 Oct 2021 09:53:45 -0700 Subject: [PATCH 2/3] fix status not being updated if cas fails Signed-off-by: Eliza Weisman --- tracing-core/src/spin/once.rs | 46 ++++++++++++++++++++--------------- 1 file changed, 27 insertions(+), 19 deletions(-) diff --git a/tracing-core/src/spin/once.rs b/tracing-core/src/spin/once.rs index fe4b53f183..096d9f11cf 100644 --- a/tracing-core/src/spin/once.rs +++ b/tracing-core/src/spin/once.rs @@ -76,25 +76,33 @@ impl Once { let mut status = self.state.load(Ordering::SeqCst); if status == INCOMPLETE { - if self - .state - .compare_exchange(INCOMPLETE, RUNNING, Ordering::SeqCst, Ordering::SeqCst) - .is_ok() - { - // We init - // We use a guard (Finish) to catch panics caused by builder - let mut finish = Finish { - state: &self.state, - panicked: true, - }; - unsafe { *self.data.get() = Some(builder()) }; - finish.panicked = false; - - status = COMPLETE; - self.state.store(status, Ordering::SeqCst); - - // This next line is strictly an optimization - return self.force_get(); + status = match self.state.compare_exchange( + INCOMPLETE, + RUNNING, + Ordering::SeqCst, + Ordering::SeqCst, + ) { + Ok(status) => { + debug_assert_eq!( + status, INCOMPLETE, + "if compare_exchange succeeded, previous status must be incomplete", + ); + // We init + // We use a guard (Finish) to catch panics caused by builder + let mut finish = Finish { + state: &self.state, + panicked: true, + }; + unsafe { *self.data.get() = Some(builder()) }; + finish.panicked = false; + + status = COMPLETE; + self.state.store(status, Ordering::SeqCst); + + // This next line is strictly an optimization + return self.force_get(); + } + Err(s) => status = s, } } From 1879769e1355b7cbcf64acf606ca85b8058e5e07 Mon Sep 17 00:00:00 2001 From: Eliza Weisman Date: Fri, 1 Oct 2021 09:57:03 -0700 Subject: [PATCH 3/3] there we go Signed-off-by: Eliza Weisman --- tracing-core/src/spin/once.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tracing-core/src/spin/once.rs b/tracing-core/src/spin/once.rs index 096d9f11cf..8643d472b2 100644 --- a/tracing-core/src/spin/once.rs +++ b/tracing-core/src/spin/once.rs @@ -96,13 +96,12 @@ impl Once { unsafe { *self.data.get() = Some(builder()) }; finish.panicked = false; - status = COMPLETE; - self.state.store(status, Ordering::SeqCst); + self.state.store(COMPLETE, Ordering::SeqCst); // This next line is strictly an optimization return self.force_get(); } - Err(s) => status = s, + Err(status) => status, } }