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

chore: fire shutdown signal on anvil node handle drop #8947

Merged
merged 7 commits into from
Sep 25, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
49 changes: 28 additions & 21 deletions crates/anvil/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -253,32 +253,32 @@ pub async fn try_spawn(mut config: NodeConfig) -> io::Result<(EthApi, NodeHandle

type IpcTask = JoinHandle<()>;

/// A handle to the spawned node and server tasks
/// A handle to the spawned node and server tasks.
///
/// This future will resolve if either the node or server task resolve/fail.
pub struct NodeHandle {
config: NodeConfig,
/// The address of the running rpc server
/// The address of the running rpc server.
addresses: Vec<SocketAddr>,
/// Join handle for the Node Service
/// Join handle for the Node Service.
pub node_service: JoinHandle<Result<(), NodeError>>,
/// Join handles (one per socket) for the Anvil server.
pub servers: Vec<JoinHandle<Result<(), NodeError>>>,
// The future that joins the ipc server, if any
/// The future that joins the ipc server, if any.
ipc_task: Option<IpcTask>,
/// A signal that fires the shutdown, fired on drop.
_signal: Option<Signal>,
/// A task manager that can be used to spawn additional tasks
/// A task manager that can be used to spawn additional tasks.
task_manager: TaskManager,
}

impl NodeHandle {
/// The [NodeConfig] the node was launched with
/// The [NodeConfig] the node was launched with.
pub fn config(&self) -> &NodeConfig {
&self.config
}

/// Prints the launch info
/// Prints the launch info.
pub(crate) fn print(&self, fork: Option<&ClientFork>) {
self.config.print(fork);
if !self.config.silent {
Expand All @@ -296,25 +296,25 @@ impl NodeHandle {
}
}

/// The address of the launched server
/// The address of the launched server.
///
/// **N.B.** this may not necessarily be the same `host + port` as configured in the
/// `NodeConfig`, if port was set to 0, then the OS auto picks an available port
/// `NodeConfig`, if port was set to 0, then the OS auto picks an available port.
pub fn socket_address(&self) -> &SocketAddr {
&self.addresses[0]
}

/// Returns the http endpoint
/// Returns the http endpoint.
pub fn http_endpoint(&self) -> String {
format!("http://{}", self.socket_address())
}

/// Returns the websocket endpoint
/// Returns the websocket endpoint.
pub fn ws_endpoint(&self) -> String {
format!("ws://{}", self.socket_address())
}

/// Returns the path of the launched ipc server, if any
/// Returns the path of the launched ipc server, if any.
pub fn ipc_path(&self) -> Option<String> {
self.config.get_ipc_path()
}
Expand All @@ -334,44 +334,51 @@ impl NodeHandle {
ProviderBuilder::new(&self.config.get_ipc_path()?).build().ok()
}

/// Signer accounts that can sign messages/transactions from the EVM node
/// Signer accounts that can sign messages/transactions from the EVM node.
pub fn dev_accounts(&self) -> impl Iterator<Item = Address> + '_ {
self.config.signer_accounts.iter().map(|wallet| wallet.address())
}

/// Signer accounts that can sign messages/transactions from the EVM node
/// Signer accounts that can sign messages/transactions from the EVM node.
pub fn dev_wallets(&self) -> impl Iterator<Item = PrivateKeySigner> + '_ {
self.config.signer_accounts.iter().cloned()
}

/// Accounts that will be initialised with `genesis_balance` in the genesis block
/// Accounts that will be initialised with `genesis_balance` in the genesis block.
pub fn genesis_accounts(&self) -> impl Iterator<Item = Address> + '_ {
self.config.genesis_accounts.iter().map(|w| w.address())
}

/// Native token balance of every genesis account in the genesis block
/// Native token balance of every genesis account in the genesis block.
pub fn genesis_balance(&self) -> U256 {
self.config.genesis_balance
}

/// Default gas price for all txs
/// Default gas price for all txs.
pub fn gas_price(&self) -> u128 {
self.config.get_gas_price()
}

/// Returns the shutdown signal
/// Returns the shutdown signal.
pub fn shutdown_signal(&self) -> &Option<Signal> {
&self._signal
}

/// Returns mutable access to the shutdown signal
/// Returns mutable access to the shutdown signal.
///
/// This can be used to extract the Signal
/// This can be used to extract the Signal.
pub fn shutdown_signal_mut(&mut self) -> &mut Option<Signal> {
&mut self._signal
}

/// Returns the task manager that can be used to spawn new tasks
/// Fires shutdown signal.
///
/// Called by long-running tests to make sure anvil instance is terminated.
pub fn fire_shutdown_signal(&mut self) {
DaniPopes marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this go into an impl Drop?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, will do and make some tests

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hm, weird, I added in drop

+impl Drop for NodeHandle {
+    fn drop(&mut self) {
+        if let Some(signal) = self._signal.take() {
+            signal.fire().unwrap()
+        }
+    }
+}

and then got again a failure like

failed to spawn node: Os { code: 98, kind: AddrInUse, message: "Address already in use" }
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

failures:
    proof::test_account_proof

going to make some more tests to see if consistently happens

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added in 277d62c ptal

self._signal.take().map(|signal| signal.fire());
DaniPopes marked this conversation as resolved.
Show resolved Hide resolved
}

/// Returns the task manager that can be used to spawn new tasks.
///
/// ```
/// use anvil::NodeHandle;
Expand Down
4 changes: 1 addition & 3 deletions crates/anvil/tests/it/anvil_api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -793,7 +793,5 @@ async fn test_reorg() {
.await;
assert!(res.is_err());

if let Some(signal) = handle.shutdown_signal_mut().take() {
signal.fire().unwrap();
}
handle.fire_shutdown_signal();
}
4 changes: 1 addition & 3 deletions crates/anvil/tests/it/gas.rs
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,5 @@ async fn test_can_use_fee_history() {
assert_eq!(latest_fee_history_fee, next_base_fee);
}

if let Some(signal) = handle.shutdown_signal_mut().take() {
signal.fire().unwrap();
}
handle.fire_shutdown_signal();
}
4 changes: 1 addition & 3 deletions crates/anvil/tests/it/proof.rs
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,5 @@ async fn can_get_random_account_proofs() {
.unwrap_or_else(|_| panic!("Failed to get proof for {acc:?}"));
}

if let Some(signal) = handle.shutdown_signal_mut().take() {
signal.fire().unwrap();
}
handle.fire_shutdown_signal();
}
4 changes: 1 addition & 3 deletions crates/anvil/tests/it/traces.rs
Original file line number Diff line number Diff line change
Expand Up @@ -860,7 +860,5 @@ async fn test_trace_filter() {
let traces = api.trace_filter(tracer).await.unwrap();
assert_eq!(traces.len(), 5);

if let Some(signal) = handle.shutdown_signal_mut().take() {
signal.fire().unwrap();
}
handle.fire_shutdown_signal();
}
Loading