From 90eb3482329448fbba49501b2f31adb68dc8ddb3 Mon Sep 17 00:00:00 2001 From: Myk Melez Date: Mon, 4 Mar 2019 16:57:40 -0800 Subject: [PATCH 1/2] manage immutable Arc rather than interiorly mutable Arc> Manager currently manages Arc> instances, which enables interior mutability of the Rkv instances. That can result in surprising behavior when fetching a mutated Rkv from the Manager. It also requires consumers to acquire a (read, at least) lock to use a managed Rkv, even though all of its methods take an immutable reference to self (&self). This branch removes the RwLock from managed Rkv instances, such that Manager returns Arc, which is immutable, cannot result in surprising behavior when fetching an Rkv from the Manager, and doesn't need to be locked. --- src/manager.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/manager.rs b/src/manager.rs index c51564c8..01ad5188 100644 --- a/src/manager.rs +++ b/src/manager.rs @@ -58,7 +58,7 @@ where } pub struct Manager { - environments: BTreeMap>>, + environments: BTreeMap>, } impl Manager { @@ -73,7 +73,7 @@ impl Manager { } /// Return the open env at `path`, returning `None` if it has not already been opened. - pub fn get<'p, P>(&self, path: P) -> Result>>, ::std::io::Error> + pub fn get<'p, P>(&self, path: P) -> Result>, ::std::io::Error> where P: Into<&'p Path>, { @@ -82,7 +82,7 @@ impl Manager { } /// Return the open env at `path`, or create it by calling `f`. - pub fn get_or_create<'p, F, P>(&mut self, path: P, f: F) -> Result>, StoreError> + pub fn get_or_create<'p, F, P>(&mut self, path: P, f: F) -> Result, StoreError> where F: FnOnce(&Path) -> Result, P: Into<&'p Path>, @@ -91,7 +91,7 @@ impl Manager { Ok(match self.environments.entry(canonical) { Entry::Occupied(e) => e.get().clone(), Entry::Vacant(e) => { - let k = Arc::new(RwLock::new(f(e.key().as_path())?)); + let k = Arc::new(f(e.key().as_path())?); e.insert(k).clone() }, }) @@ -104,7 +104,7 @@ impl Manager { path: P, capacity: c_uint, f: F, - ) -> Result>, StoreError> + ) -> Result, StoreError> where F: FnOnce(&Path, c_uint) -> Result, P: Into<&'p Path>, @@ -113,7 +113,7 @@ impl Manager { Ok(match self.environments.entry(canonical) { Entry::Occupied(e) => e.get().clone(), Entry::Vacant(e) => { - let k = Arc::new(RwLock::new(f(e.key().as_path(), capacity)?)); + let k = Arc::new(f(e.key().as_path(), capacity)?); e.insert(k).clone() }, }) From 6a77d9b6324f71b09e156c741d483c34b9c7b146 Mon Sep 17 00:00:00 2001 From: Myk Melez Date: Tue, 12 Mar 2019 11:01:59 -0700 Subject: [PATCH 2/2] fix examples and doc tests --- examples/iterator.rs | 3 +-- examples/simple-store.rs | 3 +-- src/lib.rs | 3 +-- 3 files changed, 3 insertions(+), 6 deletions(-) diff --git a/examples/iterator.rs b/examples/iterator.rs index 5a29b31e..c149c580 100644 --- a/examples/iterator.rs +++ b/examples/iterator.rs @@ -25,8 +25,7 @@ fn main() { fs::create_dir_all(root.path()).unwrap(); let p = root.path(); - let created_arc = Manager::singleton().write().unwrap().get_or_create(p, Rkv::new).unwrap(); - let k = created_arc.read().unwrap(); + let k = Manager::singleton().write().unwrap().get_or_create(p, Rkv::new).unwrap(); let store = k.open_single("store", StoreOptions::create()).unwrap(); populate_store(&k, store).unwrap(); diff --git a/examples/simple-store.rs b/examples/simple-store.rs index 292cfa30..091b5259 100644 --- a/examples/simple-store.rs +++ b/examples/simple-store.rs @@ -53,8 +53,7 @@ fn main() { let p = root.path(); // The manager enforces that each process opens the same lmdb environment at most once - let created_arc = Manager::singleton().write().unwrap().get_or_create(p, Rkv::new).unwrap(); - let k = created_arc.read().unwrap(); + let k = Manager::singleton().write().unwrap().get_or_create(p, Rkv::new).unwrap(); // Creates a store called "store" let store = k.open_single("store", StoreOptions::create()).unwrap(); diff --git a/src/lib.rs b/src/lib.rs index 7a675442..482815f7 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -60,8 +60,7 @@ //! // at most once by caching a handle to each environment that it opens. //! // Use it to retrieve the handle to an opened environment—or create one //! // if it hasn't already been opened: -//! let created_arc = Manager::singleton().write().unwrap().get_or_create(path, Rkv::new).unwrap(); -//! let env = created_arc.read().unwrap(); +//! let env = Manager::singleton().write().unwrap().get_or_create(path, Rkv::new).unwrap(); //! //! // Then you can use the environment handle to get a handle to a datastore: //! let store: SingleStore = env.open_single("mydb", StoreOptions::create()).unwrap();