Skip to content

Commit

Permalink
Fix a rare deadlock between FLUSHB and PUSH
Browse files Browse the repository at this point in the history
Signed-off-by: Valerian Saliou <valerian@valeriansaliou.name>
  • Loading branch information
valeriansaliou committed Apr 19, 2019
1 parent 373458a commit d96546b
Show file tree
Hide file tree
Showing 6 changed files with 27 additions and 7 deletions.
2 changes: 2 additions & 0 deletions src/executor/flushb.rs
Expand Up @@ -15,7 +15,9 @@ impl ExecutorFlushB {
if let StoreItem(collection, Some(bucket), None) = store {
// Important: acquire database access read lock, and reference it in context. This \
// prevents the database from being erased while using it in this block.
// Notice: acquire FST lock in write mode, as we will erase it.
general_kv_access_lock_read!();
general_fst_access_lock_write!();

if let Ok(kv_store) = StoreKVPool::acquire(StoreKVAcquireMode::OpenOnly, collection) {
// Important: acquire bucket store write lock
Expand Down
5 changes: 5 additions & 0 deletions src/executor/flushc.rs
Expand Up @@ -16,6 +16,11 @@ impl ExecutorFlushC {
// even if dropped in the inner function, as this caller would still own a reference to \
// it.
if let StoreItem(collection, None, None) = store {
// Acquire KV + FST locks in write mode, as we will erase them, we need to prevent any \
// other consumer to use them.
general_kv_access_lock_write!();
general_fst_access_lock_write!();

match (
StoreKVActionBuilder::erase(collection, None),
StoreFSTActionBuilder::erase(collection, None),
Expand Down
18 changes: 18 additions & 0 deletions src/executor/macros.rs
Expand Up @@ -45,6 +45,15 @@ macro_rules! general_kv_access_lock_read {
};
}

#[macro_export]
macro_rules! general_kv_access_lock_write {
() => {
use crate::store::kv::STORE_ACCESS_LOCK;

let _kv_access = STORE_ACCESS_LOCK.write().unwrap();
};
}

#[macro_export]
macro_rules! general_fst_access_lock_read {
() => {
Expand All @@ -53,3 +62,12 @@ macro_rules! general_fst_access_lock_read {
let _fst_access = GRAPH_ACCESS_LOCK.read().unwrap();
};
}

#[macro_export]
macro_rules! general_fst_access_lock_write {
() => {
use crate::store::fst::GRAPH_ACCESS_LOCK;

let _fst_access = GRAPH_ACCESS_LOCK.write().unwrap();
};
}
2 changes: 1 addition & 1 deletion src/store/fst.rs
Expand Up @@ -867,7 +867,7 @@ impl StoreFSTActionBuilder {
}

pub fn erase<'a, T: Into<&'a str>>(collection: T, bucket: Option<T>) -> Result<u32, ()> {
Self::dispatch_erase("fst", collection, bucket, &*GRAPH_ACCESS_LOCK)
Self::dispatch_erase("fst", collection, bucket)
}

fn build(store: StoreFSTBox) -> StoreFSTAction {
Expand Down
5 changes: 0 additions & 5 deletions src/store/generic.rs
Expand Up @@ -147,16 +147,11 @@ pub trait StoreGenericActionBuilder {
kind: &str,
collection: T,
bucket: Option<T>,
access_lock: &Arc<RwLock<bool>>,
) -> Result<u32, ()> {
let collection_str = collection.into();

info!("{} erase requested on collection: {}", kind, collection_str);

// Acquire access lock (in blocking write mode), and reference it in context
// Notice: this prevents store to be acquired from any context
let _access = access_lock.write().unwrap();

if let Some(bucket) = bucket {
Self::proceed_erase_bucket(collection_str, bucket.into())
} else {
Expand Down
2 changes: 1 addition & 1 deletion src/store/kv.rs
Expand Up @@ -397,7 +397,7 @@ impl StoreKVActionBuilder {
}

pub fn erase<'a, T: Into<&'a str>>(collection: T, bucket: Option<T>) -> Result<u32, ()> {
Self::dispatch_erase("kv", collection, bucket, &*STORE_ACCESS_LOCK)
Self::dispatch_erase("kv", collection, bucket)
}

fn build(bucket: StoreItemPart, store: Option<StoreKVBox>) -> StoreKVAction {
Expand Down

0 comments on commit d96546b

Please sign in to comment.