Make db lock the cache while it reads data

Currently, read_with_cache locks the cache twice, when checking whether
the cache has the data and write the reading data to cache. So someone
can update the cache between two lockings.

Author: Seulgi Kim

core/src/blockchain/body_db.rs

@@ -20,7 +20,7 @@ use std::sync::Arc;
 
 use ctypes::transaction::ShardTransaction;
 use kvdb::{DBTransaction, KeyValueDB};
-use parking_lot::RwLock;
+use parking_lot::{Mutex, RwLock};
 use primitives::{Bytes, H256};
 use rlp::RlpStream;
 use rlp_compress::{blocks_swapper, compress, decompress};
@@ -37,8 +37,8 @@ pub struct BodyDB {
     parcel_address_cache: RwLock<HashMap<H256, TransactionAddress>>,
     pending_parcel_addresses: RwLock<HashMap<H256, Option<TransactionAddress>>>,
 
-    transaction_address_cache: RwLock<HashMap<H256, TransactionAddresses>>,
-    pending_transaction_addresses: RwLock<HashMap<H256, Option<TransactionAddresses>>>,
+    transaction_address_cache: Mutex<HashMap<H256, TransactionAddresses>>,
+    pending_transaction_addresses: Mutex<HashMap<H256, Option<TransactionAddresses>>>,
 
     db: Arc<KeyValueDB>,
 }
@@ -53,8 +53,8 @@ impl BodyDB {
             parcel_address_cache: RwLock::new(HashMap::new()),
             pending_parcel_addresses: RwLock::new(HashMap::new()),
 
-            transaction_address_cache: RwLock::new(HashMap::new()),
-            pending_transaction_addresses: RwLock::new(HashMap::new()),
+            transaction_address_cache: Default::default(),
+            pending_transaction_addresses: Default::default(),
 
             db,
         };
