More thorough tests for when slots are full

Emulating them being full by a fake strategy.

Fixing releasing stuff.

Cargo.toml

@@ -19,6 +19,8 @@ maintenance = { status = "actively-developed" }
 [features]
 # ArcSwapWeak (for std::sycn::Weak) support
 weak = []
+# Some strategies used for testing few internal cornercases. *DO NOT USE* (no stability guarantees and their performance is likely very bad).
+internal-test-strategies = []
 
 [dependencies]
 

src/debt/mod.rs

@@ -3,8 +3,7 @@
 use std::sync::atomic::AtomicUsize;
 use std::sync::atomic::Ordering::*;
 
-pub(crate) use self::list::LocalNode;
-use self::list::Node;
+pub(crate) use self::list::{LocalNode, Node};
 use super::RefCnt;
 
 mod fast;
@@ -14,15 +13,16 @@ mod list;
 /// One debt slot.
 ///
 /// It may contain an „owed“ reference count.
-pub(crate) struct Debt(AtomicUsize);
+#[derive(Debug)]
+pub(crate) struct Debt(pub(crate) AtomicUsize);
 
 impl Debt {
     /// The value of pointer `3` should be pretty safe, for two reasons:
     ///
     /// * It's an odd number, but the pointers we have are likely aligned at least to the word size,
     ///   because the data at the end of the `Arc` has the counters.
     /// * It's in the very first page where NULL lives, so it's not mapped.
-    const NONE: usize = 0b11;
+    pub(crate) const NONE: usize = 0b11;
 }
 
 impl Default for Debt {

src/lib.rs

@@ -771,7 +771,9 @@ macro_rules! t {
 
             const ITERATIONS: usize = 10;
 
+            #[allow(deprecated)] // We use "deprecated" testing strategies in here.
             type AS<T> = ArcSwapAny<Arc<T>, $strategy>;
+            #[allow(deprecated)] // We use "deprecated" testing strategies in here.
             type ASO<T> = ArcSwapAny<Option<Arc<T>>, $strategy>;
 
             /// Similar to the one in doc tests of the lib, but more times and more intensive (we
@@ -899,66 +901,6 @@ macro_rules! t {
                 .unwrap();
             }
 
-            /// Accessing the value inside ArcSwap with Guards (and checks for the reference
-            /// counts).
-            #[test]
-            fn load_cnt() {
-                let a = Arc::new(0);
-                let shared = AS::from(Arc::clone(&a));
-                // One in shared, one in a
-                assert_eq!(2, Arc::strong_count(&a));
-                let guard = shared.load();
-                assert_eq!(0, **guard);
-                // The guard doesn't have its own ref count now
-                assert_eq!(2, Arc::strong_count(&a));
-                let guard_2 = shared.load();
-                // Unlike with guard, this does not deadlock
-                shared.store(Arc::new(1));
-                // But now, each guard got a full Arc inside it
-                assert_eq!(3, Arc::strong_count(&a));
-                // And when we get rid of them, they disappear
-                drop(guard_2);
-                assert_eq!(2, Arc::strong_count(&a));
-                let _b = Arc::clone(&guard);
-                assert_eq!(3, Arc::strong_count(&a));
-                // We can drop the guard it came from
-                drop(guard);
-                assert_eq!(2, Arc::strong_count(&a));
-                let guard = shared.load();
-                assert_eq!(1, **guard);
-                drop(shared);
-                // We can still use the guard after the shared disappears
-                assert_eq!(1, **guard);
-                let ptr = Arc::clone(&guard);
-                // One in shared, one in guard
-                assert_eq!(2, Arc::strong_count(&ptr));
-                drop(guard);
-                assert_eq!(1, Arc::strong_count(&ptr));
-            }
-
-            /// There can be only limited amount of leases on one thread. Following ones are
-            /// created, but contain full Arcs.
-            #[test]
-            fn lease_overflow() {
-                let a = Arc::new(0);
-                let shared = AS::from(Arc::clone(&a));
-                assert_eq!(2, Arc::strong_count(&a));
-                let mut guards = (0..1000).map(|_| shared.load()).collect::<Vec<_>>();
-                let count = Arc::strong_count(&a);
-                assert!(count > 2);
-                let guard = shared.load();
-                assert_eq!(count + 1, Arc::strong_count(&a));
-                drop(guard);
-                assert_eq!(count, Arc::strong_count(&a));
-                // When we delete the first one, it didn't have an Arc in it, so the ref count
-                // doesn't drop
-                guards.swap_remove(0);
-                assert_eq!(count, Arc::strong_count(&a));
-                // But new one reuses now vacant the slot and doesn't create a new Arc
-                let _guard = shared.load();
-                assert_eq!(count, Arc::strong_count(&a));
-            }
-
             #[test]
             fn load_null() {
                 let shared = ASO::<usize>::default();
@@ -1236,3 +1178,75 @@ macro_rules! t {
 }
 
 t!(tests_default, DefaultStrategy);
+#[cfg(feature = "internal-test-strategies")]
+#[allow(deprecated)]
+t!(
+    tests_full_slots,
+    crate::strategy::test_strategies::FillFastSlots
+);
+
+/// These tests assume details about the used strategy.
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    /// Accessing the value inside ArcSwap with Guards (and checks for the reference
+    /// counts).
+    #[test]
+    fn load_cnt() {
+        let a = Arc::new(0);
+        let shared = ArcSwap::from(Arc::clone(&a));
+        // One in shared, one in a
+        assert_eq!(2, Arc::strong_count(&a));
+        let guard = shared.load();
+        assert_eq!(0, **guard);
+        // The guard doesn't have its own ref count now
+        assert_eq!(2, Arc::strong_count(&a));
+        let guard_2 = shared.load();
+        // Unlike with guard, this does not deadlock
+        shared.store(Arc::new(1));
+        // But now, each guard got a full Arc inside it
+        assert_eq!(3, Arc::strong_count(&a));
+        // And when we get rid of them, they disappear
+        drop(guard_2);
+        assert_eq!(2, Arc::strong_count(&a));
+        let _b = Arc::clone(&guard);
+        assert_eq!(3, Arc::strong_count(&a));
+        // We can drop the guard it came from
+        drop(guard);
+        assert_eq!(2, Arc::strong_count(&a));
+        let guard = shared.load();
+        assert_eq!(1, **guard);
+        drop(shared);
+        // We can still use the guard after the shared disappears
+        assert_eq!(1, **guard);
+        let ptr = Arc::clone(&guard);
+        // One in shared, one in guard
+        assert_eq!(2, Arc::strong_count(&ptr));
+        drop(guard);
+        assert_eq!(1, Arc::strong_count(&ptr));
+    }
+
+    /// There can be only limited amount of leases on one thread. Following ones are
+    /// created, but contain full Arcs.
+    #[test]
+    fn lease_overflow() {
+        let a = Arc::new(0);
+        let shared = ArcSwap::from(Arc::clone(&a));
+        assert_eq!(2, Arc::strong_count(&a));
+        let mut guards = (0..1000).map(|_| shared.load()).collect::<Vec<_>>();
+        let count = Arc::strong_count(&a);
+        assert!(count > 2);
+        let guard = shared.load();
+        assert_eq!(count + 1, Arc::strong_count(&a));
+        drop(guard);
+        assert_eq!(count, Arc::strong_count(&a));
+        // When we delete the first one, it didn't have an Arc in it, so the ref count
+        // doesn't drop
+        guards.swap_remove(0);
+        assert_eq!(count, Arc::strong_count(&a));
+        // But new one reuses now vacant the slot and doesn't create a new Arc
+        let _guard = shared.load();
+        assert_eq!(count, Arc::strong_count(&a));
+    }
+}

src/strategy/hybrid.rs

@@ -61,7 +61,9 @@ impl<T: RefCnt> HybridProtection<T> {
             Err((unused_debt, replacement)) => {
                 // The debt is on the candidate we provided and it is unused, we so we just pay it
                 // back right away.
-                unused_debt.pay::<T>(candidate);
+                if !unused_debt.pay::<T>(candidate) {
+                    unsafe { T::dec(candidate) };
+                }
                 // We got a (possibly) different pointer out. But that one is already protected and
                 // the slot is paid back.
                 unsafe { Self::new(replacement as *mut _, None) }
@@ -131,18 +133,36 @@ impl<T: RefCnt> Borrow<T> for HybridProtection<T> {
     }
 }
 
+pub trait Config {
+    const USE_FAST: bool;
+}
+
+#[derive(Clone, Default)]
+pub struct DefaultConfig;
+
+impl Config for DefaultConfig {
+    const USE_FAST: bool = true;
+}
+
 #[derive(Clone, Default)]
-pub struct HybridStrategy;
+pub struct HybridStrategy<Cfg> {
+    _config: Cfg,
+}
 
-impl<T> InnerStrategy<T> for HybridStrategy
+impl<T, Cfg> InnerStrategy<T> for HybridStrategy<Cfg>
 where
     T: RefCnt,
+    Cfg: Config,
 {
     type Protected = HybridProtection<T>;
     unsafe fn load(&self, storage: &AtomicPtr<T::Base>) -> Self::Protected {
         LocalNode::with(|node| {
-            HybridProtection::attempt(node, storage)
-                .unwrap_or_else(|| HybridProtection::fallback(node, storage))
+            let fast = if Cfg::USE_FAST {
+                HybridProtection::attempt(node, storage)
+            } else {
+                None
+            };
+            fast.unwrap_or_else(|| HybridProtection::fallback(node, storage))
         })
     }
     unsafe fn wait_for_readers(&self, old: *const T::Base, storage: &AtomicPtr<T::Base>) {
@@ -155,7 +175,7 @@ where
     }
 }
 
-impl<T: RefCnt> CaS<T> for HybridStrategy {
+impl<T: RefCnt, Cfg: Config> CaS<T> for HybridStrategy<Cfg> {
     unsafe fn compare_and_swap<C: crate::as_raw::AsRaw<T::Base>>(
         &self,
         storage: &AtomicPtr<T::Base>,

src/strategy/mod.rs

@@ -45,8 +45,12 @@ use crate::ref_cnt::RefCnt;
 mod helping;
 mod hybrid;
 mod rw_lock;
+// Do not use from outside of the crate.
+#[cfg(feature = "internal-test-strategies")]
+#[doc(hidden)]
+pub mod test_strategies;
 
-use self::hybrid::HybridStrategy;
+use self::hybrid::{DefaultConfig, HybridStrategy};
 
 /// The default strategy.
 ///
@@ -74,7 +78,7 @@ use self::hybrid::HybridStrategy;
 /// similar or better properties.
 ///
 /// [`load`]: crate::ArcSwapAny::load
-pub type DefaultStrategy = HybridStrategy;
+pub type DefaultStrategy = HybridStrategy<DefaultConfig>;
 
 /// Strategy for isolating instances.
 ///

src/strategy/test_strategies.rs

@@ -0,0 +1,22 @@
+#![deprecated(note = "Only for internal testing. Do not use")]
+#![allow(deprecated)] // We need to allow ourselves the stuff we deprecate here.
+//! Some strategies for internal testing.
+//!
+//! # Warning
+//!
+//! They come with no guarantees of correctness, stability, performance or anything at all. *DO NOT
+//! USE*.
+
+use super::hybrid::{Config, HybridStrategy};
+
+/// Config for no fast slots.
+#[derive(Clone, Copy, Default)]
+pub struct NoFastSlots;
+
+impl Config for NoFastSlots {
+    const USE_FAST: bool = false;
+}
+
+/// A strategy that fills the slots with some crap to make sure we test the fallbacks too.
+#[deprecated(note = "Only for internal testing. Do not use")]
+pub type FillFastSlots = HybridStrategy<NoFastSlots>;

src/weak.rs

@@ -62,6 +62,7 @@ macro_rules! t {
 
             use crate::ArcSwapAny;
 
+            #[allow(deprecated)] // We use "deprecated" testing strategies in here.
             type ArcSwapWeak<T> = ArcSwapAny<Weak<T>, $strategy>;
 
             // Convert to weak, push it through the shared and pull it out again.
@@ -78,8 +79,6 @@ macro_rules! t {
 
             // Replace a weak pointer with a NULL one
             #[test]
-            // Miri-bug in std, see https://github.com/rust-lang/rust/issues/80365
-            //#[cfg_attr(miri, ignore)]
             fn reset() {
                 let data = Arc::new("Hello");
                 let shared = ArcSwapWeak::new(Arc::downgrade(&data));
@@ -111,3 +110,8 @@ macro_rules! t {
 }
 
 t!(tests_default, crate::DefaultStrategy);
+#[cfg(feature = "internal-test-strategies")]
+t!(
+    tests_full_slots,
+    crate::strategy::test_strategies::FillFastSlots
+);

tests/random.rs

@@ -3,7 +3,7 @@
 use std::mem;
 use std::sync::Arc;
 
-use arc_swap::{ArcSwap, ArcSwapAny, DefaultStrategy, IndependentStrategy};
+use arc_swap::{ArcSwapAny, DefaultStrategy, IndependentStrategy};
 use once_cell::sync::Lazy;
 use proptest::prelude::*;
 
@@ -59,6 +59,7 @@ macro_rules! t {
                 instructions in proptest::collection::vec(SelInstruction::random(), 1..SIZE),
             ) {
                 let mut bare = Arc::clone(&ARCS[0]);
+                #[allow(deprecated)] // We use "deprecated" testing strategies in here.
                 let a = ArcSwapAny::<_, $strategy>::from(Arc::clone(&ARCS[0]));
                 for ins in instructions {
                     match ins {
@@ -96,7 +97,8 @@ macro_rules! t {
                 ) {
                     use crate::OpsInstruction::*;
                     let mut m = 0;
-                    let a = ArcSwap::from_pointee(0usize);
+                    #[allow(deprecated)] // We use "deprecated" testing strategies in here.
+                    let a = ArcSwapAny::<_, $strategy>::new(Arc::new(0usize));
                     for ins in instructions {
                         match ins {
                             Store(v) => {
@@ -119,3 +121,5 @@ macro_rules! t {
 
 t!(@full => default, DefaultStrategy);
 t!(@full => independent, IndependentStrategy);
+#[cfg(feature = "internal-test-strategies")]
+t!(@full => full_slots, arc_swap::strategy::test_strategies::FillFastSlots);