Refinements to write lock support.

* Seal the WriteLockStrategy trait to avoid committing to a specific
  interface and avoid exposing lock implementations as part of the
  crate's public interface.
* Several updates responding to PR feedback including parameter
  order, type names, whitespace, additional test, and additional
  documentation.

benches/synchronizer.rs

@@ -8,7 +8,7 @@ use rkyv::{AlignedVec, Archive, Deserialize, Serialize};
 use wyhash::WyHash;
 
 #[cfg(unix)]
-use mmap_sync::locks::{Disabled, SingleWriter};
+use mmap_sync::locks::{LockDisabled, SingleWriter};
 use mmap_sync::synchronizer::Synchronizer;
 /// Example data-structure shared between writer and reader(s)
 #[derive(Archive, Deserialize, Serialize, Debug, PartialEq)]
@@ -68,15 +68,17 @@ pub fn bench_synchronizer(c: &mut Criterion) {
 
 #[cfg(unix)]
 fn build_synchronizers_for_strategies() -> (
-    Synchronizer<WyHash, 1024, 1_000_000_000, Disabled>,
-    Synchronizer<WyHash, 1024, 1_000_000_000, SingleWriter>,
+    Synchronizer<WyHash, LockDisabled, 1024, 1_000_000_000, >,
+    Synchronizer<WyHash, SingleWriter, 1024, 1_000_000_000>,
 ) {
     let disabled_path = "/dev/shm/mmap_sync_lock_disabled";
     let single_writer_path = "/dev/shm/mmap_sync_lock_single_writer";
 
     (
-        Synchronizer::<WyHash, 1024, 1_000_000_000, Disabled>::with_params(disabled_path.as_ref()),
-        Synchronizer::<WyHash, 1024, 1_000_000_000, SingleWriter>::with_params(
+        Synchronizer::<WyHash, LockDisabled, 1024, 1_000_000_000,>::with_params(
+            disabled_path.as_ref(),
+        ),
+        Synchronizer::<WyHash, SingleWriter,1024, 1_000_000_000>::with_params(
             single_writer_path.as_ref(),
         ),
     )

src/locks.rs

@@ -1,4 +1,13 @@
 //! Lock strategies.
+//!
+//! # Safety
+//! This crate requires that only one writer be active at a time. It is the caller's responsibility
+//! to uphold th is guarantee.
+//!
+//! Note: if multiple writers are active, it is the caller's responsibility to ensure that each
+//! writer is configured with an appropriate lock strategy. For example, mixing the `LockDisabled`
+//! strategy with any other strategy is incorrect because it disables lock checks from one of the
+//! synchronizers.
 
 #[cfg(unix)]
 use std::os::fd::AsRawFd;
@@ -13,7 +22,13 @@ use crate::synchronizer::SynchronizerError;
 
 /// The write lock strategy supports different lock implementations which can be chosen based on
 /// the guarantees required, platform support, and performance constraints.
-pub trait WriteLockStrategy<'a> {
+///
+/// Note: the lock implementations are sealed to avoid committing to a specific lock interface.
+#[allow(private_bounds)]
+pub trait WriteLockStrategy<'a>: WriteLockStrategySealed<'a> {}
+
+/// Sealed trait i
+pub(crate) trait WriteLockStrategySealed<'a> {
     type Guard: DerefMut<Target = MmapMut> + 'a;
 
     /// Create a new instance of this lock strategy.
@@ -40,9 +55,9 @@ pub trait WriteLockStrategy<'a> {
 /// Callers must ensure that there is only a single active writer. For example, the caller might
 /// ensure that only one process attempts to write to the synchronizer, and ensure that multiple
 /// instances of the process are not spawned.
-pub struct Disabled(MmapMut);
+pub struct LockDisabled(MmapMut);
 
-impl<'a> WriteLockStrategy<'a> for Disabled {
+impl<'a> WriteLockStrategySealed<'a> for LockDisabled {
     type Guard = DisabledGuard<'a>;
 
     #[inline]
@@ -62,6 +77,8 @@ impl<'a> WriteLockStrategy<'a> for Disabled {
     }
 }
 
+impl<'a> WriteLockStrategy<'a> for LockDisabled {}
+
 pub struct DisabledGuard<'a>(&'a mut MmapMut);
 
 impl<'a> Deref for DisabledGuard<'a> {
@@ -86,12 +103,11 @@ impl<'a> DerefMut for DisabledGuard<'a> {
 pub struct SingleWriter {
     mmap: MmapMut,
     file: File,
-
     locked: bool,
 }
 
 #[cfg(unix)]
-impl<'a> WriteLockStrategy<'a> for SingleWriter {
+impl<'a> WriteLockStrategySealed<'a> for SingleWriter {
     type Guard = SingleWriterGuard<'a>;
 
     #[inline]
@@ -116,6 +132,7 @@ impl<'a> WriteLockStrategy<'a> for SingleWriter {
         }
 
         // Acquire the lock for the first time.
+        // Note: the file descriptor must remain open to hold the lock.
         match unsafe { libc::flock(self.file.as_raw_fd(), libc::LOCK_EX | libc::LOCK_NB) } {
             0 => {
                 // Hold the lock until this structure is dropped.
@@ -127,6 +144,8 @@ impl<'a> WriteLockStrategy<'a> for SingleWriter {
     }
 }
 
+impl<'a> WriteLockStrategy<'a> for SingleWriter {}
+
 /// A simple guard which does not release the lock upon being dropped.
 #[cfg(unix)]
 pub struct SingleWriterGuard<'a>(&'a mut MmapMut);

src/state.rs

@@ -199,7 +199,7 @@ impl<'a, WL: WriteLockStrategy<'a>> StateContainer<WL> {
 #[cfg(all(test, unix))]
 mod tests {
     use super::*;
-    use crate::locks::SingleWriter;
+    use crate::locks::{LockDisabled, SingleWriter};
     use crate::synchronizer::SynchronizerError;
 
     #[test]
@@ -225,4 +225,14 @@ mod tests {
         drop(state1);
         assert!(state2.state_write(true).is_ok());
     }
+
+    #[test]
+    fn lock_disabled_does_not_hold_lock() {
+        static PATH: &str = "/tmp/lock_disabled_with_single_writer";
+        let mut state1 = StateContainer::<LockDisabled>::new(PATH.as_ref());
+        let mut state2 = StateContainer::<SingleWriter>::new(PATH.as_ref());
+
+        assert!(state1.state_write(true).is_ok());
+        assert!(state2.state_write(true).is_ok());
+    }
 }

src/synchronizer.rs

@@ -18,7 +18,7 @@ use wyhash::WyHash;
 use crate::data::DataContainer;
 use crate::guard::{ReadGuard, ReadResult};
 use crate::instance::InstanceVersion;
-use crate::locks::{Disabled, WriteLockStrategy};
+use crate::locks::{LockDisabled, WriteLockStrategy};
 use crate::state::StateContainer;
 use crate::synchronizer::SynchronizerError::*;
 
@@ -28,14 +28,14 @@ use crate::synchronizer::SynchronizerError::*;
 ///
 /// Template parameters:
 ///   - `H` - hasher used for checksum calculation
+///   - `WL` - optional write locking to prevent multiple writers. (default [`LockDisabled`])
 ///   - `N` - serializer scratch space size
 ///   - `SD` - sleep duration in nanoseconds used by writer during lock acquisition (default 1s)
-///   - `WL` - optional write locking to prevent multiple writers. (default [`Disabled`])
 pub struct Synchronizer<
     H: Hasher + Default = WyHash,
+    WL = LockDisabled,
     const N: usize = 1024,
     const SD: u64 = 1_000_000_000,
-    WL = Disabled,
 > {
     /// Container storing state mmap
     state_container: StateContainer<WL>,
@@ -85,7 +85,7 @@ impl Synchronizer {
     }
 }
 
-impl<'a, H, const N: usize, const SD: u64, WL> Synchronizer<H, N, SD, WL>
+impl<'a, H, WL, const N: usize, const SD: u64> Synchronizer<H, WL, N, SD>
 where
     H: Hasher + Default,
     WL: WriteLockStrategy<'a>,