Merge pull request #1364 from ethcore/miner-lock

Fix lock order when updating sealing

ethcore/src/miner/miner.rs

@@ -106,34 +106,37 @@ impl Miner {
 	#[cfg_attr(feature="dev", allow(cyclomatic_complexity))]
 	fn prepare_sealing(&self, chain: &MiningBlockChainClient) {
 		trace!(target: "miner", "prepare_sealing: entering");
-		let transactions = self.transaction_queue.lock().unwrap().top_transactions();
-		let mut sealing_work = self.sealing_work.lock().unwrap();
-		let best_hash = chain.best_block_header().sha3();
 
+		let (transactions, mut open_block) = {
+			let transactions = {self.transaction_queue.lock().unwrap().top_transactions()};
+			let mut sealing_work = self.sealing_work.lock().unwrap();
+			let best_hash = chain.best_block_header().sha3();
 /*
-		// check to see if last ClosedBlock in would_seals is actually same parent block.
-		// if so
-		//   duplicate, re-open and push any new transactions.
-		//   if at least one was pushed successfully, close and enqueue new ClosedBlock;
-		//   otherwise, leave everything alone.
-		// otherwise, author a fresh block.
+			// check to see if last ClosedBlock in would_seals is actually same parent block.
+			// if so
+			//   duplicate, re-open and push any new transactions.
+			//   if at least one was pushed successfully, close and enqueue new ClosedBlock;
+			//   otherwise, leave everything alone.
+			// otherwise, author a fresh block.
 */
-		let mut open_block = match sealing_work.pop_if(|b| b.block().fields().header.parent_hash() == &best_hash) {
-			Some(old_block) => {
-				trace!(target: "miner", "Already have previous work; updating and returning");
-				// add transactions to old_block
-				let e = self.engine();
-				old_block.reopen(e, chain.vm_factory())
-			}
-			None => {
-				// block not found - create it.
-				trace!(target: "miner", "No existing work - making new block");
-				chain.prepare_open_block(
-					self.author(),
-					self.gas_floor_target(),
-					self.extra_data()
-				)
-			}
+			let open_block = match sealing_work.pop_if(|b| b.block().fields().header.parent_hash() == &best_hash) {
+				Some(old_block) => {
+					trace!(target: "miner", "Already have previous work; updating and returning");
+					// add transactions to old_block
+					let e = self.engine();
+					old_block.reopen(e, chain.vm_factory())
+				}
+				None => {
+					// block not found - create it.
+					trace!(target: "miner", "No existing work - making new block");
+					chain.prepare_open_block(
+						self.author(),
+						self.gas_floor_target(),
+						self.extra_data()
+					)
+				}
+			};
+			(transactions, open_block)
 		};
 
 		let mut invalid_transactions = HashSet::new();
@@ -163,14 +166,16 @@ impl Miner {
 
 		let block = open_block.close();
 
-		let mut queue = self.transaction_queue.lock().unwrap();
 		let fetch_account = |a: &Address| AccountDetails {
 			nonce: chain.latest_nonce(a),
 			balance: chain.latest_balance(a),
 		};
 
-		for hash in invalid_transactions.into_iter() {
-			queue.remove_invalid(&hash, &fetch_account);
+		{
+			let mut queue = self.transaction_queue.lock().unwrap();
+			for hash in invalid_transactions.into_iter() {
+				queue.remove_invalid(&hash, &fetch_account);
+			}
 		}
 
 		if !block.transactions().is_empty() {
@@ -196,6 +201,8 @@ impl Miner {
 				trace!(target: "miner", "prepare_sealing: unable to generate seal internally");
 			}
 		}
+
+		let mut sealing_work = self.sealing_work.lock().unwrap();
 		if sealing_work.peek_last_ref().map_or(true, |pb| pb.block().fields().header.hash() != block.block().fields().header.hash()) {
 			trace!(target: "miner", "Pushing a new, refreshed or borrowed pending {}...", block.block().fields().header.hash());
 			sealing_work.push(block);