From 7834741d905819aa7e0b5e00d7534634468b7cf4 Mon Sep 17 00:00:00 2001 From: yyc12345 Date: Sat, 1 Aug 2026 17:25:38 +0800 Subject: [PATCH] feat: improve object pool - improve object pool - add docstring for test coverage --- omrf/Cargo.lock | 104 ++++++++++++++++++++--- omrf/Cargo.toml | 2 +- omrf/src/lib.rs | 14 --- omrf/src/object_pool.rs | 174 ++++++++++++++++++++++++-------------- omrf/tests/cstr_ffi.rs | 49 +++++++++-- omrf/tests/last_error.rs | 8 ++ omrf/tests/object_pool.rs | 102 ++++++++++++++++++++++ 7 files changed, 356 insertions(+), 97 deletions(-) create mode 100644 omrf/tests/object_pool.rs diff --git a/omrf/Cargo.lock b/omrf/Cargo.lock index b38c0e7..4ebf116 100644 --- a/omrf/Cargo.lock +++ b/omrf/Cargo.lock @@ -2,6 +2,78 @@ # It is not intended for manual editing. version = 4 +[[package]] +name = "bitflags" +version = "2.13.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b588b76d00fde79687d7646a9b5bdf3cc0f655e0bbd080335a95d7e96f3587da" + +[[package]] +name = "cfg-if" +version = "1.0.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9330f8b2ff13f34540b44e946ef35111825727b38d33286ef986142615121801" + +[[package]] +name = "crossbeam-utils" +version = "0.8.22" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "61803da095bee82a81bb1a452ecc25d3b2f1416d1897eb86430c6159ef717c17" + +[[package]] +name = "dashmap" +version = "6.2.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e6361d5c062261c78a176addb82d4c821ae42bed6089de0e12603cd25de2059c" +dependencies = [ + "cfg-if", + "crossbeam-utils", + "hashbrown", + "lock_api", + "once_cell", + "parking_lot_core", +] + +[[package]] +name = "hashbrown" +version = "0.14.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e5274423e17b7c9fc20b6e7e208532f9b19825d82dfd615708b70edd83df41f1" + +[[package]] +name = "libc" +version = "0.2.189" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3eaf3ede3fee6db1a4c2ee091bf8a8b4dccdc6d17f656fb07896ee72867612f2" + +[[package]] +name = "lock_api" +version = "0.4.14" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "224399e74b87b5f3557511d98dff8b14089b3dadafcab6bb93eab67d3aace965" +dependencies = [ + "scopeguard", +] + +[[package]] +name = "once_cell" +version = "1.21.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9f7c3e4beb33f85d45ae3e3a1792185706c8e16d043238c593331cc7cd313b50" + +[[package]] +name = "parking_lot_core" +version = "0.9.12" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2621685985a2ebf1c516881c026032ac7deafcda1a2c9b7850dc81e3dfcb64c1" +dependencies = [ + "cfg-if", + "libc", + "redox_syscall", + "smallvec", + "windows-link", +] + [[package]] name = "proc-macro2" version = "1.0.107" @@ -20,22 +92,34 @@ dependencies = [ "proc-macro2", ] +[[package]] +name = "redox_syscall" +version = "0.5.18" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ed2bf2547551a7053d6fdfafda3f938979645c44812fbfcda098faae3f1a362d" +dependencies = [ + "bitflags", +] + [[package]] name = "sarasacw-omrf" version = "0.1.0" dependencies = [ - "slotmap", + "dashmap", "thiserror", ] [[package]] -name = "slotmap" -version = "1.1.1" +name = "scopeguard" +version = "1.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "bdd58c3c93c3d278ca835519292445cb4b0d4dc59ccfdf7ceadaab3f8aeb4038" -dependencies = [ - "version_check", -] +checksum = "94143f37725109f92c262ed2cf5e59bce7498c01bcc1502d7b9afe439a4e9f49" + +[[package]] +name = "smallvec" +version = "1.15.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8ed6a63f02c8539c91a8685a86f4099661ba3da017932f6ebbea6de3f0fa7c90" [[package]] name = "syn" @@ -75,7 +159,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e6e4313cd5fcd3dad5cafa179702e2b244f760991f45397d14d4ebf38247da75" [[package]] -name = "version_check" -version = "0.9.5" +name = "windows-link" +version = "0.2.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0b928f33d975fc6ad9f86c8f283853ad26bdd5b10b7f1542aa2fa15e2289105a" +checksum = "f0805222e57f7521d6a62e36fa9163bc891acd422f971defe97d64e70d0a4fe5" diff --git a/omrf/Cargo.toml b/omrf/Cargo.toml index fd6a000..1266708 100644 --- a/omrf/Cargo.toml +++ b/omrf/Cargo.toml @@ -5,4 +5,4 @@ edition = "2024" [dependencies] thiserror = "2.0.12" -slotmap = "1.1.1" +dashmap = "6.2.1" diff --git a/omrf/src/lib.rs b/omrf/src/lib.rs index f777957..f1f29a6 100644 --- a/omrf/src/lib.rs +++ b/omrf/src/lib.rs @@ -9,17 +9,3 @@ macro_rules! resolve_enum { <$t>::try_from($v).map_err(|_| Error::EnumOutOfRange) }; } - -#[macro_export] -macro_rules! pull_reader { - ($pool:expr) => { - $pool.read().map_err(|_| Error::PoisonRwLock) - }; -} - -#[macro_export] -macro_rules! pull_writer { - ($pool:expr) => { - $pool.write().map_err(|_| Error::PoisonRwLock) - }; -} diff --git a/omrf/src/object_pool.rs b/omrf/src/object_pool.rs index 380cc50..a2e2aec 100644 --- a/omrf/src/object_pool.rs +++ b/omrf/src/object_pool.rs @@ -1,95 +1,139 @@ -//! When exporting resources for C interface, resource management and ownership are important things. -//! In this dynamic library, we hold all resources' ownership in Rust world, -//! and only expose a token for C code manipulation. -//! -//! We need to create a container for holding all resources and providing corresponding operations. -//! So we introduce [ObjectPool] in this module for this purpose. -use slotmap::{DefaultKey, Key, KeyData, SlotMap}; +//! A thread-safe object pool that hands out opaque integer tokens for objects it owns. +//! +//! The pool is backed by a sharded concurrent map ([`dashmap::DashMap`]); all operations take +//! `&self`, so no external lock guard is required -- different tokens can be accessed concurrently +//! from multiple threads (up to per-shard granularity). +//! +//! Tokens are generated by a monotonic [`AtomicU64`] counter starting at 1; `0` is reserved as the +//! invalid token (see [`INVALID_TOKEN`]). Because the counter never reuses a value, a token is +//! unique for the lifetime of the process. + +use dashmap::{DashMap, mapref::one::{Ref, RefMut}}; +use std::sync::atomic::{AtomicU64, Ordering}; use thiserror::Error as TeError; -/// Error occurs when operating with [ObjectPool]. +/// Error occurs when operating with [`ObjectPool`]. #[derive(Debug, TeError)] pub enum Error { #[error("given token is not presented in object pool")] NoSuchToken, } -/// The token for fetching object in [ObjectPool]. +/// The token for fetching an object in [`ObjectPool`]. pub type Token = u64; -/// Get the invalid token. -/// -/// Invalid token is always invalid for fetching object in pool, -/// And can be useful in FFI scenario. -pub fn invalid_token() -> Token { - DefaultKey::null().data().as_ffi() +/// The canonical invalid token. +/// +/// Always invalid for fetching an object in the pool, and useful as a sentinel in FFI scenarios. +/// It is `0`; the internal counter starts at `1` and never produces `0`. +pub const INVALID_TOKEN: Token = 0; + +/// A thread-safe pool that manages objects keyed by unique [`Token`]s. +/// +/// All methods take `&self`; concurrent access to different tokens proceeds in parallel (per +/// shard). See the [`get`](Self::get) / [`get_mut`](Self::get_mut) documentation for the deadlock +/// rule on holding multiple guards at once, and the closure-based +/// [`get_with`](Self::get_with) / [`get_mut_with`](Self::get_mut_with) for a deadlock-safe +/// alternative. +pub struct ObjectPool { + map: DashMap, + next_id: AtomicU64, } -/// A pool for managing objects with unique tokens. -/// -/// It is highly suggested to use this pool with [std::sync::RwLock] guard. -pub struct ObjectPool { - objs: SlotMap, -} - -impl ObjectPool { - /// Create a new [ObjectPool]. +impl ObjectPool { + /// Create a new [`ObjectPool`]. pub fn new() -> Self { Self { - objs: SlotMap::new(), + map: DashMap::new(), + next_id: AtomicU64::new(1), } } - /// Convert [slotmap] crate's [DefaultKey] to our [Token]. - fn key_to_token(key: &DefaultKey) -> Token { - key.data().as_ffi() + /// Insert an object into the pool and return its token. + /// + /// The ownership of the object is transferred to the pool. + /// + /// # Panics + /// + /// Panics if the internal token counter overflows `u64`. This is practically unreachable (see + /// module-level documentation) and is treated as unrecoverable. + pub fn allocate(&self, value: T) -> Token { + let token = self + .next_id + .fetch_update(Ordering::Relaxed, Ordering::Relaxed, |v| v.checked_add(1)) + .expect("ObjectPool token counter exhausted (u64 overflow)"); + self.map.insert(token, value); + token } - /// Convert our [Token] to [slotmap] crate's [DefaultKey]. - fn token_to_key(token: Token) -> DefaultKey { - DefaultKey::from(KeyData::from_ffi(token)) + /// Remove and drop the object corresponding to the given token. + pub fn free(&self, token: Token) -> Result<(), Error> { + self.map.remove(&token).ok_or(Error::NoSuchToken).map(|_| ()) } - /// Put given object into the pool and return its token. - /// - /// The ownership of given object is transferred to the pool. - pub fn allocate(&mut self, value: T) -> Result { - let key = self.objs.insert(value); - Ok(Self::key_to_token(&key)) - } - - /// Free the object in the pool corresponding to the given token. - pub fn free(&mut self, token: Token) -> Result<(), Error> { - let _ = self.pop(token)?; - Ok(()) - } - - /// Remove the object from the pool corresponding to the given token and return it. - /// + /// Remove the object corresponding to the given token and return it. + /// /// The ownership of the object is transferred to the caller. - pub fn pop(&mut self, token: Token) -> Result { - match self.objs.remove(Self::token_to_key(token)) { - Some(obj) => Ok(obj), - None => Err(Error::NoSuchToken), - } + pub fn pop(&self, token: Token) -> Result { + self.map.remove(&token).map(|(_, v)| v).ok_or(Error::NoSuchToken) } - /// Clear all objects in the pool. - pub fn clear(&mut self) -> () { - self.objs.clear(); + /// Remove all objects from the pool. + pub fn clear(&self) { + self.map.clear(); } - /// Get a reference to the object in the pool corresponding to the given token. - pub fn get(&self, token: Token) -> Result<&T, Error> { - self.objs - .get(Self::token_to_key(token)) - .ok_or(Error::NoSuchToken) + /// Get a shared (read) guard for the object keyed by `token`. + /// + /// The returned guard holds a read lock on the map's shard for its whole lifetime. + /// + /// # Deadlock rule -- READ CAREFULLY + /// + /// **Never hold two guards from the same [`ObjectPool`] at once.** If two guards hash to the + /// same shard, the second acquisition waits for the first to release, deadlocking the thread. + /// For sequential access prefer the closure-based [`get_with`](Self::get_with), which releases + /// the lock between calls and makes holding two guards structurally impossible. + pub fn get(&self, token: Token) -> Result, Error> { + self.map.get(&token).ok_or(Error::NoSuchToken) } - /// Get a mutable reference to the object in the pool corresponding to the given token. - pub fn get_mut(&mut self, token: Token) -> Result<&mut T, Error> { - self.objs - .get_mut(Self::token_to_key(token)) - .ok_or(Error::NoSuchToken) + /// Get an exclusive (write) guard for the object keyed by `token`. + /// + /// The returned guard holds a write lock on the map's shard for its whole lifetime. + /// + /// # Deadlock rule -- READ CAREFULLY + /// + /// **Never hold two guards from the same [`ObjectPool`] at once.** If two guards hash to the + /// same shard, the second acquisition waits for the first to release, deadlocking the thread. + /// For sequential access prefer the closure-based [`get_mut_with`](Self::get_mut_with), which + /// releases the lock between calls and makes holding two guards structurally impossible. + pub fn get_mut(&self, token: Token) -> Result, Error> { + self.map.get_mut(&token).ok_or(Error::NoSuchToken) + } + + /// Apply a closure to the object keyed by `token` (shared/read access). + /// + /// The shard lock is held only while `f` runs and is released when this function returns, so + /// the caller cannot accidentally hold two guards across statements -- this is the + /// deadlock-safe counterpart of [`get`](Self::get). + /// + /// The closure must not re-enter this [`ObjectPool`] (doing so risks the same deadlock + /// described on [`get`](Self::get)). + pub fn get_with(&self, token: Token, f: impl FnOnce(&T) -> R) -> Result { + let guard = self.map.get(&token).ok_or(Error::NoSuchToken)?; + Ok(f(&*guard)) + } + + /// Apply a closure to the object keyed by `token` (exclusive/write access). + /// + /// The shard lock is held only while `f` runs and is released when this function returns, so + /// the caller cannot accidentally hold two guards across statements -- this is the + /// deadlock-safe counterpart of [`get_mut`](Self::get_mut). + /// + /// The closure must not re-enter this [`ObjectPool`] (doing so risks the same deadlock + /// described on [`get_mut`](Self::get_mut)). + pub fn get_mut_with(&self, token: Token, f: impl FnOnce(&mut T) -> R) -> Result { + let mut guard = self.map.get_mut(&token).ok_or(Error::NoSuchToken)?; + Ok(f(&mut *guard)) } } diff --git a/omrf/tests/cstr_ffi.rs b/omrf/tests/cstr_ffi.rs index 0013dab..ec4f2d1 100644 --- a/omrf/tests/cstr_ffi.rs +++ b/omrf/tests/cstr_ffi.rs @@ -10,6 +10,7 @@ fn cptr_of(bytes: &[u8]) -> cstr_ffi::CStrPtr { bytes.as_ptr() as cstr_ffi::CStrPtr } +/// Coverage: single NUL-terminated string push -> parse round trip. #[test] fn push_single_string_round_trip() { cstr_ffi::clear_ffi_strings(); @@ -18,12 +19,16 @@ fn push_single_string_round_trip() { assert_eq!(cstr_ffi::parse_ffi_string(ptr).unwrap(), "hello"); } +/// Coverage: pointers returned by earlier `push_ffi_string` calls stay valid after the internal +/// storage reallocates from many later pushes (move-stability of the cached `CString` buffers). #[test] fn pushed_strings_remain_valid_after_more_pushes() { cstr_ffi::clear_ffi_strings(); let p1 = cstr_ffi::push_ffi_string("first").unwrap(); // Force the internal Vec to reallocate by pushing many entries. - let later: Vec<_> = (0..64).map(|_| cstr_ffi::push_ffi_string("filler").unwrap()).collect(); + let later: Vec<_> = (0..64) + .map(|_| cstr_ffi::push_ffi_string("filler").unwrap()) + .collect(); let p2 = cstr_ffi::push_ffi_string("second").unwrap(); let p3 = cstr_ffi::push_ffi_string("third").unwrap(); assert_eq!(cstr_ffi::parse_ffi_string(p1).unwrap(), "first"); @@ -32,17 +37,20 @@ fn pushed_strings_remain_valid_after_more_pushes() { assert_eq!(cstr_ffi::parse_ffi_string(later[40]).unwrap(), "filler"); } +/// Coverage: `push_ffi_string` rejects a string containing an interior NUL byte. #[test] fn push_ffi_string_rejects_interior_nul() { cstr_ffi::clear_ffi_strings(); assert!(cstr_ffi::push_ffi_string("a\x00b").is_err()); } +/// Coverage: `parse_ffi_string` rejects a null pointer. #[test] fn parse_ffi_string_null_ptr_errors() { assert!(cstr_ffi::parse_ffi_string(std::ptr::null()).is_err()); } +/// Coverage: `parse_ffi_string` rejects a NUL-terminated buffer whose content is not valid UTF-8. #[test] fn parse_ffi_string_invalid_utf8_errors() { let mut buf: Vec = vec![b'h', b'i', 0xFF]; @@ -51,6 +59,7 @@ fn parse_ffi_string_invalid_utf8_errors() { assert!(cstr_ffi::parse_ffi_string(ptr).is_err()); } +/// Coverage: nullptr-terminated string vector push -> parse round trip. #[test] fn push_string_vec_nullptr_terminated_round_trip() { cstr_ffi::clear_ffi_strings(); @@ -59,6 +68,8 @@ fn push_string_vec_nullptr_terminated_round_trip() { assert_eq!(collected, vec!["a", "bb", "ccc"]); } +/// Coverage: an empty input to `push_ffi_string_vec` yields a valid empty list (only the trailing +/// null), which parses back to an empty vector. #[test] fn push_string_vec_empty_is_valid_empty_list() { cstr_ffi::clear_ffi_strings(); @@ -67,17 +78,20 @@ fn push_string_vec_empty_is_valid_empty_list() { assert!(collected.is_empty()); } +/// Coverage: `push_ffi_string_vec` rejects an entry containing an interior NUL byte. #[test] fn push_string_vec_rejects_interior_nul() { cstr_ffi::clear_ffi_strings(); assert!(cstr_ffi::push_ffi_string_vec(&["ok", "ba\x00d"]).is_err()); } +/// Coverage: `parse_ffi_string_vec` (nullptr-terminated) rejects a null pointer. #[test] fn parse_ffi_string_vec_null_ptr_errors() { assert!(cstr_ffi::parse_ffi_string_vec(std::ptr::null()).is_err()); } +/// Coverage: count-terminated string vector push -> parse round trip, with the returned count. #[test] fn push_string_vec_with_len_round_trip() { cstr_ffi::clear_ffi_strings(); @@ -87,6 +101,8 @@ fn push_string_vec_with_len_round_trip() { assert_eq!(collected, vec!["x", "yy", "zzz"]); } +/// Coverage: empty input to `push_ffi_string_vec_with_len` yields count 0 and parses to an empty +/// vector. #[test] fn push_string_vec_with_len_empty() { cstr_ffi::clear_ffi_strings(); @@ -96,20 +112,23 @@ fn push_string_vec_with_len_empty() { assert!(collected.is_empty()); } +/// Coverage: `parse_ffi_string_vec_with_len` with count 0 accepts a null pointer and returns an +/// empty vector. #[test] fn parse_ffi_string_vec_with_len_null_zero_len_is_empty() { - assert!( - cstr_ffi::parse_ffi_string_vec_with_len(std::ptr::null(), 0) - .unwrap() - .is_empty() - ); + assert!(cstr_ffi::parse_ffi_string_vec_with_len(std::ptr::null(), 0) + .unwrap() + .is_empty()); } +/// Coverage: `parse_ffi_string_vec_with_len` with count > 0 rejects a null pointer. #[test] fn parse_ffi_string_vec_with_len_null_nonzero_errors() { assert!(cstr_ffi::parse_ffi_string_vec_with_len(std::ptr::null(), 1).is_err()); } +/// Coverage: independent single-string, count-terminated, and nullptr-terminated slots produced in +/// one call cycle all coexist and read back correctly. #[test] fn multiple_string_vec_slots_coexist() { cstr_ffi::clear_ffi_strings(); @@ -127,6 +146,8 @@ fn multiple_string_vec_slots_coexist() { ); } +/// Coverage: `push_ffi_string_view` re-exposes the source string in place (no copy) -- the pointer +/// aliases the source buffer and the length matches. #[test] fn push_ffi_string_view_points_into_source() { cstr_ffi::clear_ffi_strings(); @@ -138,6 +159,7 @@ fn push_ffi_string_view_points_into_source() { assert_eq!(parsed, owned); } +/// Coverage: a string view may contain interior NUL bytes (length-delimited, not NUL-terminated). #[test] fn string_view_allows_interior_nul() { let src: &str = "ab\x00cd"; @@ -147,16 +169,20 @@ fn string_view_allows_interior_nul() { assert_eq!(parsed.as_bytes(), src.as_bytes()); } +/// Coverage: `parse_ffi_string_view` with a null pointer and zero length is interpreted as an +/// empty string (C++ default `std::string_view`). #[test] fn parse_ffi_string_view_null_zero_is_empty() { assert_eq!(cstr_ffi::parse_ffi_string_view(std::ptr::null(), 0).unwrap(), ""); } +/// Coverage: `parse_ffi_string_view` rejects a null pointer when the length is non-zero. #[test] fn parse_ffi_string_view_null_nonzero_errors() { assert!(cstr_ffi::parse_ffi_string_view(std::ptr::null(), 3).is_err()); } +/// Coverage: `parse_ffi_string_view` rejects a buffer whose content is not valid UTF-8. #[test] fn parse_ffi_string_view_invalid_utf8_errors() { let bad: [u8; 2] = [b'a', 0xFF]; @@ -164,6 +190,8 @@ fn parse_ffi_string_view_invalid_utf8_errors() { assert!(cstr_ffi::parse_ffi_string_view(ptr, 2).is_err()); } +/// Coverage: string-view vector push -> parse round trip; the container is cached while the string +/// data stays caller-owned. #[test] fn push_string_view_vec_with_len_round_trip() { cstr_ffi::clear_ffi_strings(); @@ -176,16 +204,18 @@ fn push_string_view_vec_with_len_round_trip() { assert_eq!(parsed, vec!["alpha", "beta", "gamma"]); } +/// Coverage: empty input to `push_ffi_string_view_vec_with_len` yields count 0; parsing (which +/// ignores the possibly-dangling head) returns an empty vector. #[test] fn push_string_view_vec_with_len_empty() { cstr_ffi::clear_ffi_strings(); let (head, count) = cstr_ffi::push_ffi_string_view_vec_with_len(&[]); assert_eq!(count, 0); - // count is 0, so head is ignored (may be dangling); parsing must yield an empty vector. let collected = cstr_ffi::parse_ffi_string_view_vec_with_len(head, count).unwrap(); assert!(collected.is_empty()); } +/// Coverage: string-view vector entries may individually contain interior NUL bytes. #[test] fn string_view_vec_allows_interior_nul_entries() { cstr_ffi::clear_ffi_strings(); @@ -198,6 +228,8 @@ fn string_view_vec_allows_interior_nul_entries() { assert_eq!(parsed[1].as_bytes(), s2.as_bytes()); } +/// Coverage: `parse_ffi_string_view_vec_with_len` with count 0 accepts a null pointer and returns +/// an empty vector. #[test] fn parse_ffi_string_view_vec_null_zero_len_is_empty() { assert!(cstr_ffi::parse_ffi_string_view_vec_with_len(std::ptr::null(), 0) @@ -205,11 +237,14 @@ fn parse_ffi_string_view_vec_null_zero_len_is_empty() { .is_empty()); } +/// Coverage: `parse_ffi_string_view_vec_with_len` with count > 0 rejects a null pointer. #[test] fn parse_ffi_string_view_vec_null_nonzero_errors() { assert!(cstr_ffi::parse_ffi_string_view_vec_with_len(std::ptr::null(), 1).is_err()); } +/// Coverage: after `clear_ffi_strings`, the cache is reusable -- a fresh push/parse works +/// independently of any state pushed before the clear. #[test] fn clear_makes_cache_reusable() { cstr_ffi::clear_ffi_strings(); diff --git a/omrf/tests/last_error.rs b/omrf/tests/last_error.rs index 5b696e4..896f98d 100644 --- a/omrf/tests/last_error.rs +++ b/omrf/tests/last_error.rs @@ -27,6 +27,7 @@ fn parse_cstr(ptr: CStrPtr) -> String { .into_owned() } +/// Coverage: the cleared/default state -- no outcome recorded, code is `CERROR_OK`, message empty. #[test] fn fresh_state_is_absolute_success() { last_error::clear_last_error(); @@ -35,6 +36,8 @@ fn fresh_state_is_absolute_success() { assert_eq!(parse_cstr(last_error::get_error_message()), ""); } +/// Coverage: `set_last_error` records both the `Into`-mapped code and the `Display`-derived +/// message, and `has_last_message` becomes true. #[test] fn set_records_code_and_message() { last_error::clear_last_error(); @@ -44,6 +47,8 @@ fn set_records_code_and_message() { assert_eq!(parse_cstr(last_error::get_error_message()), "boom error"); } +/// Coverage: `clear_last_error` resets a previously-recorded outcome back to the absolute-success +/// state (no code, empty message). #[test] fn clear_resets_to_absolute_success() { last_error::set_last_error(TestErr::Boom); @@ -54,6 +59,8 @@ fn clear_resets_to_absolute_success() { assert_eq!(parse_cstr(last_error::get_error_message()), ""); } +/// Coverage: interior NUL bytes in an error's `Display` text are replaced so the message C string +/// builds without panicking, and the code is still recorded. #[test] fn interior_nul_in_message_is_sanitized() { last_error::clear_last_error(); @@ -65,6 +72,7 @@ fn interior_nul_in_message_is_sanitized() { assert!(msg.contains("value")); } +/// Coverage: a second `set_last_error` fully overwrites the previously recorded code and message. #[test] fn overwriting_replaces_previous_state() { last_error::clear_last_error(); diff --git a/omrf/tests/object_pool.rs b/omrf/tests/object_pool.rs new file mode 100644 index 0000000..05ccbcd --- /dev/null +++ b/omrf/tests/object_pool.rs @@ -0,0 +1,102 @@ +//! Integration tests for the `object_pool` module. + +use sarasacw_omrf::object_pool::{ObjectPool, INVALID_TOKEN}; + +/// Coverage: the `INVALID_TOKEN` constant is `0`, the value the counter never produces. +#[test] +fn invalid_token_is_zero() { + assert_eq!(INVALID_TOKEN, 0); +} + +/// Coverage: `allocate` returns distinct, non-zero tokens across successive calls. +#[test] +fn allocate_yields_distinct_nonzero_tokens() { + let pool: ObjectPool = ObjectPool::new(); + let t1 = pool.allocate(10); + let t2 = pool.allocate(20); + let t3 = pool.allocate(30); + assert_ne!(t1, 0); + assert_ne!(t2, 0); + assert_ne!(t3, 0); + assert_ne!(t1, t2); + assert_ne!(t2, t3); + assert_ne!(t1, t3); +} + +/// Coverage: the guard-style `get` reads and `get_mut` writes an entry, with changes observable +/// by a subsequent `get`. +#[test] +fn get_and_get_mut_round_trip() { + let pool: ObjectPool = ObjectPool::new(); + let t = pool.allocate(7); + assert_eq!(*pool.get(t).unwrap(), 7); + *pool.get_mut(t).unwrap() = 99; + assert_eq!(*pool.get(t).unwrap(), 99); +} + +/// Coverage: the deadlock-safe closure-based `get_with`/`get_mut_with` read and write an entry. +#[test] +fn get_with_and_get_mut_with_round_trip() { + let pool: ObjectPool = ObjectPool::new(); + let t = pool.allocate(1); + assert_eq!(pool.get_with(t, |v| *v).unwrap(), 1); + pool.get_mut_with(t, |v| *v = 42).unwrap(); + assert_eq!(pool.get_with(t, |v| *v).unwrap(), 42); +} + +/// Coverage: `pop` returns the owned value and invalidates the token for any further access. +#[test] +fn pop_returns_owned_value_and_invalidates_token() { + let pool: ObjectPool = ObjectPool::new(); + let t = pool.allocate("hello".to_string()); + let owned = pool.pop(t).unwrap(); + assert_eq!(owned, "hello"); + assert!(pool.get(t).is_err()); +} + +/// Coverage: `free` removes the entry; subsequent `get`/`get_mut` on the same token fail. +#[test] +fn free_invalidates_token() { + let pool: ObjectPool = ObjectPool::new(); + let t = pool.allocate(5); + pool.free(t).unwrap(); + assert!(pool.get(t).is_err()); + assert!(pool.get_mut(t).is_err()); +} + +/// Coverage: every access/mutation/removal method rejects `INVALID_TOKEN` (and any absent token). +#[test] +fn invalid_token_errors_on_all_methods() { + let pool: ObjectPool = ObjectPool::new(); + let bad = INVALID_TOKEN; + assert!(pool.get(bad).is_err()); + assert!(pool.get_mut(bad).is_err()); + assert!(pool.get_with(bad, |_| ()).is_err()); + assert!(pool.get_mut_with(bad, |_| ()).is_err()); + assert!(pool.free(bad).is_err()); + assert!(pool.pop(bad).is_err()); +} + +/// Coverage: `clear` empties the pool; all previously-issued tokens become invalid. +#[test] +fn clear_removes_all_objects() { + let pool: ObjectPool = ObjectPool::new(); + let t1 = pool.allocate(1); + let t2 = pool.allocate(2); + pool.clear(); + assert!(pool.get(t1).is_err()); + assert!(pool.get(t2).is_err()); +} + +/// Coverage: two distinct tokens can be mutated independently via sequential closure access, which +/// never holds two guards at once (the deadlock-safe pattern). +#[test] +fn two_tokens_modified_independently_via_closure() { + let pool: ObjectPool = ObjectPool::new(); + let t1 = pool.allocate(0); + let t2 = pool.allocate(0); + pool.get_mut_with(t1, |v| *v = 11).unwrap(); + pool.get_mut_with(t2, |v| *v = 22).unwrap(); + assert_eq!(pool.get_with(t1, |v| *v).unwrap(), 11); + assert_eq!(pool.get_with(t2, |v| *v).unwrap(), 22); +}