From 6a1f7f5462c878cce72cea5356acc4083b098c3d Mon Sep 17 00:00:00 2001 From: Jonathan Johnson Date: Thu, 13 Jun 2024 09:12:10 -0700 Subject: [PATCH] Fixed deadlock with DynamicMutexGuard::unlocked During the change callback process, unlocked is called to allow the change callbacks to run while the dynamic is unlocked. The error with the previous version of this code is that the during_callback_state was always overwritten when being returned. The problem is that another thread could currently have the mutex locked and could have stored its own state -- which can happen if two threads are both trying to invoke change callbacks at the same time. By moving the state saving and reloading to only happen when the mutex guard is acquired, we can ensure that interleaving threads will work correctly. --- CHANGELOG.md | 2 ++ src/value.rs | 9 ++++----- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2252b91..93db39d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 window. - `CallbackHandle` now has a `must_use` hint that might help users discover the persist function. +- Fixed a deadlock that could occur when multiple threads were attempting to + execute change callbacks for the same dynamic at the same time. ### Added diff --git a/src/value.rs b/src/value.rs index f54a927..b136053 100644 --- a/src/value.rs +++ b/src/value.rs @@ -1437,11 +1437,10 @@ where impl<'a, T> DynamicMutexGuard<'a, T> { fn unlocked(&mut self, while_unlocked: impl FnOnce()) { - MutexGuard::unlocked(&mut self.guard, || { - let current_state = self.dynamic.during_callback_state.lock().take(); - while_unlocked(); - *self.dynamic.during_callback_state.lock() = current_state; - }); + let previous_state = self.dynamic.during_callback_state.lock().take(); + MutexGuard::unlocked(&mut self.guard, while_unlocked); + + *self.dynamic.during_callback_state.lock() = previous_state; } }