Add test for circular references leading to NetworkGraph leaks

Due to two circular `Arc` references, after `stop`ping and
`drop`ping the `Node` instance the bulk of ldk-node's memory (in
the form of the `NetworkGraph`) would hang around. Here we add a
test for this in our integration tests, checking if the
`NetworkGraph` (as a proxy for other objects held in reference by
the `PeerManager`) hangs around after `Node`s are `drop`ped.

Author: TheBlueMatt

.github/workflows/rust.yml

@@ -80,11 +80,11 @@ jobs:
       - name: Test on Rust ${{ matrix.toolchain }}
         if: "matrix.platform != 'windows-latest'"
         run: |
-          RUSTFLAGS="--cfg no_download" cargo test
+          RUSTFLAGS="--cfg no_download --cfg cycle_tests" cargo test
       - name: Test with UniFFI support on Rust ${{ matrix.toolchain }}
         if: "matrix.platform != 'windows-latest' && matrix.build-uniffi"
         run: |
-          RUSTFLAGS="--cfg no_download" cargo test --features uniffi
+          RUSTFLAGS="--cfg no_download --cfg cycle_tests" cargo test --features uniffi
 
   doc:
     name: Documentation

.github/workflows/vss-integration.yml

@@ -45,4 +45,4 @@ jobs:
           cd ldk-node
           export TEST_VSS_BASE_URL="http://localhost:8080/vss"
           RUSTFLAGS="--cfg vss_test" cargo test io::vss_store
-          RUSTFLAGS="--cfg vss_test" cargo test --test integration_tests_vss
+          RUSTFLAGS="--cfg vss_test --cfg cycle_tests" cargo test --test integration_tests_vss

Cargo.toml

@@ -124,6 +124,7 @@ check-cfg = [
 	"cfg(tokio_unstable)",
 	"cfg(cln_test)",
 	"cfg(lnd_test)",
+	"cfg(cycle_tests)",
 ]
 
 [[bench]]

src/builder.rs

@@ -1684,6 +1684,18 @@ fn build_with_store_internal(
 
 	let pathfinding_scores_sync_url = pathfinding_scores_sync_config.map(|c| c.url.clone());
 
+	#[cfg(cycle_tests)]
+	let mut _leak_checker = crate::LeakChecker(Vec::new());
+	#[cfg(cycle_tests)]
+	{
+		use std::any::Any;
+		use std::sync::Weak;
+
+		_leak_checker.0.push(Arc::downgrade(&channel_manager) as Weak<dyn Any + Send + Sync>);
+		_leak_checker.0.push(Arc::downgrade(&network_graph) as Weak<dyn Any + Send + Sync>);
+		_leak_checker.0.push(Arc::downgrade(&wallet) as Weak<dyn Any + Send + Sync>);
+	}
+
 	Ok(Node {
 		runtime,
 		stop_sender,
@@ -1716,6 +1728,8 @@ fn build_with_store_internal(
 		om_mailbox,
 		async_payments_role,
 		hrn_resolver,
+		#[cfg(cycle_tests)]
+		_leak_checker,
 	})
 }
 

src/lib.rs

@@ -110,6 +110,8 @@ use std::default::Default;
 use std::net::ToSocketAddrs;
 use std::sync::{Arc, Mutex, RwLock};
 use std::time::{Duration, Instant, SystemTime, UNIX_EPOCH};
+#[cfg(cycle_tests)]
+use std::{any::Any, sync::Weak};
 
 pub use balance::{BalanceDetails, LightningBalance, PendingSweepBalance};
 use bitcoin::secp256k1::PublicKey;
@@ -173,6 +175,23 @@ use crate::scoring::setup_background_pathfinding_scores_sync;
 #[cfg(feature = "uniffi")]
 uniffi::include_scaffolding!("ldk_node");
 
+#[cfg(cycle_tests)]
+/// A list of [`Weak`]s which can be used to check that a [`Node`]'s inner fields are being
+/// properly released after the [`Node`] is dropped.
+pub struct LeakChecker(Vec<Weak<dyn Any + Send + Sync>>);
+
+#[cfg(cycle_tests)]
+impl LeakChecker {
+	/// Asserts that all the stored [`Weak`]s point to contents which have been freed.
+	///
+	/// This will (obviously) panic if the [`Node`] has not yet been dropped.
+	pub fn assert_no_leaks(&self) {
+		for weak in self.0.iter() {
+			assert_eq!(weak.strong_count(), 0);
+		}
+	}
+}
+
 /// The main interface object of LDK Node, wrapping the necessary LDK and BDK functionalities.
 ///
 /// Needs to be initialized and instantiated through [`Builder::build`].
@@ -208,6 +227,8 @@ pub struct Node {
 	om_mailbox: Option<Arc<OnionMessageMailbox>>,
 	async_payments_role: Option<AsyncPaymentsRole>,
 	hrn_resolver: Arc<HRNResolver>,
+	#[cfg(cycle_tests)]
+	_leak_checker: LeakChecker,
 }
 
 impl Node {