Is it okay to use Mutex<bool>?

[jbe]

@nbaraz

Mutex has 4 states: locked + true, locked + false, unlocked + true, unlocked + false. This is not always necessary.

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:

@FRGH

IMO if it's difficult for some reason to put your data inside the mutex, then you should just use Mutex<()> (and keep closed as it's own field) […]

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>:

@Heliozoa

async fn reset_and_seed_db() {
    let mut initialised = INITIALISED.lock().await;
    if *initialised {
        return;
    }
    
    // do stuff

    *initialised = true;
}

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; edit²: except if we make the Bool an AtomicBool or wrap it in yet another Mutex ).

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.

---

@FRGH

[…] you should just use Mutex<()> (and keep closed as it's own field) […]

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 () ↩︎