From e4dfd9320d87b0d9db72155eb3afa9153d0f3458 Mon Sep 17 00:00:00 2001 From: Jonathan Johnson Date: Sun, 31 Dec 2023 10:21:45 -0800 Subject: [PATCH] Fixed a possible deadlock in block_until_updated The animation doctest randomly failed on me this morning, and I isolated it to the order of the two mutexes. There's no reason to hold the deadlock mutex for more than the check for deadlock, so I switched which mutex the condvar is synchronizing using to ensure that another thread couldn't cause a deadlock by dropping a DynamicMutexGuard while the block_until_updated thread is performing its state check. --- src/value.rs | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/value.rs b/src/value.rs index 8acfa1b..57cacde 100644 --- a/src/value.rs +++ b/src/value.rs @@ -1594,16 +1594,18 @@ impl DynamicReader { /// This function panics if this value is already locked by the current /// thread. pub fn block_until_updated(&mut self) -> bool { - let mut deadlock_state = self.source.during_callback_state.lock().ignore_poison(); assert!( - deadlock_state + self.source + .during_callback_state + .lock() + .ignore_poison() .as_ref() .map_or(true, |state| state.locked_thread != std::thread::current().id()), "deadlocked" ); + let mut state = self.source.state.lock().ignore_poison(); loop { - let state = self.source.state.lock().ignore_poison(); if state.wrapped.generation != self.read_generation { return true; } else if state.readers == Arc::strong_count(&self.source) @@ -1611,10 +1613,9 @@ impl DynamicReader { { return false; } - drop(state); // Wait for a notification of a change, which is synch - deadlock_state = self.source.sync.wait(deadlock_state).ignore_poison(); + state = self.source.sync.wait(state).ignore_poison(); } }