Fix deadlock in tests that are using with_gossip (#2533)

* fixed gossip related tests deadlock

* cleaning sync tests log

* fixed typo in logs

* send GRANDPA messages without holding validator lock

* Revert "fixed gossip related tests deadlock"

This reverts commit d3fe55e45e32b7ea2e9f05f1c511972c708209ad.
This commit is contained in:
Svyatoslav Nikolsky
2019-05-10 15:14:42 +03:00
committed by Gavin Wood
parent dfbaedd535
commit 42fa5f6209
3 changed files with 23 additions and 33 deletions
@@ -425,6 +425,8 @@ struct Inner<Block: BlockT> {
next_rebroadcast: Instant,
}
type MaybeMessage<Block> = Option<(Vec<PeerId>, NeighborPacket<NumberFor<Block>>)>;
impl<Block: BlockT> Inner<Block> {
fn new(config: crate::Config) -> Self {
Inner {
@@ -437,11 +439,9 @@ impl<Block: BlockT> Inner<Block> {
}
/// Note a round in a set has started.
fn note_round<F>(&mut self, round: Round, set_id: SetId, send_neighbor: F)
where F: FnOnce(Vec<PeerId>, NeighborPacket<NumberFor<Block>>)
{
fn note_round(&mut self, round: Round, set_id: SetId) -> MaybeMessage<Block> {
if self.local_view.round == round && self.local_view.set_id == set_id {
return
return None;
}
debug!(target: "afg", "Voter {} noting beginning of round {:?} to network.",
@@ -451,28 +451,28 @@ impl<Block: BlockT> Inner<Block> {
self.local_view.set_id = set_id;
self.live_topics.push(round, set_id);
self.multicast_neighbor_packet(send_neighbor);
self.multicast_neighbor_packet()
}
/// Note that a voter set with given ID has started. Does nothing if the last
/// call to the function was with the same `set_id`.
fn note_set<F>(&mut self, set_id: SetId, send_neighbor: F)
where F: FnOnce(Vec<PeerId>, NeighborPacket<NumberFor<Block>>)
{
if self.local_view.set_id == set_id { return }
fn note_set(&mut self, set_id: SetId) -> MaybeMessage<Block> {
if self.local_view.set_id == set_id {
return None;
}
self.local_view.update_set(set_id);
self.live_topics.push(Round(0), set_id);
self.multicast_neighbor_packet(send_neighbor);
self.multicast_neighbor_packet()
}
/// Note that we've imported a commit finalizing a given block.
fn note_commit_finalized<F>(&mut self, finalized: NumberFor<Block>, send_neighbor: F)
where F: FnOnce(Vec<PeerId>, NeighborPacket<NumberFor<Block>>)
{
fn note_commit_finalized(&mut self, finalized: NumberFor<Block>) -> MaybeMessage<Block> {
if self.local_view.last_commit.as_ref() < Some(&finalized) {
self.local_view.last_commit = Some(finalized);
self.multicast_neighbor_packet(send_neighbor)
self.multicast_neighbor_packet()
} else {
None
}
}
@@ -560,9 +560,7 @@ impl<Block: BlockT> Inner<Block> {
(neighbor_topics, Action::Discard(cb))
}
fn multicast_neighbor_packet<F>(&self, send_neighbor: F)
where F: FnOnce(Vec<PeerId>, NeighborPacket<NumberFor<Block>>)
{
fn multicast_neighbor_packet(&self) -> MaybeMessage<Block> {
let packet = NeighborPacket {
round: self.local_view.round,
set_id: self.local_view.set_id,
@@ -570,7 +568,7 @@ impl<Block: BlockT> Inner<Block> {
};
let peers = self.peers.inner.keys().cloned().collect();
send_neighbor(peers, packet);
Some((peers, packet))
}
}
@@ -596,21 +594,24 @@ impl<Block: BlockT> GossipValidator<Block> {
pub(super) fn note_round<F>(&self, round: Round, set_id: SetId, send_neighbor: F)
where F: FnOnce(Vec<PeerId>, NeighborPacket<NumberFor<Block>>)
{
self.inner.write().note_round(round, set_id, send_neighbor);
self.inner.write().note_round(round, set_id)
.map(|(to, msg)| send_neighbor(to, msg));
}
/// Note that a voter set with given ID has started.
pub(super) fn note_set<F>(&self, set_id: SetId, send_neighbor: F)
where F: FnOnce(Vec<PeerId>, NeighborPacket<NumberFor<Block>>)
{
self.inner.write().note_set(set_id, send_neighbor);
self.inner.write().note_set(set_id)
.map(|(to, msg)| send_neighbor(to, msg));
}
/// Note that we've imported a commit finalizing a given block.
pub(super) fn note_commit_finalized<F>(&self, finalized: NumberFor<Block>, send_neighbor: F)
where F: FnOnce(Vec<PeerId>, NeighborPacket<NumberFor<Block>>)
{
self.inner.write().note_commit_finalized(finalized, send_neighbor);
self.inner.write().note_commit_finalized(finalized)
.map(|(to, msg)| send_neighbor(to, msg));
}
fn report(&self, who: PeerId, cost_benefit: i32) {
+1 -1
View File
@@ -685,7 +685,7 @@ impl<B: BlockT, S: NetworkSpecialization<B>, H: ExHashT> Protocol<B, S, H> {
{
for (who, peer) in self.context_data.peers.iter() {
if peer.block_request.as_ref().map_or(false, |(t, _)| (tick - *t).as_secs() > REQUEST_TIMEOUT_SEC) {
trace!(target: "sync", "Reqeust timeout {}", who);
trace!(target: "sync", "Request timeout {}", who);
aborting.push(who.clone());
} else if peer.obsolete_requests.values().any(|t| (tick - *t).as_secs() > REQUEST_TIMEOUT_SEC) {
trace!(target: "sync", "Obsolete timeout {}", who);
-11
View File
@@ -47,7 +47,6 @@ use runtime_primitives::{Justification, ConsensusEngineId};
use crate::service::{network_channel, NetworkChan, NetworkLink, NetworkMsg, NetworkPort, TransactionPool};
use crate::specialization::NetworkSpecialization;
use test_client::{self, AccountKeyring};
use log::debug;
pub use test_client::runtime::{Block, Extrinsic, Hash, Transfer};
pub use test_client::TestClient;
@@ -170,7 +169,6 @@ impl<S: NetworkSpecialization<Block>> Link<Block> for TestLink<S> {
/// with `ImportQueue`.
#[cfg(any(test, feature = "test-helpers"))]
fn synchronized(&self) {
trace!(target: "test_network", "Synchronizing");
drop(self.network_to_protocol_sender.unbounded_send(FromNetworkMsg::Synchronize))
}
}
@@ -241,14 +239,12 @@ impl<S: NetworkSpecialization<Block>> ProtocolChannel<S> {
/// Wait until synchronization response is generated by the protocol.
pub fn wait_sync(&self) -> Result<(), RecvError> {
trace!(target: "test_network", "Waiting for sync");
loop {
match self.protocol_to_network_receiver.receiver().recv() {
Ok(NetworkMsg::Synchronized) => return Ok(()),
Err(error) => return Err(error),
Ok(msg) => self.buffered_messages.lock().push_back(msg),
}
trace!(target: "test_network", "Retrying sync");
}
}
@@ -390,9 +386,7 @@ impl<D, S: NetworkSpecialization<Block>> Peer<D, S> {
/// Synchronize with import queue.
#[cfg(any(test, feature = "test-helpers"))]
fn import_queue_sync(&self) {
trace!(target: "test_network", "syncing this queue");
self.import_queue.synchronize();
trace!(target: "test_network", "wating for sync to finish");
let _ = self.net_proto_channel.wait_sync();
}
@@ -728,7 +722,6 @@ pub trait TestNetFactory: Sized {
}
loop {
debug!(target: "test_network", "loop iteration");
// we only deliver Status messages during start
let need_continue = self.route_single(true, None, &|msg| match *msg {
NetworkMsg::Outgoing(_, crate::message::generic::Message::Status(_)) => true,
@@ -755,7 +748,6 @@ pub trait TestNetFactory: Sized {
let mut to_disconnect = HashSet::new();
let peers = self.peers();
for peer in peers {
debug!(target: "test_network", "checking peer");
if let Some(message) = peer.pending_message(message_filter) {
match message {
NetworkMsg::Outgoing(recipient_id, packet) => {
@@ -794,13 +786,10 @@ pub trait TestNetFactory: Sized {
}
}
}
debug!(target: "test_network", "syncing queues");
// make sure that the protocol(s) has processed all messages that have been queued
self.peers().iter().for_each(|peer| peer.import_queue_sync());
debug!(target: "test_network", "queues synced");
had_messages
}