Skip to content

Commit 0731080

Browse files
Revert #958 (#1039)
This PR can create deadlocks - there is nothing synchronizing the mutex/condvar with the `Arc` of the `Zalsa`, therefore code may wake up by the `Condvar` but not see an updated refcount, go to sleep again, and never wake up.
1 parent 0a3eec6 commit 0731080

File tree

1 file changed

+21
-20
lines changed

1 file changed

+21
-20
lines changed

src/storage.rs

Lines changed: 21 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@ pub struct StorageHandle<Db> {
2525

2626
impl<Db> Clone for StorageHandle<Db> {
2727
fn clone(&self) -> Self {
28+
*self.coordinate.clones.lock() += 1;
29+
2830
Self {
2931
zalsa_impl: self.zalsa_impl.clone(),
3032
coordinate: CoordinateDrop(Arc::clone(&self.coordinate)),
@@ -51,7 +53,7 @@ impl<Db: Database> StorageHandle<Db> {
5153
Self {
5254
zalsa_impl: Arc::new(Zalsa::new::<Db>(event_callback, jars)),
5355
coordinate: CoordinateDrop(Arc::new(Coordinate {
54-
coordinate_lock: Mutex::default(),
56+
clones: Mutex::new(1),
5557
cvar: Default::default(),
5658
})),
5759
phantom: PhantomData,
@@ -93,6 +95,17 @@ impl<Db> Drop for Storage<Db> {
9395
}
9496
}
9597

98+
struct Coordinate {
99+
/// Counter of the number of clones of actor. Begins at 1.
100+
/// Incremented when cloned, decremented when dropped.
101+
clones: Mutex<usize>,
102+
cvar: Condvar,
103+
}
104+
105+
// We cannot panic while holding a lock to `clones: Mutex<usize>` and therefore we cannot enter an
106+
// inconsistent state.
107+
impl RefUnwindSafe for Coordinate {}
108+
96109
impl<Db: Database> Default for Storage<Db> {
97110
fn default() -> Self {
98111
Self::new(None)
@@ -155,15 +168,12 @@ impl<Db: Database> Storage<Db> {
155168
.zalsa_impl
156169
.event(&|| Event::new(EventKind::DidSetCancellationFlag));
157170

158-
let mut coordinate_lock = self.handle.coordinate.coordinate_lock.lock();
159-
let zalsa = loop {
160-
if Arc::strong_count(&self.handle.zalsa_impl) == 1 {
161-
// SAFETY: The strong count is 1, and we never create any weak pointers,
162-
// so we have a unique reference.
163-
break unsafe { &mut *(Arc::as_ptr(&self.handle.zalsa_impl).cast_mut()) };
164-
}
165-
coordinate_lock = self.handle.coordinate.cvar.wait(coordinate_lock);
166-
};
171+
let mut clones = self.handle.coordinate.clones.lock();
172+
while *clones != 1 {
173+
clones = self.handle.coordinate.cvar.wait(clones);
174+
}
175+
// The ref count on the `Arc` should now be 1
176+
let zalsa = Arc::get_mut(&mut self.handle.zalsa_impl).unwrap();
167177
// cancellation is done, so reset the flag
168178
zalsa.runtime_mut().reset_cancellation_flag();
169179
zalsa
@@ -250,16 +260,6 @@ impl<Db: Database> Clone for Storage<Db> {
250260
}
251261
}
252262

253-
/// A simplified `WaitGroup`, this is used together with `Arc<Zalsa>` as the actual counter
254-
struct Coordinate {
255-
coordinate_lock: Mutex<()>,
256-
cvar: Condvar,
257-
}
258-
259-
// We cannot panic while holding a lock to `clones: Mutex<usize>` and therefore we cannot enter an
260-
// inconsistent state.
261-
impl RefUnwindSafe for Coordinate {}
262-
263263
struct CoordinateDrop(Arc<Coordinate>);
264264

265265
impl std::ops::Deref for CoordinateDrop {
@@ -272,6 +272,7 @@ impl std::ops::Deref for CoordinateDrop {
272272

273273
impl Drop for CoordinateDrop {
274274
fn drop(&mut self) {
275+
*self.0.clones.lock() -= 1;
275276
self.0.cvar.notify_all();
276277
}
277278
}

0 commit comments

Comments
 (0)