diff --git a/components/salsa-2022/src/storage.rs b/components/salsa-2022/src/storage.rs index 49b0a50a7..df92ed786 100644 --- a/components/salsa-2022/src/storage.rs +++ b/components/salsa-2022/src/storage.rs @@ -48,6 +48,9 @@ struct Shared { /// Conditional variable that is used to coordinate cancellation. /// When the main thread writes to the database, it blocks until each of the snapshots can be cancelled. cvar: Arc, + + /// Mutex that is used to protect the `jars` field when waiting for snapshots to be dropped. + noti_lock: Arc>, } // ANCHOR: default @@ -62,6 +65,7 @@ where shared: Shared { jars: Some(Arc::from(jars)), cvar: Arc::new(Default::default()), + noti_lock: Arc::new(parking_lot::Mutex::new(())), }, routes: Arc::new(routes), runtime: Runtime::default(), @@ -135,19 +139,23 @@ where loop { self.runtime.set_cancellation_flag(); + // Acquire lock before we check if we have unique access to the jars. + // If we do not yet have unique access, we will go to sleep and wait for + // the snapshots to be dropped, which will signal the cond var associated + // with this lock. + // + // NB: We have to acquire the lock first to ensure that we can check for + // unique access and go to sleep waiting on the condvar atomically, + // as described in PR #474. + let mut guard = self.shared.noti_lock.lock(); // If we have unique access to the jars, we are done. if Arc::get_mut(self.shared.jars.as_mut().unwrap()).is_some() { return; } // Otherwise, wait until some other storage entities have dropped. - // We create a mutex here because the cvar api requires it, but we - // don't really need one as the data being protected is actually - // the jars above. // // The cvar `self.shared.cvar` is notified by the `Drop` impl. - let mutex = parking_lot::Mutex::new(()); - let mut guard = mutex.lock(); self.shared.cvar.wait(&mut guard); } } @@ -167,6 +175,7 @@ where Self { jars: self.jars.clone(), cvar: self.cvar.clone(), + noti_lock: self.noti_lock.clone(), } } } @@ -178,6 +187,7 @@ where fn drop(&mut self) { // Drop the Arc reference before the cvar is notified, // since other threads are sleeping, waiting for it to reach 1. + let _guard = self.shared.noti_lock.lock(); drop(self.shared.jars.take()); self.shared.cvar.notify_all(); }