wip

crates/storage/db-api/src/cursor.rs

@@ -1,7 +1,6 @@
 use std::{
-    cell::Cell,
+    cell::RefCell,
     fmt,
-    mem::ManuallyDrop,
     ops::{Bound, RangeBounds},
 };
 
@@ -372,54 +371,41 @@ impl<T: DupSort, CURSOR: DbDupCursorRO<T>> Iterator for DupWalker<'_, T, CURSOR>
     }
 }
 
-/// A wrapper around a cursor that returns it to a cell on drop.
-///
-/// This allows cursors to be reused across multiple operations,
-/// reducing the overhead of repeatedly creating new cursors.
-pub struct ReusableCursor<'tx, 'cell, T: Table, C: DbCursorRO<T>> {
-    cursor: ManuallyDrop<C>,
-    cell: &'cell Cell<Option<C>>,
-    _phantom: std::marker::PhantomData<&'tx T>,
+/// A guard that returns a cursor to its slot on drop.
+pub struct CursorGuard<'a, C> {
+    cursor: Option<C>,
+    slot: &'a RefCell<Option<C>>,
 }
 
-impl<'tx, 'cell, T, C> fmt::Debug for ReusableCursor<'tx, 'cell, T, C>
-where
-    T: Table,
-    C: DbCursorRO<T> + fmt::Debug,
-{
+impl<C: fmt::Debug> fmt::Debug for CursorGuard<'_, C> {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        f.debug_struct("ReusableCursor").field("cursor", &self.cursor).finish()
+        f.debug_struct("CursorGuard").field("cursor", &self.cursor).finish()
     }
 }
 
-impl<'tx, 'cell, T: Table, C: DbCursorRO<T>> ReusableCursor<'tx, 'cell, T, C> {
-    /// Creates a new `ReusableCursor` from a cursor and a cell to return it to.
-    pub const fn new(cursor: C, cell: &'cell Cell<Option<C>>) -> Self {
-        Self { cursor: ManuallyDrop::new(cursor), cell, _phantom: std::marker::PhantomData }
+impl<C> CursorGuard<'_, C> {
+    /// Creates a new `CursorGuard` from a cursor and a slot to return it to.
+    pub const fn new(cursor: C, slot: &RefCell<Option<C>>) -> CursorGuard<'_, C> {
+        CursorGuard { cursor: Some(cursor), slot }
     }
 }
 
-impl<'tx, 'cell, T: Table, C: DbCursorRO<T>> Drop for ReusableCursor<'tx, 'cell, T, C> {
+impl<C> Drop for CursorGuard<'_, C> {
     fn drop(&mut self) {
-        // SAFETY: We take ownership of the cursor and return it to the cell.
-        // The cursor won't be dropped automatically due to ManuallyDrop.
-        let cursor = unsafe { ManuallyDrop::take(&mut self.cursor) };
-        self.cell.set(Some(cursor));
+        *self.slot.borrow_mut() = self.cursor.take();
     }
 }
 
-impl<'tx, 'cell, T: Table, C: DbCursorRO<T>> std::ops::Deref for ReusableCursor<'tx, 'cell, T, C> {
+impl<C> std::ops::Deref for CursorGuard<'_, C> {
     type Target = C;
 
     fn deref(&self) -> &Self::Target {
-        &self.cursor
+        self.cursor.as_ref().expect("cursor is always Some until drop")
     }
 }
 
-impl<'tx, 'cell, T: Table, C: DbCursorRO<T>> std::ops::DerefMut
-    for ReusableCursor<'tx, 'cell, T, C>
-{
+impl<C> std::ops::DerefMut for CursorGuard<'_, C> {
     fn deref_mut(&mut self) -> &mut Self::Target {
-        &mut self.cursor
+        self.cursor.as_mut().expect("cursor is always Some until drop")
     }
 }

crates/storage/provider/src/providers/state/cursor_reuse.rs

@@ -1,24 +1,24 @@
 use reth_db_api::{
-    cursor::{DbCursorRO, DbDupCursorRO, ReusableCursor},
+    cursor::{CursorGuard, DbCursorRO, DbDupCursorRO},
     tables::{AccountsHistory, PlainStorageState, StorageChangeSets, StoragesHistory},
     transaction::DbTx,
     DatabaseError,
 };
