From be9caff6b158a762418815ff2cbaf521b6727029 Mon Sep 17 00:00:00 2001
From: Jan Tuomi <jan@jantuomi.fi>
Date: Sat, 18 Jan 2025 09:48:55 +0200
Subject: Make lock manager stricter

---
 log_db/src/common.rs |  1 -
 log_db/src/engine.rs | 15 ++++++++-------
 log_db/src/lock.rs   | 25 +++++++++++++++++++++----
 3 files changed, 29 insertions(+), 12 deletions(-)

(limited to 'log_db')

diff --git a/log_db/src/common.rs b/log_db/src/common.rs
index cbf6ce4..9a41074 100644
--- a/log_db/src/common.rs
+++ b/log_db/src/common.rs
@@ -25,7 +25,6 @@ pub const INITIALIZED_FILENAME: &str = "initialized";
 
 pub const METADATA_FILE_HEADER_SIZE: usize = 24;
 pub const METADATA_ROW_LENGTH: usize = 16;
-pub const DEFAULT_READ_BUF_SIZE: usize = 1024 * 1024; // 1 MB
 pub const LOCK_WAIT_MAX_MS: u64 = 1000;
 
 // Serialized value tags
diff --git a/log_db/src/engine.rs b/log_db/src/engine.rs
index 8ad4590..c682704 100644
--- a/log_db/src/engine.rs
+++ b/log_db/src/engine.rs
@@ -61,8 +61,6 @@ impl<R: Recordable> Engine<R> {
             fs::File::create(data_dir_path.join(INITIALIZED_FILENAME))?;
         }
 
-        lock_manager.unlock()?;
-
         // Calculate the index of the primary value in a record
         let primary_key_index = config
             .fields
@@ -125,6 +123,7 @@ impl<R: Recordable> Engine<R> {
 
         info!("Database ready.");
 
+        engine.lock_manager.unlock()?;
         Ok(engine)
     }
 
@@ -506,7 +505,7 @@ impl<R: Recordable> Engine<R> {
         let correct = is_file_same_as_path(&self.active_metadata_file, &active_metadata_path)?;
         if !correct {
             debug!("Metadata file has been rotated. Reopening...");
-            let mut metadata_file = APPEND_MODE.open(&active_metadata_path)?;
+            let metadata_file = APPEND_MODE.open(&active_metadata_path)?;
 
             let metadata_header = read_metadata_header(&mut self.active_metadata_file)?;
 
@@ -715,20 +714,22 @@ impl<R: Recordable> Engine<R> {
             .map(|(_, t)| t)
     }
 
+    #[inline]
     pub fn with_exclusive_lock<T>(
         &mut self,
         f: impl FnOnce(&mut Self) -> DBResult<T>,
     ) -> DBResult<T> {
         self.lock_manager.lock_exclusive()?;
-        let result = f(self)?;
+        let result = f(self);
         self.lock_manager.unlock()?;
-        Ok(result)
+        result
     }
 
+    #[inline]
     pub fn with_shared_lock<T>(&mut self, f: impl FnOnce(&mut Self) -> DBResult<T>) -> DBResult<T> {
         self.lock_manager.lock_shared()?;
-        let result = f(self)?;
+        let result = f(self);
         self.lock_manager.unlock()?;
-        Ok(result)
+        result
     }
 }
diff --git a/log_db/src/lock.rs b/log_db/src/lock.rs
index 01961f9..adb2809 100644
--- a/log_db/src/lock.rs
+++ b/log_db/src/lock.rs
@@ -12,7 +12,6 @@ enum LockState {
     NotLocked,
     Shared,
     Exclusive,
-    ManualExclusive,
 }
 
 impl LockManager {
@@ -47,7 +46,13 @@ impl LockManager {
 
     pub fn lock_shared(&mut self) -> DBResult<()> {
         if self.state == LockState::Shared {
-            return Ok(());
+            return Err(DBError::LockRequestError(
+                "Already holding a shared lock".to_owned(),
+            ));
+        } else if self.state == LockState::Exclusive {
+            return Err(DBError::LockRequestError(
+                "Cannot acquire shared lock while holding an exclusive lock".to_owned(),
+            ));
         }
 
         let mut timeout = 5;
@@ -74,8 +79,14 @@ impl LockManager {
     }
 
     pub fn lock_exclusive(&mut self) -> DBResult<()> {
-        if self.state == LockState::Exclusive || self.state == LockState::ManualExclusive {
-            return Ok(());
+        if self.state == LockState::Exclusive {
+            return Err(DBError::LockRequestError(
+                "Already holding an exclusive lock".to_owned(),
+            ));
+        } else if self.state == LockState::Shared {
+            return Err(DBError::LockRequestError(
+                "Cannot acquire exclusive lock while holding a shared lock".to_owned(),
+            ));
         }
 
         // Create a lock on the exclusive lock request file to signal to readers that they should wait
@@ -93,6 +104,12 @@ impl LockManager {
     }
 
     pub fn unlock(&mut self) -> DBResult<()> {
+        if self.state == LockState::NotLocked {
+            return Err(DBError::LockRequestError(
+                "Not holding any locks".to_owned(),
+            ));
+        }
+
         self.lock_file.unlock()?;
         self.state = LockState::NotLocked;
         Ok(())
-- 
cgit v1.3