From db46e3ba28ca56a5218723724d69b750970daa66 Mon Sep 17 00:00:00 2001 From: Alice Ryhl Date: Tue, 29 Mar 2022 11:57:33 +0200 Subject: [PATCH 1/4] Add TSAN support --- src/bytes.rs | 6 ++++++ src/bytes_mut.rs | 6 ++++++ 2 files changed, 12 insertions(+) diff --git a/src/bytes.rs b/src/bytes.rs index 753bb5c44..aa958153e 100644 --- a/src/bytes.rs +++ b/src/bytes.rs @@ -1082,8 +1082,14 @@ unsafe fn release_shared(ptr: *mut Shared) { // > "acquire" operation before deleting the object. // // [1]: (www.boost.org/doc/libs/1_55_0/doc/html/atomic/usage_examples.html) + #[cfg(not(thread = "sanitize"))] atomic::fence(Ordering::Acquire); + // Thread sanitizer does not support atomic fences. Use an atomic load + // instead. + #[cfg(thread = "sanitize")] + (*ptr).ref_count.load(Ordering::Acquire); + // Drop the data Box::from_raw(ptr); } diff --git a/src/bytes_mut.rs b/src/bytes_mut.rs index 868b955e2..675665e22 100644 --- a/src/bytes_mut.rs +++ b/src/bytes_mut.rs @@ -1287,8 +1287,14 @@ unsafe fn release_shared(ptr: *mut Shared) { // > "acquire" operation before deleting the object. // // [1]: (www.boost.org/doc/libs/1_55_0/doc/html/atomic/usage_examples.html) + #[cfg(not(thread = "sanitize"))] atomic::fence(Ordering::Acquire); + // Thread sanitizer does not support atomic fences. Use an atomic load + // instead. + #[cfg(thread = "sanitize")] + (*ptr).ref_count.load(Ordering::Acquire); + // Drop the data Box::from_raw(ptr); } From 017b8a7e08961f0b89bd968b6e477b69537eb509 Mon Sep 17 00:00:00 2001 From: Alice Ryhl Date: Thu, 31 Mar 2022 13:42:38 +0200 Subject: [PATCH 2/4] Try using AcqRel for fetch_sub --- src/bytes.rs | 10 +++++----- src/bytes_mut.rs | 10 +++++----- src/loom.rs | 2 ++ 3 files changed, 12 insertions(+), 10 deletions(-) diff --git a/src/bytes.rs b/src/bytes.rs index aa958153e..dd4e37424 100644 --- a/src/bytes.rs +++ b/src/bytes.rs @@ -1061,7 +1061,7 @@ unsafe fn shallow_clone_vec( unsafe fn release_shared(ptr: *mut Shared) { // `Shared` storage... follow the drop steps from Arc. - if (*ptr).ref_cnt.fetch_sub(1, Ordering::Release) != 1 { + if (*ptr).ref_cnt.fetch_sub(1, Ordering::AcqRel) != 1 { return; } @@ -1082,13 +1082,13 @@ unsafe fn release_shared(ptr: *mut Shared) { // > "acquire" operation before deleting the object. // // [1]: (www.boost.org/doc/libs/1_55_0/doc/html/atomic/usage_examples.html) - #[cfg(not(thread = "sanitize"))] - atomic::fence(Ordering::Acquire); + //#[cfg(not(thread = "sanitize"))] + //atomic::fence(Ordering::Acquire); // Thread sanitizer does not support atomic fences. Use an atomic load // instead. - #[cfg(thread = "sanitize")] - (*ptr).ref_count.load(Ordering::Acquire); + //#[cfg(thread = "sanitize")] + //(*ptr).ref_count.load(Ordering::Acquire); // Drop the data Box::from_raw(ptr); diff --git a/src/bytes_mut.rs b/src/bytes_mut.rs index 675665e22..1f85fced3 100644 --- a/src/bytes_mut.rs +++ b/src/bytes_mut.rs @@ -1266,7 +1266,7 @@ unsafe fn increment_shared(ptr: *mut Shared) { unsafe fn release_shared(ptr: *mut Shared) { // `Shared` storage... follow the drop steps from Arc. - if (*ptr).ref_count.fetch_sub(1, Ordering::Release) != 1 { + if (*ptr).ref_count.fetch_sub(1, Ordering::AcqRel) != 1 { return; } @@ -1287,13 +1287,13 @@ unsafe fn release_shared(ptr: *mut Shared) { // > "acquire" operation before deleting the object. // // [1]: (www.boost.org/doc/libs/1_55_0/doc/html/atomic/usage_examples.html) - #[cfg(not(thread = "sanitize"))] - atomic::fence(Ordering::Acquire); + //#[cfg(not(thread = "sanitize"))] + //atomic::fence(Ordering::Acquire); // Thread sanitizer does not support atomic fences. Use an atomic load // instead. - #[cfg(thread = "sanitize")] - (*ptr).ref_count.load(Ordering::Acquire); + //#[cfg(thread = "sanitize")] + //(*ptr).ref_count.load(Ordering::Acquire); // Drop the data Box::from_raw(ptr); diff --git a/src/loom.rs b/src/loom.rs index 1cae8812e..38596a2c7 100644 --- a/src/loom.rs +++ b/src/loom.rs @@ -1,6 +1,7 @@ #[cfg(not(all(test, loom)))] pub(crate) mod sync { pub(crate) mod atomic { + #[allow(unused_imports)] pub(crate) use core::sync::atomic::{fence, AtomicPtr, AtomicUsize, Ordering}; pub(crate) trait AtomicMut { @@ -23,6 +24,7 @@ pub(crate) mod sync { #[cfg(all(test, loom))] pub(crate) mod sync { pub(crate) mod atomic { + #[allow(unused_imports)] pub(crate) use loom::sync::atomic::{fence, AtomicPtr, AtomicUsize, Ordering}; pub(crate) trait AtomicMut {} From 876ca1db7213a85292b6e289cafa3ff8d3d8fe2b Mon Sep 17 00:00:00 2001 From: Alice Ryhl Date: Sun, 3 Apr 2022 17:16:44 +0200 Subject: [PATCH 3/4] Always use load rather than fence --- src/bytes.rs | 11 ++++------- src/bytes_mut.rs | 11 ++++------- 2 files changed, 8 insertions(+), 14 deletions(-) diff --git a/src/bytes.rs b/src/bytes.rs index dd4e37424..eee0790d3 100644 --- a/src/bytes.rs +++ b/src/bytes.rs @@ -7,7 +7,7 @@ use alloc::{borrow::Borrow, boxed::Box, string::String, vec::Vec}; use crate::buf::IntoIter; #[allow(unused)] use crate::loom::sync::atomic::AtomicMut; -use crate::loom::sync::atomic::{self, AtomicPtr, AtomicUsize, Ordering}; +use crate::loom::sync::atomic::{AtomicPtr, AtomicUsize, Ordering}; use crate::Buf; /// A cheaply cloneable and sliceable chunk of contiguous memory. @@ -1061,7 +1061,7 @@ unsafe fn shallow_clone_vec( unsafe fn release_shared(ptr: *mut Shared) { // `Shared` storage... follow the drop steps from Arc. - if (*ptr).ref_cnt.fetch_sub(1, Ordering::AcqRel) != 1 { + if (*ptr).ref_cnt.fetch_sub(1, Ordering::Release) != 1 { return; } @@ -1082,13 +1082,10 @@ unsafe fn release_shared(ptr: *mut Shared) { // > "acquire" operation before deleting the object. // // [1]: (www.boost.org/doc/libs/1_55_0/doc/html/atomic/usage_examples.html) - //#[cfg(not(thread = "sanitize"))] - //atomic::fence(Ordering::Acquire); - + // // Thread sanitizer does not support atomic fences. Use an atomic load // instead. - //#[cfg(thread = "sanitize")] - //(*ptr).ref_count.load(Ordering::Acquire); + (*ptr).ref_cnt.load(Ordering::Acquire); // Drop the data Box::from_raw(ptr); diff --git a/src/bytes_mut.rs b/src/bytes_mut.rs index 1f85fced3..af1a97570 100644 --- a/src/bytes_mut.rs +++ b/src/bytes_mut.rs @@ -16,7 +16,7 @@ use crate::buf::{IntoIter, UninitSlice}; use crate::bytes::Vtable; #[allow(unused)] use crate::loom::sync::atomic::AtomicMut; -use crate::loom::sync::atomic::{self, AtomicPtr, AtomicUsize, Ordering}; +use crate::loom::sync::atomic::{AtomicPtr, AtomicUsize, Ordering}; use crate::{Buf, BufMut, Bytes}; /// A unique reference to a contiguous slice of memory. @@ -1266,7 +1266,7 @@ unsafe fn increment_shared(ptr: *mut Shared) { unsafe fn release_shared(ptr: *mut Shared) { // `Shared` storage... follow the drop steps from Arc. - if (*ptr).ref_count.fetch_sub(1, Ordering::AcqRel) != 1 { + if (*ptr).ref_count.fetch_sub(1, Ordering::Release) != 1 { return; } @@ -1287,13 +1287,10 @@ unsafe fn release_shared(ptr: *mut Shared) { // > "acquire" operation before deleting the object. // // [1]: (www.boost.org/doc/libs/1_55_0/doc/html/atomic/usage_examples.html) - //#[cfg(not(thread = "sanitize"))] - //atomic::fence(Ordering::Acquire); - + // // Thread sanitizer does not support atomic fences. Use an atomic load // instead. - //#[cfg(thread = "sanitize")] - //(*ptr).ref_count.load(Ordering::Acquire); + (*ptr).ref_count.load(Ordering::Acquire); // Drop the data Box::from_raw(ptr); From 007fb7d63fcf14102c39c180b4fa60fe8f0fd233 Mon Sep 17 00:00:00 2001 From: Alice Ryhl Date: Sun, 3 Apr 2022 17:17:56 +0200 Subject: [PATCH 4/4] Remove #[allow(unused_imports)] --- src/loom.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/loom.rs b/src/loom.rs index 38596a2c7..9e6b2d5e2 100644 --- a/src/loom.rs +++ b/src/loom.rs @@ -1,8 +1,7 @@ #[cfg(not(all(test, loom)))] pub(crate) mod sync { pub(crate) mod atomic { - #[allow(unused_imports)] - pub(crate) use core::sync::atomic::{fence, AtomicPtr, AtomicUsize, Ordering}; + pub(crate) use core::sync::atomic::{AtomicPtr, AtomicUsize, Ordering}; pub(crate) trait AtomicMut { fn with_mut(&mut self, f: F) -> R @@ -24,8 +23,7 @@ pub(crate) mod sync { #[cfg(all(test, loom))] pub(crate) mod sync { pub(crate) mod atomic { - #[allow(unused_imports)] - pub(crate) use loom::sync::atomic::{fence, AtomicPtr, AtomicUsize, Ordering}; + pub(crate) use loom::sync::atomic::{AtomicPtr, AtomicUsize, Ordering}; pub(crate) trait AtomicMut {} }