diff --git a/ethcore/src/blockchain/blockchain.rs b/ethcore/src/blockchain/blockchain.rs index de0c72a3826..c359a73607d 100644 --- a/ethcore/src/blockchain/blockchain.rs +++ b/ethcore/src/blockchain/blockchain.rs @@ -196,6 +196,7 @@ pub struct BlockChain { pending_best_block: RwLock>, pending_block_hashes: RwLock>, + pending_block_details: RwLock>, pending_transaction_addresses: RwLock>>, } @@ -438,6 +439,7 @@ impl BlockChain { cache_man: Mutex::new(cache_man), pending_best_block: RwLock::new(None), pending_block_hashes: RwLock::new(HashMap::new()), + pending_block_details: RwLock::new(HashMap::new()), pending_transaction_addresses: RwLock::new(HashMap::new()), }; @@ -894,17 +896,6 @@ impl BlockChain { /// Prepares extras update. fn prepare_update(&self, batch: &mut DBTransaction, update: ExtrasUpdate, is_best: bool) { - { - let block_hashes: Vec<_> = update.block_details.keys().cloned().collect(); - - let mut write_details = self.block_details.write(); - batch.extend_with_cache(db::COL_EXTRA, &mut *write_details, update.block_details, CacheUpdatePolicy::Overwrite); - - let mut cache_man = self.cache_man.lock(); - for hash in block_hashes { - cache_man.note_used(CacheID::BlockDetails(hash)); - } - } { let mut write_receipts = self.block_receipts.write(); @@ -916,7 +907,7 @@ impl BlockChain { batch.extend_with_cache(db::COL_EXTRA, &mut *write_blocks_blooms, update.blocks_blooms, CacheUpdatePolicy::Remove); } - // These cached values must be updated last with all three locks taken to avoid + // These cached values must be updated last with all four locks taken to avoid // cache decoherence { let mut best_block = self.pending_best_block.write(); @@ -934,8 +925,10 @@ impl BlockChain { }, } let mut write_hashes = self.pending_block_hashes.write(); + let mut write_details = self.pending_block_details.write(); let mut write_txs = self.pending_transaction_addresses.write(); + batch.extend_with_cache(db::COL_EXTRA, &mut *write_details, update.block_details, CacheUpdatePolicy::Overwrite); batch.extend_with_cache(db::COL_EXTRA, &mut *write_hashes, update.block_hashes, CacheUpdatePolicy::Overwrite); batch.extend_with_option_cache(db::COL_EXTRA, &mut *write_txs, update.transactions_addresses, CacheUpdatePolicy::Overwrite); } @@ -945,9 +938,11 @@ impl BlockChain { pub fn commit(&self) { let mut pending_best_block = self.pending_best_block.write(); let mut pending_write_hashes = self.pending_block_hashes.write(); + let mut pending_block_details = self.pending_block_details.write(); let mut pending_write_txs = self.pending_transaction_addresses.write(); let mut best_block = self.best_block.write(); + let mut write_block_details = self.block_details.write(); let mut write_hashes = self.block_hashes.write(); let mut write_txs = self.transaction_addresses.write(); // update best block @@ -960,9 +955,11 @@ impl BlockChain { let pending_hashes_keys: Vec<_> = pending_write_hashes.keys().cloned().collect(); let enacted_txs_keys: Vec<_> = enacted_txs.keys().cloned().collect(); + let pending_block_hashes: Vec<_> = pending_block_details.keys().cloned().collect(); write_hashes.extend(mem::replace(&mut *pending_write_hashes, HashMap::new())); write_txs.extend(enacted_txs.into_iter().map(|(k, v)| (k, v.expect("Transactions were partitioned; qed")))); + write_block_details.extend(mem::replace(&mut *pending_block_details, HashMap::new())); for hash in retracted_txs.keys() { write_txs.remove(hash); @@ -976,6 +973,10 @@ impl BlockChain { for hash in enacted_txs_keys { cache_man.note_used(CacheID::TransactionAddresses(hash)); } + + for hash in pending_block_hashes { + cache_man.note_used(CacheID::BlockDetails(hash)); + } } /// Iterator that lists `first` and then all of `first`'s ancestors, by hash. @@ -1296,6 +1297,11 @@ impl BlockChain { ancient_block_number: best_ancient_block.as_ref().map(|b| b.number), } } + + #[cfg(test)] + pub fn db(&self) -> &Arc { + &self.db + } } #[cfg(test)] diff --git a/ethcore/src/client/client.rs b/ethcore/src/client/client.rs index 49195d952a3..2d22b2d433e 100644 --- a/ethcore/src/client/client.rs +++ b/ethcore/src/client/client.rs @@ -1216,3 +1216,33 @@ impl MayPanic for Client { self.panic_handler.on_panic(closure); } } + + +#[test] +fn should_not_cache_details_before_commit() { + use tests::helpers::*; + use std::thread; + use std::time::Duration; + use std::sync::atomic::{AtomicBool, Ordering}; + + let client = generate_dummy_client(0); + let genesis = client.chain_info().best_block_hash; + let (new_hash, new_block) = get_good_dummy_block_hash(); + + let go = { + // Separate thread uncommited transaction + let go = Arc::new(AtomicBool::new(false)); + let go_thread = go.clone(); + let another_client = client.reference().clone(); + thread::spawn(move || { + let mut batch = DBTransaction::new(&*another_client.chain.read().db().clone()); + another_client.chain.read().insert_block(&mut batch, &new_block, Vec::new()); + go_thread.store(true, Ordering::SeqCst); + }); + go + }; + + while !go.load(Ordering::SeqCst) { thread::park_timeout(Duration::from_millis(5)); } + + assert!(client.tree_route(&genesis, &new_hash).is_none()); +} diff --git a/ethcore/src/tests/helpers.rs b/ethcore/src/tests/helpers.rs index 7b826472039..7b952b30ced 100644 --- a/ethcore/src/tests/helpers.rs +++ b/ethcore/src/tests/helpers.rs @@ -388,7 +388,7 @@ pub fn get_good_dummy_block_fork_seq(start_number: usize, count: usize, parent_h r } -pub fn get_good_dummy_block() -> Bytes { +pub fn get_good_dummy_block_hash() -> (H256, Bytes) { let mut block_header = Header::new(); let test_spec = get_test_spec(); let test_engine = &test_spec.engine; @@ -399,7 +399,12 @@ pub fn get_good_dummy_block() -> Bytes { block_header.set_parent_hash(test_spec.genesis_header().hash()); block_header.set_state_root(test_spec.genesis_header().state_root().clone()); - create_test_block(&block_header) + (block_header.hash(), create_test_block(&block_header)) +} + +pub fn get_good_dummy_block() -> Bytes { + let (_, bytes) = get_good_dummy_block_hash(); + bytes } pub fn get_bad_state_dummy_block() -> Bytes {