Is it okay to use Mutex<bool>?

69

After this long discussion, I decided to actually use the AtomicBool approach (source). In that context, I also invested some effort to add SAFETY comments to all uses of unsafe in mmtkvdb's code. I found it very helpful to enable the unsafe_op_in_unsafe_fn lint.

Adding the SAFETY comments, I found several soundness issues (critical bug fixes in mmtkvdb 0.14.1). It turned out that doing FFI calls in a sound way can be a really hard task. Especially LMDB comes with a lot of preconditions combined with sometimes-automatic releasing of resources. One of the more twisted examples from the LMDB API specification:

mdb_cursor_open():

A cursor cannot be used when its database handle is closed. Nor when its transaction has ended, except with mdb_cursor_renew(). It can be discarded with mdb_cursor_close(). A cursor in a write-transaction can be closed before its transaction ends, and will otherwise be closed when its transaction ends. A cursor in a read-only transaction must be closed explicitly, before or after its transaction ends. It can be reused with mdb_cursor_renew() before finally closing it.

Note: Earlier documentation said that cursors in every transaction were closed when the transaction committed or aborted.

:face_with_spiral_eyes:

What I (think I) learned from this discussion and from reviewing all my unsafe uses:

  • Mutex<bool> gives guarantees over AtomicBool. Often, the synchronization properties of Mutex<bool> might not be needed. But if you need them and you want to avoid using a Mutex, then you'll have to use more complex patterns using AtomicBool, rather than simply using SeqCst. SeqCst loads and stores most likely will not give you what you need. And remember: SeqCst only gives any advantage over Acquire/Release/AcqRel if there is more than one atomic involved!
  • Sometimes, synchronization happens implicitly. An example is malloc/free in C. But there may be other cases, e.g. Tokio's Notify, which may result in synchronization (see also this post by me in "A flag type that supports waiting asynchronously"). I think many APIs don't explicitly specify when or if such synchronization takes place. Being careful about that, one might end up using a lot of unnecessary extra-synchronization (as in my case of using Mutex<bool> where it (likely) wasn't necessary).
  • C API specifications, in particular, are often underspecified. You might need to make certain (reasonable) assumptions that aren't explicitly documented.
  • Writing sound unsafe code in Rust is harder than I imagined. Explicitly writing down the reason why each and every unsafe block is sound can be helpful, even if it seems "obvious" at a first glance. Using the unsafe_op_in_unsafe_fn lint is a valuable tool as well.
  • Of course, Mutex<bool> is a safe fallback. If you use that, you don't need to worry about synchronization. You might still endup with deadlocks though.
2 Likes