chore: make it harder to misuse raw pointers (#5447)

crates/storage/libmdbx-rs/benches/transaction.rs

@@ -32,30 +32,31 @@ fn bench_get_rand(c: &mut Criterion) {
 fn bench_get_rand_raw(c: &mut Criterion) {
     let n = 100u32;
     let (_dir, env) = setup_bench_db(n);
-    let _txn = env.begin_ro_txn().unwrap();
-    let db = _txn.open_db(None).unwrap();
+    let txn = env.begin_ro_txn().unwrap();
+    let db = txn.open_db(None).unwrap();
 
     let mut keys: Vec<String> = (0..n).map(get_key).collect();
     keys.shuffle(&mut XorShiftRng::from_seed(Default::default()));
 
     let dbi = db.dbi();
-    let txn = _txn.txn();
 
     let mut key_val: MDBX_val = MDBX_val { iov_len: 0, iov_base: ptr::null_mut() };
     let mut data_val: MDBX_val = MDBX_val { iov_len: 0, iov_base: ptr::null_mut() };
 
     c.bench_function("bench_get_rand_raw", |b| {
         b.iter(|| unsafe {
-            let mut i: size_t = 0;
-            for key in &keys {
-                key_val.iov_len = key.len() as size_t;
-                key_val.iov_base = key.as_bytes().as_ptr() as *mut _;
-
-                mdbx_get(txn, dbi, &key_val, &mut data_val);
-
-                i += key_val.iov_len;
-            }
-            black_box(i);
+            txn.with_raw_tx_ptr(|txn| {
+                let mut i: size_t = 0;
+                for key in &keys {
+                    key_val.iov_len = key.len() as size_t;
+                    key_val.iov_base = key.as_bytes().as_ptr() as *mut _;
+
+                    mdbx_get(txn, dbi, &key_val, &mut data_val);
+
+                    i += key_val.iov_len;
+                }
+                black_box(i);
+            });
         })
     });
 }
@@ -84,33 +85,34 @@ fn bench_put_rand(c: &mut Criterion) {
 
 fn bench_put_rand_raw(c: &mut Criterion) {
     let n = 100u32;
-    let (_dir, _env) = setup_bench_db(0);
+    let (_dir, env) = setup_bench_db(0);
 
     let mut items: Vec<(String, String)> = (0..n).map(|n| (get_key(n), get_data(n))).collect();
     items.shuffle(&mut XorShiftRng::from_seed(Default::default()));
 
-    let dbi = _env.begin_ro_txn().unwrap().open_db(None).unwrap().dbi();
-    let env = _env.env();
+    let dbi = env.begin_ro_txn().unwrap().open_db(None).unwrap().dbi();
 
     let mut key_val: MDBX_val = MDBX_val { iov_len: 0, iov_base: ptr::null_mut() };
     let mut data_val: MDBX_val = MDBX_val { iov_len: 0, iov_base: ptr::null_mut() };
 
     c.bench_function("bench_put_rand_raw", |b| {
         b.iter(|| unsafe {
             let mut txn: *mut MDBX_txn = ptr::null_mut();
-            mdbx_txn_begin_ex(env, ptr::null_mut(), 0, &mut txn, ptr::null_mut());
-
-            let mut i: ::libc::c_int = 0;
-            for (key, data) in items.iter() {
-                key_val.iov_len = key.len() as size_t;
-                key_val.iov_base = key.as_bytes().as_ptr() as *mut _;
-                data_val.iov_len = data.len() as size_t;
-                data_val.iov_base = data.as_bytes().as_ptr() as *mut _;
-
-                i += mdbx_put(txn, dbi, &key_val, &mut data_val, 0);
-            }
-            assert_eq!(0, i);
-            mdbx_txn_abort(txn);
+            env.with_raw_env_ptr(|env| {
+                mdbx_txn_begin_ex(env, ptr::null_mut(), 0, &mut txn, ptr::null_mut());
+
+                let mut i: ::libc::c_int = 0;
+                for (key, data) in items.iter() {
+                    key_val.iov_len = key.len() as size_t;
+                    key_val.iov_base = key.as_bytes().as_ptr() as *mut _;
+                    data_val.iov_len = data.len() as size_t;
+                    data_val.iov_base = data.as_bytes().as_ptr() as *mut _;
+
+                    i += mdbx_put(txn, dbi, &key_val, &mut data_val, 0);
+                }
+                assert_eq!(0, i);
+                mdbx_txn_abort(txn);
+            });
         })
     });
 }

crates/storage/libmdbx-rs/src/environment.rs

@@ -98,15 +98,29 @@ impl Environment {
     }
 
     /// Returns true if the environment was opened as WRITEMAP.
+    #[inline]
     pub fn is_write_map(&self) -> bool {
         self.inner.env_kind.is_write_map()
     }
 
     /// Returns the kind of the environment.
+    #[inline]
     pub fn env_kind(&self) -> EnvironmentKind {
         self.inner.env_kind
     }
 
+    /// Returns true if the environment was opened in [Mode::ReadWrite] mode.
+    #[inline]
+    pub fn is_read_write(&self) -> bool {
+        self.inner.txn_manager.is_some()
+    }
+
+    /// Returns true if the environment was opened in [Mode::ReadOnly] mode.
+    #[inline]
+    pub fn is_read_only(&self) -> bool {
+        self.inner.txn_manager.is_none()
+    }
+
     /// Returns the manager that handles transaction messages.
     ///
     /// Requires [Mode::ReadWrite] and returns None otherwise.
@@ -115,15 +129,6 @@ impl Environment {
         self.inner.txn_manager.as_ref()
     }
 
-    /// Returns a raw pointer to the underlying MDBX environment.
-    ///
-    /// The caller **must** ensure that the pointer is not dereferenced after the lifetime of the
-    /// environment.
-    #[inline]
-    pub fn env(&self) -> *mut ffi::MDBX_env {
-        self.inner.env
-    }
-
     /// Create a read-only transaction for use with the environment.
     #[inline]
     pub fn begin_ro_txn(&self) -> Result<Transaction<'_, RO>> {
@@ -133,7 +138,7 @@ impl Environment {
     /// Create a read-write transaction for use with the environment. This method will block while
     /// there are any other read-write transactions open on the environment.
     pub fn begin_rw_txn(&self) -> Result<Transaction<'_, RW>> {
-        let sender = self.txn_manager().ok_or(Error::Access)?;
+        let sender = self.txn_manager().ok_or(Error::WriteTransactionUnsupportedInReadOnlyMode)?;
         let txn = loop {
             let (tx, rx) = sync_channel(0);
             sender
@@ -154,17 +159,40 @@ impl Environment {
         Ok(Transaction::new_from_ptr(self, txn.0))
     }
 
+    /// Returns a raw pointer to the underlying MDBX environment.
+    ///
+    /// The caller **must** ensure that the pointer is never dereferenced after the environment has
+    /// been dropped.
+    #[inline]
+    pub(crate) fn env_ptr(&self) -> *mut ffi::MDBX_env {
+        self.inner.env
+    }
+
+    /// Executes the given closure once
+    ///
+    /// This is only intended to be used when accessing mdbx ffi functions directly is required.
+    ///
+    /// The caller **must** ensure that the pointer is only used within the closure.
+    #[inline]
+    #[doc(hidden)]
+    pub fn with_raw_env_ptr<F, T>(&self, f: F) -> T
+    where
+        F: FnOnce(*mut ffi::MDBX_env) -> T,
+    {
+        (f)(self.env_ptr())
+    }
+
     /// Flush the environment data buffers to disk.
     pub fn sync(&self, force: bool) -> Result<bool> {
-        mdbx_result(unsafe { ffi::mdbx_env_sync_ex(self.env(), force, false) })
+        mdbx_result(unsafe { ffi::mdbx_env_sync_ex(self.env_ptr(), force, false) })
     }
 
     /// Retrieves statistics about this environment.
     pub fn stat(&self) -> Result<Stat> {
         unsafe {
             let mut stat = Stat::new();
             mdbx_result(ffi::mdbx_env_stat_ex(
-                self.env(),
+                self.env_ptr(),
                 ptr::null(),
                 stat.mdb_stat(),
                 size_of::<Stat>(),
@@ -178,7 +206,7 @@ impl Environment {
         unsafe {
             let mut info = Info(mem::zeroed());
             mdbx_result(ffi::mdbx_env_info_ex(
-                self.env(),
+                self.env_ptr(),
                 ptr::null(),
                 &mut info.0,
                 size_of::<Info>(),
@@ -237,8 +265,15 @@ impl Environment {
 /// This holds the raw pointer to the MDBX environment and the transaction manager.
 /// The env is opened via [mdbx_env_create](ffi::mdbx_env_create) and closed when this type drops.
 struct EnvironmentInner {
+    /// The raw pointer to the MDBX environment.
+    ///
+    /// Accessing the environment is thread-safe as long as long as this type exists.
     env: *mut ffi::MDBX_env,
+    /// Whether the environment was opened as WRITEMAP.
     env_kind: EnvironmentKind,
+    /// the sender half of the transaction manager channel
+    ///
+    /// Only set if the environment was opened in [Mode::ReadWrite] mode.
     txn_manager: Option<SyncSender<TxnManagerMessage>>,
 }
 

crates/storage/libmdbx-rs/src/error.rs

@@ -56,6 +56,7 @@ pub enum Error {
     TooLarge,
     DecodeErrorLenDiff,
     NestedTransactionsUnsupportedWithWriteMap,
+    WriteTransactionUnsupportedInReadOnlyMode,
     Other(i32),
 }
 

crates/storage/libmdbx-rs/src/transaction.rs

@@ -74,7 +74,7 @@ where
         let mut txn: *mut ffi::MDBX_txn = ptr::null_mut();
         unsafe {
             mdbx_result(ffi::mdbx_txn_begin_ex(
-                env.env(),
+                env.env_ptr(),
                 ptr::null_mut(),
                 K::OPEN_FLAGS,
                 &mut txn,
@@ -100,10 +100,14 @@ where
     /// The caller **must** ensure that the pointer is not used after the
     /// lifetime of the transaction.
     #[inline]
-    pub(crate) fn txn_execute<F: FnOnce(*mut ffi::MDBX_txn) -> T, T>(&self, f: F) -> T {
+    pub(crate) fn txn_execute<F, T>(&self, f: F) -> T
+    where
+        F: FnOnce(*mut ffi::MDBX_txn) -> T,
+    {
         self.inner.txn_execute(f)
     }
 
+    /// Returns a copy of the pointer to the underlying MDBX transaction.
     pub(crate) fn txn_ptr(&self) -> TransactionPtr {
         self.inner.txn.clone()
     }
@@ -114,6 +118,21 @@ where
         self.inner.txn.txn
     }
 
+    /// Executes the given closure once
+    ///
+    /// This is only intended to be used when accessing mdbx ffi functions directly is required.
+    ///
+    /// The caller **must** ensure that the pointer is only used within the closure.
+    #[inline]
+    #[doc(hidden)]
+    pub fn with_raw_tx_ptr<F, T>(&self, f: F) -> T
+    where
+        F: FnOnce(*mut ffi::MDBX_txn) -> T,
+    {
+        let _lock = self.inner.txn.lock.lock();
+        f(self.inner.txn.txn)
+    }
+
     /// Returns a raw pointer to the MDBX environment.
     pub fn env(&self) -> &Environment {
         self.inner.env
@@ -278,7 +297,10 @@ where
     }
 
     #[inline]
-    fn txn_execute<F: FnOnce(*mut ffi::MDBX_txn) -> T, T>(&self, f: F) -> T {
+    fn txn_execute<F, T>(&self, f: F) -> T
+    where
+        F: FnOnce(*mut ffi::MDBX_txn) -> T,
+    {
         self.txn.txn_execute(f)
     }
 }
@@ -449,7 +471,7 @@ impl<'env> Transaction<'env, RO> {
     /// Caller must close ALL other [Database] and [Cursor] instances pointing to the same dbi
     /// BEFORE calling this function.
     pub unsafe fn close_db(&self, db: Database<'_>) -> Result<()> {
-        mdbx_result(ffi::mdbx_dbi_close(self.env().env(), db.dbi()))?;
+        mdbx_result(ffi::mdbx_dbi_close(self.env().env_ptr(), db.dbi()))?;
 
         Ok(())
     }
@@ -501,7 +523,10 @@ impl TransactionPtr {
 
     /// Executes the given closure once the lock on the transaction is acquired.
     #[inline]
-    pub(crate) fn txn_execute<F: FnOnce(*mut ffi::MDBX_txn) -> T, T>(&self, f: F) -> T {
+    pub(crate) fn txn_execute<F, T>(&self, f: F) -> T
+    where
+        F: FnOnce(*mut ffi::MDBX_txn) -> T,
+    {
         let _lck = self.lock.lock();
         (f)(self.txn)
     }