Fixed shared cache race on import (#4194)

* Fixed is_best race on import

* Take import lock outside of backend

* Actually take the lock

substrate/client/src/call_executor.rs

@@ -96,7 +96,10 @@ where
 			None,
 		)
 		.map(|(result, _, _)| result)?;
-		self.backend.destroy_state(state)?;
+		{
+			let _lock = self.backend.get_import_lock().read();
+			self.backend.destroy_state(state)?;
+		}
 		Ok(return_data.into_encoded())
 	}
 
@@ -179,7 +182,10 @@ where
 			)
 			.map(|(result, _, _)| result)
 		}?;
-		self.backend.destroy_state(state)?;
+		{
+			let _lock = self.backend.get_import_lock().read();
+			self.backend.destroy_state(state)?;
+		}
 		Ok(result)
 	}
 
@@ -194,7 +200,10 @@ where
 			None,
 		);
 		let version = self.executor.runtime_version(&mut ext);
-		self.backend.destroy_state(state)?;
+		{
+			let _lock = self.backend.get_import_lock().read();
+			self.backend.destroy_state(state)?;
+		}
 		version.ok_or(error::Error::VersionInvalid.into())
 	}
 

substrate/client/src/client.rs

@@ -710,7 +710,7 @@ impl<B, E, Block, RA> Client<B, E, Block, RA> where
 		Err: From<error::Error>,
 	{
 		let inner = || {
-			let _import_lock = self.backend.get_import_lock().lock();
+			let _import_lock = self.backend.get_import_lock().write();
 
 			let mut op = ClientImportOperation {
 				op: self.backend.begin_operation()?,

substrate/client/src/in_mem.rs

@@ -18,7 +18,7 @@
 
 use std::collections::{HashMap, HashSet};
 use std::sync::Arc;
-use parking_lot::{RwLock, Mutex};
+use parking_lot::RwLock;
 use primitives::{ChangesTrieConfiguration, storage::well_known_keys};
 use primitives::offchain::storage::{
 	InMemOffchainStorage as OffchainStorage
@@ -562,7 +562,7 @@ where
 	states: RwLock<HashMap<Block::Hash, InMemory<H>>>,
 	changes_trie_storage: ChangesTrieStorage<Block, H>,
 	blockchain: Blockchain<Block>,
-	import_lock: Mutex<()>,
+	import_lock: RwLock<()>,
 }
 
 impl<Block, H> Backend<Block, H>
@@ -712,7 +712,7 @@ where
 		Ok(Zero::zero())
 	}
 
-	fn get_import_lock(&self) -> &Mutex<()> {
+	fn get_import_lock(&self) -> &RwLock<()> {
 		&self.import_lock
 	}
 }

substrate/client/src/light/backend.rs

@@ -19,7 +19,7 @@
 
 use std::collections::HashMap;
 use std::sync::Arc;
-use parking_lot::{RwLock, Mutex};
+use parking_lot::RwLock;
 
 use state_machine::{Backend as StateBackend, TrieBackend, backend::InMemory as InMemoryState, ChangesTrieTransaction};
 use primitives::offchain::storage::InMemOffchainStorage;
@@ -49,7 +49,7 @@ const IN_MEMORY_EXPECT_PROOF: &str = "InMemory state backend has Void error type
 pub struct Backend<S, H: Hasher> {
 	blockchain: Arc<Blockchain<S>>,
 	genesis_state: RwLock<Option<InMemoryState<H>>>,
-	import_lock: Mutex<()>,
+	import_lock: RwLock<()>,
 }
 
 /// Light block (header and justification) import operation.
@@ -216,7 +216,7 @@ impl<S, Block, H> ClientBackend<Block, H> for Backend<S, H> where
 		Err(ClientError::NotAvailableOnLightClient)
 	}
 
-	fn get_import_lock(&self) -> &Mutex<()> {
+	fn get_import_lock(&self) -> &RwLock<()> {
 		&self.import_lock
 	}
 }