Dynamic::compare_swap + Validations now block

This sounds like a regression, but it was masking a "race condition".
DynamicGuard runs the change callbacks on drop in a background thread.
Validations was using the guard to not have to lock twice.

This led to an issue where the invalid count might be non-zero due to
the callbacks not being invoked, preventing the closure from being
invoked even though there are no validation errors.

Introducing compare_swap gives a higher-level API for Validations to
use, and it also ensures the callbacks are able to be run in the current
thread.

CHANGELOG.md

@@ -28,6 +28,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 - `Validations::validate_result` attaches a `Dynamic<Result<T,E>>` to the
   validations. This was already available on `when` conditioned validations.
+- `Dynamic::[try_]compare_swap` allows swapping the contents of a dynamic after
+  verifying the current contents.
 
 ## v0.1.3 (2023-12-19)
 

src/value.rs

@@ -494,6 +494,59 @@ impl<T> Dynamic<T> {
         let _old = self.replace(new_value);
     }
 
+    /// Replaces the current value with `new_value` if the current value is
+    /// equal to `expected_current`.
+    ///
+    /// Returns `Ok` with the overwritten value upon success.
+    ///
+    /// # Errors
+    ///
+    /// - [`TryCompareSwapError::Deadlock`]: This operation would result in a
+    ///       thread deadlock.
+    /// - [`TryCompareSwapError::CurrentValueMismatch`]: The current value did
+    ///       not match `expected_current`. The `T` returned is a clone of the
+    ///       currently stored value.
+    pub fn try_compare_swap(
+        &self,
+        expected_current: &T,
+        new_value: T,
+    ) -> Result<T, TryCompareSwapError<T>>
+    where
+        T: Clone + PartialEq,
+    {
+        match self.0.map_mut(|value, changed| {
+            if value == expected_current {
+                Ok(std::mem::replace(value, new_value))
+            } else {
+                *changed = false;
+                Err(TryCompareSwapError::CurrentValueMismatch(value.clone()))
+            }
+        }) {
+            Ok(old) => old,
+            Err(_) => Err(TryCompareSwapError::Deadlock),
+        }
+    }
+
+    /// Replaces the current value with `new_value` if the current value is
+    /// equal to `expected_current`.
+    ///
+    /// Returns `Ok` with the overwritten value upon success.
+    ///
+    /// # Errors
+    ///
+    /// Returns `Err` with the currently stored value when `expected_current`
+    /// does not match the currently stored value.
+    pub fn compare_swap(&self, expected_current: &T, new_value: T) -> Result<T, T>
+    where
+        T: Clone + PartialEq,
+    {
+        match self.try_compare_swap(expected_current, new_value) {
+            Ok(old) => Ok(old),
+            Err(TryCompareSwapError::Deadlock) => unreachable!("deadlocked"),
+            Err(TryCompareSwapError::CurrentValueMismatch(value)) => Err(value),
+        }
+    }
+
     /// Returns a new reference-based reader for this dynamic value.
     ///
     /// # Panics
@@ -618,6 +671,17 @@ impl<T> Dynamic<T> {
     }
 }
 
+/// An error returned from [`Dynamic::try_compare_swap`].
+#[derive(Debug, Clone, Eq, PartialEq)]
+pub enum TryCompareSwapError<T> {
+    /// The dynamic is already locked for exclusive access by the current
+    /// thread. This operation would result in a deadlock.
+    Deadlock,
+    /// The current value did not match the expected value. This variant's value
+    /// is the value at the time of comparison.
+    CurrentValueMismatch(T),
+}
+
 impl<T> Debug for Dynamic<T>
 where
     T: Debug,
@@ -2361,11 +2425,9 @@ impl Validations {
         F: FnMut(T) -> R + Send + 'static,
         R: Default,
     {
-        let mut state = self.state.lock();
-        if let ValidationsState::Initial = &*state {
-            *state = ValidationsState::Checked;
-        }
-        drop(state);
+        let _result = self
+            .state
+            .compare_swap(&ValidationsState::Initial, ValidationsState::Checked);
         if self.invalid.get() == 0 {
             handler(t)
         } else {
@@ -2552,3 +2614,12 @@ fn map_cycle_is_finite() {
     a.set(1);
     assert_eq!(a.get(), 2);
 }
+
+#[test]
+fn compare_swap() {
+    let dynamic = Dynamic::new(1);
+    assert_eq!(dynamic.compare_swap(&1, 2), Ok(1));
+    assert_eq!(dynamic.compare_swap(&1, 0), Err(2));
+    assert_eq!(dynamic.compare_swap(&2, 0), Ok(2));
+    assert_eq!(dynamic.get(), 0);
+}