[eclipse-iceoryx#361] Add timeout to static storage open

elfenpiff · Aug 30, 2024 · b05edc4 · b05edc4
1 parent c7689b2
commit b05edc4
Show file tree

Hide file tree

Showing 5 changed files with 109 additions and 48 deletions.
diff --git a/iceoryx2-cal/src/static_storage/file.rs b/iceoryx2-cal/src/static_storage/file.rs
@@ -47,6 +47,7 @@ pub use crate::named_concept::*;
 pub use crate::static_storage::*;
 
 use iceoryx2_bb_log::{fail, trace, warn};
+use iceoryx2_bb_posix::adaptive_wait::AdaptiveWaitBuilder;
 use iceoryx2_bb_posix::{
     directory::*, file::*, file_descriptor::FileDescriptorManagement, file_type::FileType,
 };
@@ -414,7 +415,7 @@ impl crate::static_storage::StaticStorageBuilder<Storage> for Builder {
         })
     }
 
-    fn open(self) -> Result<Storage, StaticStorageOpenError> {
+    fn open(self, timeout: Duration) -> Result<Storage, StaticStorageOpenError> {
         let msg = "Unable to open static storage";
         let origin = "static_storage::File::Builder::open()";
 
@@ -423,21 +424,39 @@ impl crate::static_storage::StaticStorageBuilder<Storage> for Builder {
             with StaticStorageOpenError::DoesNotExist,
             "{} due to a failure while opening the file.", msg);
 
-        let metadata = fail!(from origin,
+        let mut wait_for_read_access = fail!(from self,
+            when AdaptiveWaitBuilder::new().create(),
+            with StaticStorageOpenError::InternalError,
+            "{} since the AdaptiveWait could not be initialized.", msg);
+
+        let mut elapsed_time = Duration::ZERO;
+
+        loop {
+            let metadata = fail!(from origin,
             when file.metadata(), with StaticStorageOpenError::Read,
             "{} due to a failure while reading the files metadata.", msg);
 
-        if metadata.permission() != FINAL_PERMISSIONS {
-            fail!(from origin, with StaticStorageOpenError::IsLocked,
-                "{} since the static storage is still being created (in locked state), try later.", msg);
-        }
+            if metadata.permission() != FINAL_PERMISSIONS {
+                if elapsed_time >= timeout {
+                    fail!(from origin,
+                        with StaticStorageOpenError::InitializationNotYetFinalized,
+                        "{} since the static storage is still being created (in locked  state), try later.",
+                        msg);
+                }
 
-        Ok(Storage {
-            name: self.storage_name,
-            config: self.config,
-            has_ownership: self.has_ownership,
-            file,
-            len: metadata.size(),
-        })
+                elapsed_time = fail!(from self,
+                    when wait_for_read_access.wait(),
+                    with StaticStorageOpenError::InternalError,
+                    "{} since the adaptive wait call failed.", msg);
+            } else {
+                return Ok(Storage {
+                    name: self.storage_name,
+                    config: self.config,
+                    has_ownership: self.has_ownership,
+                    file,
+                    len: metadata.size(),
+                });
+            }
+        }
     }
 }
diff --git a/iceoryx2-cal/src/static_storage/mod.rs b/iceoryx2-cal/src/static_storage/mod.rs
@@ -16,7 +16,7 @@
 pub mod file;
 pub mod process_local;
 
-use std::fmt::Debug;
+use std::{fmt::Debug, time::Duration};
 
 use iceoryx2_bb_log::fail;
 use iceoryx2_bb_system_types::file_name::*;
@@ -38,7 +38,7 @@ pub enum StaticStorageCreateError {
 pub enum StaticStorageOpenError {
     DoesNotExist,
     Read,
-    IsLocked,
+    InitializationNotYetFinalized,
     InternalError,
 }
 
@@ -87,7 +87,10 @@ pub trait StaticStorageBuilder<T: StaticStorage>: Sized + NamedConceptBuilder<T>
 
     /// Opens an already existing [`StaticStorage`]. If the creation of the [`StaticStorage`] is
     /// not finalized it shall return an error.
-    fn open(self) -> Result<T, StaticStorageOpenError>;
+    /// The provided defines how long the [`StaticStorageBuilder`]
+    /// shall wait for [`StaticStorageBuilder::create_locked()`]
+    /// to finalize the initialization and unlock the storage.
+    fn open(self, timeout: Duration) -> Result<T, StaticStorageOpenError>;
 }
 
 /// A locked (uninitialized) static storage which is present but without content

diff --git a/iceoryx2-cal/src/static_storage/process_local.rs b/iceoryx2-cal/src/static_storage/process_local.rs
@@ -39,6 +39,7 @@
 pub use crate::named_concept::*;
 pub use crate::static_storage::*;
 use iceoryx2_bb_log::{fail, fatal_panic};
+use iceoryx2_bb_posix::adaptive_wait::AdaptiveWaitBuilder;
 use iceoryx2_bb_posix::mutex::*;
 use once_cell::sync::Lazy;
 use std::collections::HashMap;
@@ -301,30 +302,46 @@ impl StaticStorageBuilder<Storage> for Builder {
         self
     }
 
-    fn open(self) -> Result<Storage, StaticStorageOpenError> {
+    fn open(self, timeout: Duration) -> Result<Storage, StaticStorageOpenError> {
         let msg = "Failed to open static storage";
+        let mut wait_for_read_access = fail!(from self,
+            when AdaptiveWaitBuilder::new().create(),
+            with StaticStorageOpenError::InternalError,
+            "{} since the AdaptiveWait could not be initialized.", msg);
 
-        let mut guard = fail!(from self, when PROCESS_LOCAL_STORAGE.lock(),
+        let mut elapsed_time = Duration::ZERO;
+
+        loop {
+            let mut guard = fail!(from self, when PROCESS_LOCAL_STORAGE.lock(),
                 with StaticStorageOpenError::InternalError,
                 "{} due to a failure while acquiring the lock.", msg);
-        let entry = guard.get_mut(&self.config.path_for(&self.name));
-        if entry.is_none() {
-            fail!(from self, with StaticStorageOpenError::DoesNotExist,
+
+            let entry = guard.get_mut(&self.config.path_for(&self.name));
+            if entry.is_none() {
+                fail!(from self, with StaticStorageOpenError::DoesNotExist,
                                 "{} since the storage does not exist.", msg);
-        }
+            }
 
-        let entry = entry.unwrap();
-        if entry.content.is_locked {
-            fail!(from self, with StaticStorageOpenError::IsLocked,
-                    "{} since the static storage is still being created (in locked state), try later.", msg);
+            let entry = entry.unwrap();
+            if entry.content.is_locked {
+                if elapsed_time >= timeout {
+                    fail!(from self, with StaticStorageOpenError::InitializationNotYetFinalized,
+                        "{} since the static storage is still being created (in locked state), try later.", msg);
+                }
+
+                elapsed_time = fail!(from self,
+                    when wait_for_read_access.wait(),
+                    with StaticStorageOpenError::InternalError,
+                    "{} since the adaptive wait call failed.", msg);
+            } else {
+                return Ok(Storage {
+                    name: self.name,
+                    has_ownership: self.has_ownership,
+                    config: self.config,
+                    content: entry.content.clone(),
+                });
+            }
         }
-
-        Ok(Storage {
-            name: self.name,
-            has_ownership: self.has_ownership,
-            config: self.config,
-            content: entry.content.clone(),
-        })
     }
 
     fn create_locked(self) -> Result<<Storage as StaticStorage>::Locked, StaticStorageCreateError> {

diff --git a/iceoryx2-cal/tests/static_storage_file_tests.rs b/iceoryx2-cal/tests/static_storage_file_tests.rs
@@ -19,6 +19,7 @@ use iceoryx2_bb_system_types::file_name::FileName;
 use iceoryx2_bb_system_types::file_path::FilePath;
 use iceoryx2_bb_testing::assert_that;
 use iceoryx2_cal::static_storage::file::*;
+use std::time::Duration;
 
 fn generate_name() -> FileName {
     let mut file = FileName::new(b"communication_channel_tests_").unwrap();
@@ -48,7 +49,10 @@ fn static_storage_file_custom_path_and_suffix_works() {
         .unwrap();
     assert_that!(*storage_guard.name(), eq storage_name);
 
-    let storage_reader = Builder::new(&storage_name).config(&config).open().unwrap();
+    let storage_reader = Builder::new(&storage_name)
+        .config(&config)
+        .open(Duration::ZERO)
+        .unwrap();
     assert_that!(*storage_reader.name(), eq storage_name);
 
     let content_len = content.len() as u64;
@@ -78,7 +82,10 @@ fn static_storage_file_path_is_created_when_it_does_not_exist() {
         .create(content.as_bytes());
     assert_that!(storage_guard, is_ok);
 
-    let storage_reader = Builder::new(&storage_name).config(&config).open().unwrap();
+    let storage_reader = Builder::new(&storage_name)
+        .config(&config)
+        .open(Duration::ZERO)
+        .unwrap();
     assert_that!(*storage_reader.name(), eq storage_name);
 
     let content_len = content.len() as u64;

diff --git a/iceoryx2-cal/tests/static_storage_trait_tests.rs b/iceoryx2-cal/tests/static_storage_trait_tests.rs
@@ -23,6 +23,7 @@ mod static_storage {
     use std::sync::atomic::{AtomicU64, Ordering};
     use std::sync::Barrier;
     use std::sync::Mutex;
+    use std::time::Duration;
 
     /// The list all storage tests requires that all other tests are not interfering and therefore
     /// we cannot let them run concurrently.
@@ -54,7 +55,9 @@ mod static_storage {
 
         assert_that!(*storage_guard.name(), eq storage_name);
 
-        let storage_reader = Sut::Builder::new(&storage_name).open().unwrap();
+        let storage_reader = Sut::Builder::new(&storage_name)
+            .open(Duration::ZERO)
+            .unwrap();
 
         assert_that!(*storage_reader.name(), eq storage_name);
         let content_len = content.len() as u64;
@@ -72,7 +75,7 @@ mod static_storage {
         let storage_name = generate_name();
 
         let _test_guard = TEST_MUTEX.lock();
-        let storage_reader = Sut::Builder::new(&storage_name).open();
+        let storage_reader = Sut::Builder::new(&storage_name).open(Duration::ZERO);
 
         assert_that!(storage_reader, is_err);
         assert_that!(
@@ -91,7 +94,7 @@ mod static_storage {
             Sut::Builder::new(&storage_name).create(unsafe { content.as_mut_vec() }.as_slice());
 
         drop(storage_guard);
-        let result = Sut::Builder::new(&storage_name).open();
+        let result = Sut::Builder::new(&storage_name).open(Duration::ZERO);
         assert_that!(result, is_err);
         assert_that!(result.err().unwrap(), eq StaticStorageOpenError::DoesNotExist);
     }
@@ -123,7 +126,9 @@ mod static_storage {
         let storage_guard =
             Sut::Builder::new(&storage_name).create(unsafe { content.as_mut_vec() }.as_slice());
 
-        let storage_reader = Sut::Builder::new(&storage_name).open().unwrap();
+        let storage_reader = Sut::Builder::new(&storage_name)
+            .open(Duration::ZERO)
+            .unwrap();
         drop(storage_guard);
 
         let content_len = content.len() as u64;
@@ -135,7 +140,7 @@ mod static_storage {
             .unwrap();
         assert_that!(read_content, eq content.clone());
 
-        let storage_reader = Sut::Builder::new(&storage_name).open();
+        let storage_reader = Sut::Builder::new(&storage_name).open(Duration::ZERO);
         assert_that!(storage_reader, is_err);
         assert_that!(
             storage_reader.err().unwrap(), eq
@@ -156,11 +161,13 @@ mod static_storage {
         let storage_guard =
             Sut::Builder::new(&storage_name).create(unsafe { content.as_mut_vec() }.as_slice());
 
-        let storage_reader = Sut::Builder::new(&storage_name).open().unwrap();
+        let storage_reader = Sut::Builder::new(&storage_name)
+            .open(Duration::ZERO)
+            .unwrap();
         drop(storage_guard);
         drop(storage_reader);
 
-        let storage_reader = Sut::Builder::new(&storage_name).open();
+        let storage_reader = Sut::Builder::new(&storage_name).open(Duration::ZERO);
         assert_that!(storage_reader, is_err);
         assert_that!(
             storage_reader.err().unwrap(), eq
@@ -177,8 +184,12 @@ mod static_storage {
         let storage_guard =
             Sut::Builder::new(&storage_name).create(unsafe { content.as_mut_vec() }.as_slice());
 
-        let storage_reader_alt = Sut::Builder::new(&storage_name).open().unwrap();
-        let storage_reader = Sut::Builder::new(&storage_name).open().unwrap();
+        let storage_reader_alt = Sut::Builder::new(&storage_name)
+            .open(Duration::ZERO)
+            .unwrap();
+        let storage_reader = Sut::Builder::new(&storage_name)
+            .open(Duration::ZERO)
+            .unwrap();
         drop(storage_guard);
 
         let content_len = content.len() as u64;
@@ -206,7 +217,9 @@ mod static_storage {
         let _storage_guard =
             Sut::Builder::new(&storage_name).create(unsafe { content.as_mut_vec() }.as_slice());
 
-        let storage_reader = Sut::Builder::new(&storage_name).open().unwrap();
+        let storage_reader = Sut::Builder::new(&storage_name)
+            .open(Duration::ZERO)
+            .unwrap();
 
         let content_len = content.len() as u64;
         assert_that!(storage_reader, len content_len);
@@ -368,18 +381,20 @@ mod static_storage {
         assert_that!(Sut::does_exist(&storage_name), eq Err(NamedConceptDoesExistError::UnderlyingResourcesBeingSetUp));
         assert_that!(*storage_guard.as_ref().unwrap().name(), eq storage_name);
 
-        let storage_reader = Sut::Builder::new(&storage_name).open();
+        let storage_reader = Sut::Builder::new(&storage_name).open(Duration::ZERO);
         assert_that!(storage_reader, is_err);
         assert_that!(
             storage_reader.err().unwrap(), eq
-            StaticStorageOpenError::IsLocked
+            StaticStorageOpenError::InitializationNotYetFinalized
         );
 
         let storage_guard = storage_guard.unwrap().unlock(content.as_bytes());
         assert_that!(storage_guard, is_ok);
         assert_that!(Sut::does_exist(&storage_name), eq Ok(true));
 
-        let storage_reader = Sut::Builder::new(&storage_name).open().unwrap();
+        let storage_reader = Sut::Builder::new(&storage_name)
+            .open(Duration::ZERO)
+            .unwrap();
 
         assert_that!(*storage_reader.name(), eq storage_name);
         let content_len = content.len() as u64;