Skip to content

Commit 8065a9b

Browse files
committed
Handle thread join errors uniformly with helper
1 parent bc58a1e commit 8065a9b

File tree

5 files changed

+132
-13
lines changed

5 files changed

+132
-13
lines changed

Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

stackslib/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,7 @@ mutants = "0.0.3"
9393
rlimit = "0.10.2"
9494
chrono = "0.4.19"
9595
tempfile = "3.3"
96+
proptest = { version = "1.6.0", default-features = false, features = ["std"] }
9697

9798
[features]
9899
default = []

stackslib/src/burnchains/burnchain.rs

Lines changed: 31 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -450,6 +450,23 @@ impl BurnchainBlock {
450450
}
451451

452452
impl Burnchain {
453+
pub fn handle_thread_join<T>(
454+
handle: std::thread::JoinHandle<Result<T, burnchain_error>>,
455+
name: &str,
456+
) -> Result<T, burnchain_error> {
457+
match handle.join() {
458+
Ok(Ok(val)) => Ok(val),
459+
Ok(Err(e)) => {
460+
warn!("{} thread error: {:?}", name, e);
461+
Err(e)
462+
}
463+
Err(_) => {
464+
error!("{} thread panicked", name);
465+
Err(burnchain_error::ThreadChannelError)
466+
}
467+
}
468+
}
469+
453470
pub fn new(
454471
working_dir: &str,
455472
chain_name: &str,
@@ -1783,20 +1800,21 @@ impl Burnchain {
17831800
}
17841801

17851802
// join up
1786-
let _ = download_thread.join().unwrap();
1787-
let _ = parse_thread.join().unwrap();
1788-
let block_header = match db_thread.join().unwrap() {
1789-
Ok(x) => x,
1790-
Err(e) => {
1791-
warn!("Failed to join burnchain download thread: {:?}", &e);
1792-
if let burnchain_error::CoordinatorClosed = e {
1793-
return Err(burnchain_error::CoordinatorClosed);
1794-
} else {
1795-
return Err(burnchain_error::TrySyncAgain);
1796-
}
1797-
}
1798-
};
1803+
let download_result = Self::handle_thread_join(download_thread, "download");
1804+
let parse_result = Self::handle_thread_join(parse_thread, "parse");
1805+
let db_result = Self::handle_thread_join(db_thread, "db");
1806+
1807+
if let Err(e) = download_result {
1808+
warn!("Download thread failed: {:?}", e);
1809+
return Err(e);
1810+
}
1811+
1812+
if let Err(e) = parse_result {
1813+
warn!("Parse thread failed: {:?}", e);
1814+
return Err(e);
1815+
}
17991816

1817+
let block_header = db_result?;
18001818
if block_header.block_height < end_block {
18011819
warn!(
18021820
"Try synchronizing the burn chain again: final snapshot {} < {}",

stackslib/src/burnchains/tests/mod.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
pub mod affirmation;
1818
pub mod burnchain;
1919
pub mod db;
20+
pub mod thread_join;
2021

2122
use std::collections::HashMap;
2223

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,98 @@
1+
// Copyright (C) 2025 Stacks Open Internet Foundation
2+
//
3+
// This program is free software: you can redistribute it and/or modify
4+
// it under the terms of the GNU General Public License as published by
5+
// the Free Software Foundation, either version 3 of the License, or
6+
// (at your option) any later version.
7+
//
8+
// This program is distributed in the hope that it will be useful,
9+
// but WITHOUT ANY WARRANTY; without even the implied warranty of
10+
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
11+
// GNU General Public License for more details.
12+
//
13+
// You should have received a copy of the GNU General Public License
14+
// along with this program. If not, see <http://www.gnu.org/licenses/>.
15+
16+
use std::time::Duration;
17+
use std::{panic, thread};
18+
19+
use proptest::prelude::*;
20+
21+
use crate::burnchains::bitcoin::Error as bitcoin_error;
22+
use crate::burnchains::{Burnchain, Error as burnchain_error};
23+
24+
#[test]
25+
fn join_success() {
26+
proptest!(|(v in any::<u32>())| {
27+
let h = thread::spawn(move || Ok(v));
28+
let r = Burnchain::handle_thread_join::<u32>(h, "test");
29+
prop_assert!(r.is_ok());
30+
prop_assert_eq!(r.unwrap(), v);
31+
});
32+
}
33+
34+
#[test]
35+
fn join_with_name() {
36+
proptest!(|(s in "[a-zA-Z0-9_-]{1,20}")| {
37+
let h = thread::spawn(|| Ok(42));
38+
let r = Burnchain::handle_thread_join::<u32>(h, &s);
39+
prop_assert!(r.is_ok());
40+
prop_assert_eq!(r.unwrap(), 42);
41+
});
42+
}
43+
44+
#[test]
45+
fn join_download_error() {
46+
proptest!(|(x in any::<u32>())| {
47+
let h = thread::spawn(move || {
48+
let _ = x;
49+
Err(burnchain_error::DownloadError(
50+
bitcoin_error::ConnectionError))
51+
});
52+
let r = Burnchain::handle_thread_join::<u32>(h, "test");
53+
prop_assert!(r.is_err());
54+
match r {
55+
Err(burnchain_error::DownloadError(_)) => {}
56+
_ => return Err(TestCaseError::fail("Expected DownloadError")),
57+
}
58+
});
59+
}
60+
61+
#[test]
62+
fn join_parse_error() {
63+
let h = thread::spawn(move || Err(burnchain_error::ParseError));
64+
let r = Burnchain::handle_thread_join::<u32>(h, "test");
65+
assert!(r.is_err());
66+
match r {
67+
Err(burnchain_error::ParseError) => {}
68+
_ => panic!("Expected ParseError"),
69+
}
70+
}
71+
72+
#[test]
73+
fn join_delay() {
74+
proptest!(|(d in 10u64..100, v in any::<u32>())| {
75+
let h = thread::spawn(move || {
76+
thread::sleep(Duration::from_millis(d));
77+
Ok(v)
78+
});
79+
let r = Burnchain::handle_thread_join::<u32>(h, "test");
80+
prop_assert!(r.is_ok());
81+
prop_assert_eq!(r.unwrap(), v);
82+
});
83+
}
84+
85+
#[test]
86+
fn join_panics() {
87+
let h = thread::spawn(|| {
88+
panic!("boom");
89+
#[allow(unreachable_code)]
90+
Ok(42)
91+
});
92+
let r = Burnchain::handle_thread_join::<u32>(h, "test");
93+
assert!(r.is_err());
94+
match r {
95+
Err(burnchain_error::ThreadChannelError) => {}
96+
_ => panic!("Expected ThreadChannelError"),
97+
}
98+
}

0 commit comments

Comments
 (0)