diff --git a/rpc/src/tests/mod.rs b/rpc/src/tests/mod.rs index ddbeb27508..eb8e860651 100644 --- a/rpc/src/tests/mod.rs +++ b/rpc/src/tests/mod.rs @@ -224,3 +224,11 @@ fn always_success_transaction() -> TransactionView { fn setup(consensus: Consensus) -> RpcTestSuite { setup_rpc_test_suite(20, Some(consensus)) } + +// Shut down background tasks to release `Arc` and allow temp dir cleanup. +// It's should be safe in this test suite. +impl Drop for RpcTestSuite { + fn drop(&mut self) { + ckb_stop_handler::broadcast_exit_signals(); + } +} diff --git a/shared/src/types/header_map/mod.rs b/shared/src/types/header_map/mod.rs index f820fd967b..84fc6155c8 100644 --- a/shared/src/types/header_map/mod.rs +++ b/shared/src/types/header_map/mod.rs @@ -1,5 +1,5 @@ use ckb_async_runtime::Handle; -use ckb_logger::info; +use ckb_logger::{debug, info}; use ckb_stop_handler::{CancellationToken, new_tokio_exit_rx}; use ckb_types::packed::Byte32; use std::sync::Arc; @@ -51,7 +51,7 @@ impl HeaderMap { } let size_limit = memory_limit / ITEM_BYTES_SIZE; let inner = Arc::new(HeaderMapKernel::new(tmpdir, size_limit, ibd_finished)); - let map = Arc::clone(&inner); + let map_weak = Arc::downgrade(&inner); let stop_rx: CancellationToken = new_tokio_exit_rx(); async_handle.spawn(async move { @@ -60,7 +60,12 @@ impl HeaderMap { loop { tokio::select! { _ = interval.tick() => { - map.limit_memory(); + if let Some(map) = map_weak.upgrade() { + map.limit_memory(); + } else { + debug!("HeaderMap inner was dropped, exiting background task"); + break; + } } _ = stop_rx.cancelled() => { info!("HeaderMap limit_memory received exit signal, exit now"); diff --git a/test/src/lib.rs b/test/src/lib.rs index 10809267a1..57752d6f8c 100644 --- a/test/src/lib.rs +++ b/test/src/lib.rs @@ -216,6 +216,9 @@ pub fn main_test() { spec_name, seconds, node_log_paths, + // node_paths is ignored here to let TempPathBuf handle + // automatic directory cleanup when this scope ends. + .. } => { test_results.push(TestResult { spec_name: spec_name.clone(), @@ -239,6 +242,8 @@ pub fn main_test() { spec_name, seconds, node_log_paths, + // same as above + .. } => { test_results.push(TestResult { spec_name: spec_name.clone(), diff --git a/test/src/net.rs b/test/src/net.rs index 8e5cc3c964..f25ad3230d 100644 --- a/test/src/net.rs +++ b/test/src/net.rs @@ -1,5 +1,5 @@ use crate::Node; -use crate::utils::{find_available_port, message_name, temp_path, wait_until}; +use crate::utils::{TempPathBuf, find_available_port, message_name, wait_until}; use ckb_app_config::NetworkConfig; use ckb_async_runtime::{Runtime, new_global_runtime}; use ckb_chain_spec::consensus::Consensus; @@ -12,7 +12,6 @@ use ckb_network::{ }; use ckb_util::Mutex; use std::collections::HashMap; -use std::path::PathBuf; use std::sync::Arc; use std::time::Duration; @@ -20,12 +19,15 @@ pub type NetMessage = (PeerIndex, ProtocolId, Bytes); pub struct Net { p2p_port: u16, - working_dir: PathBuf, protocols: Vec, controller: NetworkController, register_rx: Receiver<(String, PeerIndex, Receiver)>, receivers: HashMap)>, + // _async_runtime MUST be declared before _working_dir + // to ensure the runtime stops and releases file handles + // before the TempDir attempts to clean up. _async_runtime: Runtime, + _working_dir: TempPathBuf, } impl Net { @@ -35,13 +37,13 @@ impl Net { "Net cannot initialize with empty protocols" ); let p2p_port = find_available_port(); - let working_dir = temp_path(spec_name, "net"); + let working_dir = TempPathBuf::new(spec_name, "net"); let p2p_listen = format!("/ip4/127.0.0.1/tcp/{p2p_port}").parse().unwrap(); let network_state = Arc::new( NetworkState::from_config(NetworkConfig { listen_addresses: vec![p2p_listen], - path: (&working_dir).into(), + path: working_dir.to_path_buf(), max_peers: 128, max_outbound_peers: 128, discovery_local_address: true, @@ -79,19 +81,15 @@ impl Net { .unwrap(); Self { p2p_port, - working_dir, protocols, controller, register_rx, receivers: Default::default(), _async_runtime: async_runtime, + _working_dir: working_dir, } } - pub fn working_dir(&self) -> &PathBuf { - &self.working_dir - } - pub fn p2p_listen(&self) -> String { format!("/ip4/127.0.0.1/tcp/{}", self.p2p_port) } diff --git a/test/src/node.rs b/test/src/node.rs index 79b6d9f843..04276c6b20 100644 --- a/test/src/node.rs +++ b/test/src/node.rs @@ -1,6 +1,6 @@ use crate::global::binary; use crate::rpc::RpcClient; -use crate::utils::{find_available_port, temp_path, wait_until}; +use crate::utils::{TempPathBuf, find_available_port, wait_until}; use crate::{SYSTEM_CELL_ALWAYS_FAILURE_INDEX, SYSTEM_CELL_ALWAYS_SUCCESS_INDEX}; use ckb_app_config::{AppConfig, CKBAppConfig, ExitCode}; use ckb_chain_spec::ChainSpec; @@ -69,7 +69,7 @@ pub struct Node { pub struct InnerNode { spec_node_name: String, - working_dir: PathBuf, + working_dir: TempPathBuf, consensus: Consensus, p2p_listen: String, rpc_client: RpcClient, @@ -81,7 +81,7 @@ pub struct InnerNode { impl Node { pub fn new(spec_name: &str, node_name: &str) -> Self { - let working_dir = temp_path(spec_name, node_name); + let working_dir = TempPathBuf::new(spec_name, node_name); // Copy node template into node's working directory let cells_dir = working_dir.join("specs").join("cells"); @@ -126,7 +126,10 @@ impl Node { modifier(&mut app_config); fs::write(&app_config_path, toml::to_string(&app_config).unwrap()).unwrap(); - *self = Self::init(self.working_dir(), self.inner.spec_node_name.clone()); + *self = Self::init( + self.inner.working_dir.clone(), + self.inner.spec_node_name.clone(), + ); } pub fn modify_chain_spec(&mut self, modifier: M) @@ -139,11 +142,14 @@ impl Node { modifier(&mut chain_spec); fs::write(&chain_spec_path, toml::to_string(&chain_spec).unwrap()).unwrap(); - *self = Self::init(self.working_dir(), self.inner.spec_node_name.clone()); + *self = Self::init( + self.inner.working_dir.clone(), + self.inner.spec_node_name.clone(), + ); } // Initialize Node instance based on working directory - fn init(working_dir: PathBuf, spec_node_name: String) -> Self { + fn init(working_dir: TempPathBuf, spec_node_name: String) -> Self { let app_config = { let app_config_path = working_dir.join("ckb.toml"); let toml = fs::read(app_config_path).unwrap(); @@ -189,6 +195,10 @@ impl Node { } pub fn working_dir(&self) -> PathBuf { + self.inner.working_dir.to_path_buf() + } + + pub fn owned_working_dir(&self) -> TempPathBuf { self.inner.working_dir.clone() } diff --git a/test/src/utils.rs b/test/src/utils.rs index 4434922564..c76101b970 100644 --- a/test/src/utils.rs +++ b/test/src/utils.rs @@ -16,9 +16,12 @@ use std::convert::Into; use std::env; use std::fs::read_to_string; use std::net::{Ipv4Addr, SocketAddrV4, TcpListener}; -use std::path::PathBuf; +use std::ops::Deref; +use std::path::{Path, PathBuf}; +use std::sync::Arc; use std::thread; use std::time::{Duration, Instant, SystemTime, UNIX_EPOCH}; +use tempfile::TempDir; pub const FLAG_SINCE_RELATIVE: u64 = 0b1000_0000_0000_0000_0000_0000_0000_0000_0000_0000_0000_0000_0000_0000_0000_0000; @@ -199,23 +202,47 @@ pub fn assert_send_transaction_ok(node: &Node, transaction: &TransactionView) { assert!(result.is_ok(), "result: {:?}", result.unwrap_err()); } -/// Return a random path located on temp_dir -/// -/// We use `tempdir` only for generating a random path, and expect the corresponding directory -/// that `tempdir` creates be deleted when go out of this function. -pub fn temp_path(case_name: &str, suffix: &str) -> PathBuf { - let mut builder = tempfile::Builder::new(); - let prefix = ["ckb-it", case_name, suffix, ""].join("-"); - builder.prefix(&prefix); - let tempdir = if let Ok(val) = env::var("CKB_INTEGRATION_TEST_TMP") { - builder.tempdir_in(val) - } else { - builder.tempdir() +#[derive(Debug, Clone)] +pub struct TempPathBuf { + path: PathBuf, + _tempdir: Arc, +} + +impl TempPathBuf { + /// Return a random path located on temp_dir + pub fn new(case_name: &str, suffix: &str) -> Self { + let mut builder = tempfile::Builder::new(); + let prefix = ["ckb-it", case_name, suffix, ""].join("-"); + builder.prefix(&prefix); + let tempdir = if let Ok(val) = env::var("CKB_INTEGRATION_TEST_TMP") { + builder.tempdir_in(val) + } else { + builder.tempdir() + } + .expect("create tempdir failed"); + let path = tempdir.path().to_owned(); + Self { + path, + _tempdir: Arc::new(tempdir), + } + } + + pub fn path(&self) -> &Path { + &self.path + } +} + +impl Deref for TempPathBuf { + type Target = PathBuf; + fn deref(&self) -> &Self::Target { + &self.path + } +} + +impl AsRef for TempPathBuf { + fn as_ref(&self) -> &Path { + self.path.as_ref() } - .expect("create tempdir failed"); - let path = tempdir.path().to_owned(); - tempdir.close().expect("close tempdir failed"); - path } /// Generate new blocks and explode these cellbases into `n` live cells diff --git a/test/src/worker.rs b/test/src/worker.rs index 1d82fc38f6..a30347ce09 100644 --- a/test/src/worker.rs +++ b/test/src/worker.rs @@ -1,3 +1,4 @@ +use crate::utils::TempPathBuf; use crate::{Spec, utils::nodes_panicked}; use ckb_channel::{Receiver, Sender, unbounded}; use ckb_logger::{error, info}; @@ -24,18 +25,20 @@ pub enum Notify { Done { spec_name: String, seconds: u64, - node_paths: Vec, + node_paths: Vec, }, Error { spec_error: Box, spec_name: String, seconds: u64, node_log_paths: Vec, + node_paths: Vec, }, Panick { spec_name: String, seconds: u64, node_log_paths: Vec, + node_paths: Vec, }, Stop, } @@ -163,9 +166,10 @@ impl Worker { .unwrap(); let mut nodes = spec.before_run(); + // Used to extend the lifecycle of TempPathBuf let node_paths = nodes .iter() - .map(|node| node.working_dir()) + .map(|node| node.owned_working_dir()) .collect::>(); let node_log_paths = nodes.iter().map(|node| node.log_path()).collect::>(); let result = panic::catch_unwind(panic::AssertUnwindSafe(|| { @@ -184,6 +188,7 @@ impl Worker { spec_name: spec.name().to_string(), seconds: now.elapsed().as_secs(), node_log_paths, + node_paths, }) .unwrap(); } else if let Some(spec_error) = spec_error { @@ -193,6 +198,7 @@ impl Worker { spec_name: spec.name().to_string(), seconds: now.elapsed().as_secs(), node_log_paths, + node_paths, }) .unwrap(); } else {