@@ -88,7 +88,7 @@ impl BodyDB {
 
     pub fn update_best_block(&self, batch: &mut DBTransaction, best_block_changed: &BestBlockChanged) {
         let mut pending_parcel_addresses = self.pending_parcel_addresses.write();
-        let mut pending_transaction_addresses = self.pending_transaction_addresses.write();
+        let mut pending_transaction_addresses = self.pending_transaction_addresses.lock();
         batch.extend_with_option_cache(
             db::COL_EXTRA,
             &mut *pending_parcel_addresses,
@@ -108,8 +108,8 @@ impl BodyDB {
         let mut parcel_address_cache = self.parcel_address_cache.write();
         let mut pending_parcel_addresses = self.pending_parcel_addresses.write();
 
-        let mut transaction_address_cache = self.transaction_address_cache.write();
-        let mut pending_transaction_addresses = self.pending_transaction_addresses.write();
+        let mut transaction_address_cache = self.transaction_address_cache.lock();
+        let mut pending_transaction_addresses = self.pending_transaction_addresses.lock();
 
         let new_parcels = mem::replace(&mut *pending_parcel_addresses, HashMap::new());
         let (retracted_parcels, enacted_parcels) =
@@ -296,12 +296,12 @@ impl BodyProvider for BodyDB {
 
     /// Get the address of parcel with given hash.
     fn transaction_address(&self, hash: &H256) -> Option<TransactionAddress> {
-        let result = self.db.read_with_cache(db::COL_EXTRA, &self.parcel_address_cache, hash)?;
+        let result = self.db.read_with_cache(db::COL_EXTRA, &mut *self.parcel_address_cache.write(), hash)?;
         Some(result)
     }
 
     fn transaction_addresses_by_tracker(&self, tracker: &H256) -> Option<TransactionAddresses> {
-        Some(self.db.read_with_cache(db::COL_EXTRA, &self.transaction_address_cache, tracker)?)
+        Some(self.db.read_with_cache(db::COL_EXTRA, &mut *self.transaction_address_cache.lock(), tracker)?)
     }
 
     /// Get block body data

core/src/blockchain/headerchain.rs

@@ -20,7 +20,7 @@ use std::sync::Arc;
 
 use ctypes::BlockNumber;
 use kvdb::{DBTransaction, KeyValueDB};
-use parking_lot::RwLock;
+use parking_lot::{Mutex, RwLock};
 use primitives::{Bytes, H256};
 use rlp_compress::{blocks_swapper, compress, decompress};
 
@@ -49,7 +49,7 @@ pub struct HeaderChain {
     // cache
     header_cache: RwLock<HashMap<H256, Bytes>>,
     detail_cache: RwLock<HashMap<H256, BlockDetails>>,
-    hash_cache: RwLock<HashMap<BlockNumber, H256>>,
+    hash_cache: Mutex<HashMap<BlockNumber, H256>>,
 
     db: Arc<KeyValueDB>,
 
@@ -100,8 +100,8 @@ impl HeaderChain {
             best_proposal_header_hash: RwLock::new(best_proposal_header_hash),
 
             header_cache: RwLock::new(HashMap::new()),
-            detail_cache: RwLock::new(HashMap::new()),
-            hash_cache: RwLock::new(HashMap::new()),
+            detail_cache: Default::default(),
+            hash_cache: Default::default(),
 
             db,
 
@@ -173,7 +173,7 @@ impl HeaderChain {
         let mut best_header_hash = self.best_header_hash.write();
         let mut best_proposal_header_hash = self.best_proposal_header_hash.write();
         let mut write_block_details = self.detail_cache.write();
-        let mut write_hashes = self.hash_cache.write();
+        let mut write_hashes = self.hash_cache.lock();
         // update best block
         if let Some(hash) = pending_best_header_hash.take() {
             *best_header_hash = hash;
@@ -368,7 +368,7 @@ impl HeaderProvider for HeaderChain {
 
     /// Get the familial details concerning a block.
     fn block_details(&self, hash: &H256) -> Option<BlockDetails> {
-        let result = self.db.read_with_cache(db::COL_EXTRA, &self.detail_cache, hash)?;
+        let result = self.db.read_with_cache(db::COL_EXTRA, &mut *self.detail_cache.write(), hash)?;
         Some(result)
     }
 
@@ -378,7 +378,7 @@ impl HeaderProvider for HeaderChain {
         if self.best_header().number() < index {
             return None
         }
-        let result = self.db.read_with_cache(db::COL_EXTRA, &self.hash_cache, &index)?;
+        let result = self.db.read_with_cache(db::COL_EXTRA, &mut *self.hash_cache.lock(), &index)?;
         Some(result)
     }
 

core/src/blockchain/invoice_db.rs

@@ -79,7 +79,7 @@ impl InvoiceProvider for InvoiceDB {
 
     /// Get invoices of block with given hash.
     fn block_invoices(&self, hash: &H256) -> Option<BlockInvoices> {
-        let result = self.db.read_with_cache(db::COL_EXTRA, &self.invoice_cache, hash)?;
+        let result = self.db.read_with_cache(db::COL_EXTRA, &mut *self.invoice_cache.write(), hash)?;
         Some(result)
     }
 

core/src/db.rs

@@ -190,21 +190,19 @@ pub trait Readable {
         R: Deref<Target = [u8]>;
 
     /// Returns value for given key either in cache or in database.
-    fn read_with_cache<K, T, C>(&self, col: Option<u32>, cache: &RwLock<C>, key: &K) -> Option<T>
+    fn read_with_cache<K, T, C>(&self, col: Option<u32>, cache: &mut C, key: &K) -> Option<T>
     where
         K: Key<T> + Eq + Hash + Clone,
         T: Clone + rlp::Decodable,
         C: Cache<K, T>, {
         {
-            let read = cache.read();
-            if let Some(v) = read.get(key) {
+            if let Some(v) = cache.get(key) {
                 return Some(v.clone())
             }
         }
 
         self.read(col, key).map(|value: T| {
-            let mut write = cache.write();
-            write.insert(key.clone(), value.clone());
+            cache.insert(key.clone(), value.clone());
             value
         })
     }