-use std::cell::Cell;
+use std::cell::RefCell;
 
 /// Container for reusable database cursors.
 ///
 /// Holds optional cached cursors for frequently accessed tables. When a cursor is requested,
 /// it returns a cached cursor if available, otherwise creates a new one. The cursor is
-/// automatically returned to the cache when dropped via the `ReusableCursor` wrapper.
+/// automatically returned to the cache when dropped via the `CursorGuard` wrapper.
 ///
 /// This reduces cursor allocation overhead for state providers that perform many sequential
 /// database operations.
 pub(crate) struct ReusableStateCursors<TX: DbTx> {
-    storage_changesets: Cell<Option<TX::DupCursor<StorageChangeSets>>>,
-    plain_storage_state: Cell<Option<TX::DupCursor<PlainStorageState>>>,
-    accounts_history: Cell<Option<TX::Cursor<AccountsHistory>>>,
-    storages_history: Cell<Option<TX::Cursor<StoragesHistory>>>,
+    storage_changesets: RefCell<Option<TX::DupCursor<StorageChangeSets>>>,
+    plain_storage_state: RefCell<Option<TX::DupCursor<PlainStorageState>>>,
+    accounts_history: RefCell<Option<TX::Cursor<AccountsHistory>>>,
+    storages_history: RefCell<Option<TX::Cursor<StoragesHistory>>>,
 }
 
 impl<TX: DbTx> std::fmt::Debug for ReusableStateCursors<TX> {
@@ -28,84 +28,82 @@ impl<TX: DbTx> std::fmt::Debug for ReusableStateCursors<TX> {
 }
 
 impl<TX: DbTx> ReusableStateCursors<TX> {
-    /// Creates a new `ReusableStateCursors` with empty cursor cells.
+    /// Creates a new `ReusableStateCursors` with empty cursor slots.
     pub(crate) const fn new() -> Self {
         Self {
-            storage_changesets: Cell::new(None),
-            plain_storage_state: Cell::new(None),
-            accounts_history: Cell::new(None),
-            storages_history: Cell::new(None),
+            storage_changesets: RefCell::new(None),
+            plain_storage_state: RefCell::new(None),
+            accounts_history: RefCell::new(None),
+            storages_history: RefCell::new(None),
         }
     }
 
     /// Gets a reusable cursor for the `StorageChangeSets` table.
-    ///
-    /// If a cursor is cached, it will be reused. Otherwise, a new cursor is created.
-    pub(crate) fn storage_changesets<'tx, 'cell>(
-        &'cell self,
-        tx: &'tx TX,
-    ) -> Result<
-        ReusableCursor<'tx, 'cell, StorageChangeSets, TX::DupCursor<StorageChangeSets>>,
-        DatabaseError,
-    >
+    pub(crate) fn storage_changesets(
+        &self,
+        tx: &TX,
+    ) -> Result<CursorGuard<'_, TX::DupCursor<StorageChangeSets>>, DatabaseError>
     where
         TX::DupCursor<StorageChangeSets>: DbDupCursorRO<StorageChangeSets>,
     {
-        let cursor =
-            self.storage_changesets.take().map(Ok).unwrap_or_else(|| tx.cursor_dup_read())?;
-        Ok(ReusableCursor::new(cursor, &self.storage_changesets))
+        let cursor = self
+            .storage_changesets
+            .borrow_mut()
+            .take()
+            .map(Ok)
+            .unwrap_or_else(|| tx.cursor_dup_read())?;
+        Ok(CursorGuard::new(cursor, &self.storage_changesets))
     }
 
     /// Gets a reusable cursor for the `PlainStorageState` table.
-    ///
-    /// If a cursor is cached, it will be reused. Otherwise, a new cursor is created.
-    pub(crate) fn plain_storage_state<'tx, 'cell>(
-        &'cell self,
-        tx: &'tx TX,
-    ) -> Result<
-        ReusableCursor<'tx, 'cell, PlainStorageState, TX::DupCursor<PlainStorageState>>,
-        DatabaseError,
-    >
+    pub(crate) fn plain_storage_state(
+        &self,
+        tx: &TX,
+    ) -> Result<CursorGuard<'_, TX::DupCursor<PlainStorageState>>, DatabaseError>
     where
         TX::DupCursor<PlainStorageState>: DbDupCursorRO<PlainStorageState>,
     {
-        let cursor =
-            self.plain_storage_state.take().map(Ok).unwrap_or_else(|| tx.cursor_dup_read())?;
-        Ok(ReusableCursor::new(cursor, &self.plain_storage_state))
+        let cursor = self
+            .plain_storage_state
+            .borrow_mut()
+            .take()
+            .map(Ok)
+            .unwrap_or_else(|| tx.cursor_dup_read())?;
+        Ok(CursorGuard::new(cursor, &self.plain_storage_state))
     }
 
     /// Gets a reusable cursor for the `AccountsHistory` table.
-    ///
-    /// If a cursor is cached, it will be reused. Otherwise, a new cursor is created.
-    pub(crate) fn accounts_history<'tx, 'cell>(
-        &'cell self,
-        tx: &'tx TX,
-    ) -> Result<
-        ReusableCursor<'tx, 'cell, AccountsHistory, TX::Cursor<AccountsHistory>>,
-        DatabaseError,
-    >
+    pub(crate) fn accounts_history(
+        &self,
+        tx: &TX,
+    ) -> Result<CursorGuard<'_, TX::Cursor<AccountsHistory>>, DatabaseError>
     where
         TX::Cursor<AccountsHistory>: DbCursorRO<AccountsHistory>,
     {
-        let cursor = self.accounts_history.take().map(Ok).unwrap_or_else(|| tx.cursor_read())?;
-        Ok(ReusableCursor::new(cursor, &self.accounts_history))
+        let cursor = self
+            .accounts_history
+            .borrow_mut()
+            .take()
+            .map(Ok)
+            .unwrap_or_else(|| tx.cursor_read())?;
+        Ok(CursorGuard::new(cursor, &self.accounts_history))
     }
 
     /// Gets a reusable cursor for the `StoragesHistory` table.
-    ///
-    /// If a cursor is cached, it will be reused. Otherwise, a new cursor is created.
-    pub(crate) fn storages_history<'tx, 'cell>(
-        &'cell self,
-        tx: &'tx TX,
-    ) -> Result<
-        ReusableCursor<'tx, 'cell, StoragesHistory, TX::Cursor<StoragesHistory>>,
-        DatabaseError,
-    >
+    pub(crate) fn storages_history(
+        &self,
+        tx: &TX,
+    ) -> Result<CursorGuard<'_, TX::Cursor<StoragesHistory>>, DatabaseError>
     where
         TX::Cursor<StoragesHistory>: DbCursorRO<StoragesHistory>,
     {
-        let cursor = self.storages_history.take().map(Ok).unwrap_or_else(|| tx.cursor_read())?;
-        Ok(ReusableCursor::new(cursor, &self.storages_history))
+        let cursor = self
+            .storages_history
+            .borrow_mut()
+            .take()
+            .map(Ok)
+            .unwrap_or_else(|| tx.cursor_read())?;
+        Ok(CursorGuard::new(cursor, &self.storages_history))
     }
 }
 

crates/storage/provider/src/providers/state/historical.rs

@@ -503,7 +503,7 @@ impl<Provider: DBProvider + BlockNumReader> HistoricalStateProvider<Provider> {
 
     /// Returns a new provider that takes the `TX` as reference
     #[inline(always)]
-    fn as_ref(&self) -> HistoricalStateProviderRef<'_, Provider> {
+    const fn as_ref(&self) -> HistoricalStateProviderRef<'_, Provider> {
         HistoricalStateProviderRef::new_with_lowest_available_blocks(
             &self.provider,
             self.block_number,

crates/storage/provider/src/providers/state/latest.rs

@@ -199,7 +199,7 @@ impl<Provider: DBProvider> LatestStateProvider<Provider> {
 
     /// Returns a new provider that takes the `TX` as reference
     #[inline(always)]
-    fn as_ref(&self) -> LatestStateProviderRef<'_, Provider> {
+    const fn as_ref(&self) -> LatestStateProviderRef<'_, Provider> {
         LatestStateProviderRef::new(&self.0)
     }
 }