Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

merge queue: embarking main (c885de4) and [#7219 + #7218 + #7241 + #7226 + #7222 + #7238 + #7229] together #7243

Closed
wants to merge 34 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
2812dba
Remove duplicate asserts
upbqdn Jul 15, 2023
5c061d9
Remove workarounds for inserting trees into NFS
upbqdn Jul 15, 2023
7868e71
Use correct height for constructing new chain
upbqdn Jul 15, 2023
60a287f
Don't push the 0th block into a chain
upbqdn Jul 15, 2023
0fd231c
Don't commit two blocks at the same height
upbqdn Jul 15, 2023
b4bf35c
Fix typo
teor2345 Jul 16, 2023
f6b8176
Add an async-error feature and an initial module structure
teor2345 Jul 11, 2023
ee78f36
Implement checking for panics in OS threads and async tasks
teor2345 Jul 12, 2023
74ce0d3
Implement waiting for panics in OS threads and async tasks
teor2345 Jul 13, 2023
a59b04e
Add a TODO to simplify some state request error handling
teor2345 Jul 13, 2023
35cdec5
Use the new panic-checking methods in zebra-state
teor2345 Jul 13, 2023
4b59bdd
Use new panic-checking methods in zebra-network
teor2345 Jul 17, 2023
9ddf377
fixup! Implement waiting for panics in OS threads and async tasks
teor2345 Jul 17, 2023
93ecfc5
Replace existing async code with generic panic-checking methods
teor2345 Jul 17, 2023
7c31404
Simplify trait to a single method
teor2345 Jul 17, 2023
22b0c5b
Move thread panic code into generic trait impls
teor2345 Jul 17, 2023
b54da8b
Generate chains with at least two blocks
upbqdn Jul 17, 2023
6c2eaa4
Merge branch 'fix-height-in-tests' of github.com:ZcashFoundation/zebr…
upbqdn Jul 17, 2023
389a2ba
build(deps): bump the cli group with 1 update
dependabot[bot] Jul 17, 2023
6910116
Simplify option handling
teor2345 Jul 17, 2023
0e65540
Fix comment
teor2345 Jul 17, 2023
b6a4f63
Add missing track_caller
teor2345 Jul 17, 2023
e6687b2
build(deps): bump insta from 1.30.0 to 1.31.0
dependabot[bot] Jul 17, 2023
6c3cf02
build(deps): bump the crypto group with 1 update
dependabot[bot] Jul 17, 2023
d4896be
Bumps sha2/secp256k1, updates deny.toml
arya2 Jul 17, 2023
deea124
removes unused import, updates method calls
arya2 Jul 17, 2023
a358127
Change dependabot.yml Actions schedule and fix groups
teor2345 Jul 18, 2023
2e248b8
Merge of #7219
mergify[bot] Jul 18, 2023
859acaf
Merge of #7218
mergify[bot] Jul 18, 2023
772aa04
Merge of #7241
mergify[bot] Jul 18, 2023
220277a
Merge of #7226
mergify[bot] Jul 18, 2023
3c3e64a
Merge of #7222
mergify[bot] Jul 18, 2023
4da828f
Merge of #7238
mergify[bot] Jul 18, 2023
e504245
Merge of #7229
mergify[bot] Jul 18, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Simplify trait to a single method
  • Loading branch information
teor2345 committed Jul 17, 2023
commit 7c314040ce253963b816ca5fb439a5f7e29b4d93
27 changes: 3 additions & 24 deletions zebra-chain/src/diagnostic/task.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,15 +24,9 @@ pub trait CheckForPanics {
}

/// A trait that waits for a task to finish, then handles panics and cancellations.
pub trait WaitForTermination {
pub trait WaitForPanics {
/// The underlying task output, after removing panics and unwrapping termination results.
type UnwrappedOutput;

/// The underlying task output, after removing panics,
/// and replacing expected terminations with a default value.
///
/// This is typically a `Result` containing the underlying task output type.
type ResultOutput;
type Output;

/// Waits for `self` to finish, then check if its output is:
/// - a panic payload: resume that panic,
Expand All @@ -49,20 +43,5 @@ pub trait WaitForTermination {
///
/// If `self` contains an expected termination, and we're shutting down anyway.
#[track_caller]
fn panic_on_early_termination(self) -> Self::UnwrappedOutput;

/// Waits for `self` to finish, then check if its output is:
/// - a panic payload: resume that panic,
/// - an unexpected termination: return that error,
/// - an expected termination: return the provided `expected_termination_value`.
///
/// Otherwise, returns the task return value of `self`.
///
/// # Panics
///
/// If `self` contains a panic payload.
#[track_caller]
fn error_on_early_termination<D>(self, expected_termination_value: D) -> Self::ResultOutput
where
D: Into<Self::ResultOutput> + Send + 'static;
fn wait_for_panics(self) -> Self::Output;
}
52 changes: 4 additions & 48 deletions zebra-chain/src/diagnostic/task/future.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use tokio::task::{JoinError, JoinHandle};

use crate::shutdown::is_shutting_down;

use super::{CheckForPanics, WaitForTermination};
use super::{CheckForPanics, WaitForPanics};

/// This is the return type of the [`JoinHandle`] future.
impl<T> CheckForPanics for Result<T, JoinError> {
Expand Down Expand Up @@ -59,13 +59,11 @@ impl CheckForPanics for JoinError {
}
}

impl<T> WaitForTermination for JoinHandle<T>
impl<T> WaitForPanics for JoinHandle<T>
where
T: Send + 'static,
{
type UnwrappedOutput = BoxFuture<'static, T>;

type ResultOutput = BoxFuture<'static, Result<T, JoinError>>;
type Output = BoxFuture<'static, T>;

/// Returns a future which waits for `self` to finish, then checks if its output is:
/// - a panic payload: resume that panic,
Expand All @@ -83,7 +81,7 @@ where
/// If `self` contains an expected termination, and we're shutting down anyway.
/// Futures hang by returning `Pending` and not setting a waker, so this uses minimal resources.
#[track_caller]
fn panic_on_early_termination(self) -> Self::UnwrappedOutput {
fn wait_for_panics(self) -> Self::Output {
async move {
match self.await.check_for_panics() {
Ok(task_output) => task_output,
Expand All @@ -92,46 +90,4 @@ where
}
.boxed()
}

/// Returns a future which waits for `self` to finish, then checks if its output is:
/// - a panic payload: resume that panic,
/// - an unexpected termination: return that error,
/// - an expected termination: return the provided `expected_termination_value`.
///
/// Otherwise, returns the task return value of `self`.
///
/// # Panics
///
/// If `self` contains a panic payload.
#[track_caller]
fn error_on_early_termination<D>(self, expected_termination_value: D) -> Self::ResultOutput
where
D: Into<Self::ResultOutput> + Send + 'static,
{
async move {
let task_result = self.await;

let Err(join_error) = task_result else {
// Always `Ok`
return task_result;
};

match join_error.try_into_panic() {
Ok(panic_payload) => panic::resume_unwind(panic_payload),

// Substitute the default value
Err(task_cancelled) if is_shutting_down() => {
debug!(
?task_cancelled,
"ignoring cancelled task because Zebra is shutting down"
);

expected_termination_value.into().await
}

Err(task_cancelled) => Err(task_cancelled),
}
}
.boxed()
}
}
27 changes: 9 additions & 18 deletions zebra-chain/src/diagnostic/task/thread.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,13 @@
//! - task handles
//! - errors and panics

use std::{panic, thread};
use std::{
panic,
sync::Arc,
thread::{self, JoinHandle},
};

use super::{CheckForPanics, WaitForTermination};
use super::{CheckForPanics, WaitForPanics};

impl<T> CheckForPanics for thread::Result<T> {
type Output = T;
Expand All @@ -24,25 +28,12 @@ impl<T> CheckForPanics for thread::Result<T> {
}
}

impl<T> WaitForTermination for thread::JoinHandle<T> {
type UnwrappedOutput = T;

type ResultOutput = thread::Result<T>;
impl<T> WaitForPanics for JoinHandle<T> {
type Output = T;

/// Waits for the thread to finish, then panics if the thread panicked.
#[track_caller]
fn panic_on_early_termination(self) -> Self::UnwrappedOutput {
fn wait_for_panics(self) -> Self::Output {
self.join().check_for_panics()
}

/// Waits for the thread to finish, then panics if the thread panicked.
///
/// Threads can't be cancelled except by using a panic, so the returned result is always `Ok`.
#[track_caller]
fn error_on_early_termination<D>(self, _expected_termination_value: D) -> Self::ResultOutput
where
D: Into<Self::ResultOutput> + 'static,
{
Ok(self.panic_on_early_termination())
}
}
6 changes: 3 additions & 3 deletions zebra-network/src/peer_set/candidate_set.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use tokio::time::{sleep_until, timeout, Instant};
use tower::{Service, ServiceExt};
use tracing::Span;

use zebra_chain::{diagnostic::task::WaitForTermination, serialization::DateTime32};
use zebra_chain::{diagnostic::task::WaitForPanics, serialization::DateTime32};

use crate::{
constants, meta_addr::MetaAddrChange, peer_set::set::MorePeers, types::MetaAddr, AddressBook,
Expand Down Expand Up @@ -348,7 +348,7 @@ where
tokio::task::spawn_blocking(move || {
span.in_scope(|| address_book.lock().unwrap().extend(addrs))
})
.panic_on_early_termination()
.wait_for_panics()
.await
}

Expand Down Expand Up @@ -403,7 +403,7 @@ where
// Correctness: Spawn address book accesses on a blocking thread, to avoid deadlocks (see #1976).
let span = Span::current();
let next_peer = tokio::task::spawn_blocking(move || span.in_scope(next_peer))
.panic_on_early_termination()
.wait_for_panics()
.await?;

// Security: rate-limit new outbound peer connections
Expand Down
14 changes: 7 additions & 7 deletions zebra-network/src/peer_set/initialize.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ use tower::{
use tracing::Span;
use tracing_futures::Instrument;

use zebra_chain::{chain_tip::ChainTip, diagnostic::task::WaitForTermination};
use zebra_chain::{chain_tip::ChainTip, diagnostic::task::WaitForPanics};

use crate::{
address_book_updater::AddressBookUpdater,
Expand Down Expand Up @@ -206,7 +206,7 @@ where

// Wait for the initial seed peer count
let mut active_outbound_connections = initial_peers_join
.panic_on_early_termination()
.wait_for_panics()
.await
.expect("unexpected error connecting to initial peers");
let active_initial_peer_count = active_outbound_connections.update_count();
Expand Down Expand Up @@ -343,7 +343,7 @@ where
}
.in_current_span(),
)
.panic_on_early_termination()
.wait_for_panics()
})
.collect();

Expand Down Expand Up @@ -615,7 +615,7 @@ where
peerset_tx.clone(),
)
.await?
.panic_on_early_termination();
.wait_for_panics();

handshakes.push(handshake_task);

Expand Down Expand Up @@ -903,7 +903,7 @@ where
}
.in_current_span(),
)
.panic_on_early_termination();
.wait_for_panics();

handshakes.push(handshake_or_crawl_handle);
}
Expand All @@ -924,7 +924,7 @@ where
}
.in_current_span(),
)
.panic_on_early_termination();
.wait_for_panics();

handshakes.push(crawl_handle);
}
Expand Down Expand Up @@ -1100,7 +1100,7 @@ async fn report_failed(address_book: Arc<std::sync::Mutex<AddressBook>>, addr: M
let updated_addr = tokio::task::spawn_blocking(move || {
span.in_scope(|| address_book.lock().unwrap().update(addr))
})
.panic_on_early_termination()
.wait_for_panics()
.await;

assert_eq!(
Expand Down
Loading