Is it okay to use Mutex<bool>?

7

It's not just about the state of the mutex. It's also about its implicit synchronization guarantees (see above link regarding Mutex<()>). These guarantees are not really documented well in my opinion.

That brings me back to @FRGH's comment:

I wonder: Aren't we often requiring synchronization that goes beyond whats actually inside the mutex?

For example in the original thread, @Heliozoa proposed to use an (async) Mutex<bool>:

Here the mutex also synchronizes not just the bool but also whatever "// do stuff" does. Yet I feel like using a Mutex<()> and a separate bool would be rather confusing than helpful (edit: and, moreover, not even possible with safe Rust; edit2: except if we make the Bool an AtomicBool :sweat_smile: or wrap it in yet another Mutex :see_no_evil:).

Of course you could also use a dedicated synchronization primitive like @alice proposed here, but I still feel like it's okay (and idiomatic) to rely on Mutex<T> synchronizing more than T[1]. Not sure though.

As I pointed out above when writing "not even possible with safe Rust", it will not work unless I make the Bool and AtomicBool or wrap it in yet another Mutex or use UnsafeCell:

@@ -331,12 +331,12 @@ impl<'a> TxnBackend<'a> {
     fn close_cursors(&mut self) {
         for cursor in take(&mut self.cursors).iter() {
             // NOTE: mutex should never be poisoned
-            let mut closed = cursor.closed.lock().unwrap();
-            *closed = true;
+            let guard = cursor.mutex.lock().unwrap();
+            cursor.closed = true;
             unsafe {
                 lmdb::mdb_cursor_close(cursor.inner);
             }
-            drop(closed);
+            drop(guard);
         }
     }
 }
@@ -542,7 +542,8 @@ impl<K: ?Sized, V: ?Sized, C> Eq for Db<K, V, C> {}
 #[derive(Debug)]
 struct CursorBackend {
     inner: *mut lmdb::MDB_cursor,
-    closed: Mutex<bool>,
+    mutex: Mutex<()>,
+    closed: bool,
     inner_txn: *mut lmdb::MDB_txn,
 }
 
@@ -552,20 +553,21 @@ unsafe impl Sync for CursorBackend {}
 impl CursorBackend {
     /// Panic if cursor handle does not belong to transaction
     fn assert_txn_backend(&self, txn_backend: &TxnBackend) {
-        let closed = self.closed.lock().unwrap();
-        if *closed || self.inner_txn != txn_backend.inner {
+        let guard = self.mutex.lock().unwrap();
+        if self.closed || self.inner_txn != txn_backend.inner {
             // Avoid mutex poisoning by dropping mutex guard before panicking
-            drop(closed);
+            drop(guard);
             panic!("cursor does not belong to transaction");
         }
-        drop(closed);
+        drop(guard);
     }
 }
 
 impl Drop for CursorBackend {
     fn drop(&mut self) {
         // NOTE: mutex should never be poisoned
-        if !*self.closed.get_mut().unwrap() {
+        self.mutex.get_mut().unwrap();
+        if !self.closed {
             unsafe {
                 lmdb::mdb_cursor_close(self.inner);
             }
@@ -1576,6 +1578,7 @@ fn new_cursor<K, V, C>(&mut self, db: &Db<K, V, C>) -> io::Result<Cursor<K, V, C
             let inner_cursor = inner_cursor.assume_init();
             let cursor_backend = Arc::new(CursorBackend {
                 inner: inner_cursor,
+                mutex: Default::default(),
                 closed: Default::default(),
                 inner_txn: self.inner,
             });

Compiler errors:

error[E0594]: cannot assign to data in an `Arc`
   --> src/lib.rs:335:13
    |
335 |             cursor.closed = true;
    |             ^^^^^^^^^^^^^^^^^^^^ cannot assign
    |
    = help: trait `DerefMut` is required to modify through a dereference, but it is not implemented for `Arc<CursorBackend>`

For more information about this error, try `rustc --explain E0594`.
error: could not compile `mmtkvdb` due to previous error

Thus I feel like using Mutex<bool> is the best choice here, even if synchronization also covers the raw pointer(s) in addition to the bool.

  1. even if T is distinct from () ↩︎