Revert "Don’t hold onto ReadOptions.inner when iterating" (#661)

master
Michal Nazarewicz 2 years ago committed by GitHub
parent 78dba2100b
commit 06ba0738c7
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
  1. 19
      src/db_iterator.rs
  2. 4
      src/db_options.rs

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

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

Loading…
Cancel
Save