Revert "Don’t hold onto ReadOptions.inner when iterating" (rust-rocks…

…db#661)

src/db_iterator.rs

@@ -72,10 +72,15 @@ pub type DBRawIterator<'a> = DBRawIteratorWithThreadMode<'a, DB>;
 pub struct DBRawIteratorWithThreadMode<'a, D: DBAccess> {
     inner: std::ptr::NonNull<ffi::rocksdb_iterator_t>,
 
-    /// When iterate_upper_bound or iterate_lower_bound is set, the inner
-    /// C iterator keeps a pointer to the bounds.  We need to hold onto those
-    /// slices or else C iterator ends up reading freed memory.
-    _bounds: (Option<Vec<u8>>, Option<Vec<u8>>),
+    /// When iterate_lower_bound or iterate_upper_bound are set, the inner
+    /// C iterator keeps a pointer to the upper bound inside `_readopts`.
+    /// Storing this makes sure the upper bound is always alive when the
+    /// iterator is being used.
+    ///
+    /// And yes, we need to store the entire ReadOptions structure since C++
+    /// ReadOptions keep reference to C rocksdb_readoptions_t wrapper which
+    /// point to vectors we own.  See issue #660.
+    _readopts: ReadOptions,
 
     db: PhantomData<&'a D>,
 }
@@ -96,17 +101,15 @@ impl<'a, D: DBAccess> DBRawIteratorWithThreadMode<'a, D> {
         Self::from_inner(inner, readopts)
     }
 
-    fn from_inner(inner: *mut ffi::rocksdb_iterator_t, mut readopts: ReadOptions) -> Self {
+    fn from_inner(inner: *mut ffi::rocksdb_iterator_t, readopts: ReadOptions) -> Self {
         // This unwrap will never fail since rocksdb_create_iterator and
         // rocksdb_create_iterator_cf functions always return non-null.  They
         // use new and deference the result so any nulls would end up in SIGSEGV
         // there and we have bigger issue.
         let inner = std::ptr::NonNull::new(inner).unwrap();
-        let lower = readopts.iterate_lower_bound.take();
-        let upper = readopts.iterate_upper_bound.take();
         Self {
             inner,
-            _bounds: (lower, upper),
+            _readopts: readopts,
             db: PhantomData,
         }
     }

src/db_options.rs

@@ -336,8 +336,8 @@ pub struct BlockBasedOptions {
 
 pub struct ReadOptions {
     pub(crate) inner: *mut ffi::rocksdb_readoptions_t,
-    pub(crate) iterate_upper_bound: Option<Vec<u8>>,
-    pub(crate) iterate_lower_bound: Option<Vec<u8>>,
+    iterate_upper_bound: Option<Vec<u8>>,
+    iterate_lower_bound: Option<Vec<u8>>,
 }
 
 /// Configuration of cuckoo-based storage.