fix panic in seed subcommand (#401)

Co-authored-by: Jane Lusby <jane@zfnd.org>

Prior to this change, the seed subcommand would consistently encounter a panic in one of the background tasks, but would continue running after the panic. This is indicative of two bugs. 

First, zebrad was not configured to treat panics as non recoverable and instead defaulted to the tokio defaults, which are to catch panics in tasks and return them via the join handle if available, or to print them if the join handle has been discarded. This is likely a poor fit for zebrad as an application, we do not need to maximize uptime or minimize the extent of an outage should one of our tasks / services start encountering panics. Ignoring a panic increases our risk of observing invalid state, causing all sorts of wild and bad bugs. To deal with this we've switched the default panic behavior from `unwind` to `abort`. This makes panics fail immediately and take down the entire application, regardless of where they occur, which is consistent with our treatment of misbehaving connections.

The second bug is the panic itself. This was triggered by a duplicate entry in the initial_peers set. To fix this we've switched the storage for the peers from a `Vec` to a `HashSet`, which has similar properties but guarantees uniqueness of its keys.
This commit is contained in:
Jane Lusby 2020-05-27 17:40:12 -07:00 committed by GitHub
parent 8276bed400
commit 8c178c3ee4
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 18 additions and 9 deletions

View File

@ -9,3 +9,9 @@ members = [
"zebra-client",
"zebrad",
]
[profile.dev]
panic = "abort"
[profile.release]
panic = "abort"

View File

@ -1,7 +1,7 @@
use std::{
net::{SocketAddr, ToSocketAddrs},
string::String,
time::Duration,
time::Duration, collections::HashSet,
};
use zebra_chain::Network;
@ -21,11 +21,11 @@ pub struct Config {
/// A list of initial peers for the peerset when operating on
/// mainnet.
pub initial_mainnet_peers: Vec<String>,
pub initial_mainnet_peers: HashSet<String>,
/// A list of initial peers for the peerset when operating on
/// testnet.
pub initial_testnet_peers: Vec<String>,
pub initial_testnet_peers: HashSet<String>,
/// The outgoing request buffer size for the peer set.
pub peerset_request_buffer_size: usize,
@ -49,16 +49,16 @@ pub struct Config {
}
impl Config {
fn parse_peers<S: ToSocketAddrs>(peers: Vec<S>) -> Vec<SocketAddr> {
fn parse_peers<S: ToSocketAddrs>(peers: HashSet<S>) -> HashSet<SocketAddr> {
peers
.iter()
.flat_map(|s| s.to_socket_addrs())
.flatten()
.collect::<Vec<SocketAddr>>()
.collect()
}
/// Get the initial seed peers based on the configured network.
pub fn initial_peers(&self) -> Vec<SocketAddr> {
pub fn initial_peers(&self) -> HashSet<SocketAddr> {
match self.network {
Network::Mainnet => Config::parse_peers(self.initial_mainnet_peers.clone()),
Network::Testnet => Config::parse_peers(self.initial_testnet_peers.clone()),

View File

@ -148,7 +148,7 @@ where
/// the results over `tx`.
#[instrument(skip(initial_peers, connector, tx))]
async fn add_initial_peers<S>(
initial_peers: Vec<SocketAddr>,
initial_peers: std::collections::HashSet<SocketAddr>,
connector: S,
mut tx: mpsc::Sender<PeerChange>,
) where

View File

@ -163,6 +163,7 @@ where
self.ready_services.insert(key, svc);
}
Poll::Ready(Some(Err((key, UnreadyError::Canceled)))) => {
trace!(?key, "service was canceled");
debug_assert!(!self.cancel_handles.contains_key(&key))
}
Poll::Ready(Some(Err((key, UnreadyError::Inner(e))))) => {

View File

@ -108,7 +108,9 @@ impl Application for ZebradApp {
/// Get logging configuration from command-line options
fn tracing_config(&self, command: &EntryPoint<ZebradCmd>) -> trace::Config {
if command.verbose {
if let Ok(env) = std::env::var("ZEBRAD_LOG") {
trace::Config::from(env)
} else if command.verbose {
trace::Config::verbose()
} else {
trace::Config::default()

View File

@ -60,7 +60,7 @@ impl ConnectCmd {
// Use a different listen addr so that we don't conflict with another local node.
config.listen_addr = "127.0.0.1:38233".parse().unwrap();
// Connect only to the specified peer.
config.initial_mainnet_peers = vec![self.addr.to_string()];
config.initial_mainnet_peers.insert(self.addr.to_string());
let (mut peer_set, _address_book) = zebra_network::init(config, node).await;
let mut retry_peer_set =