Merge pull request #1964 from TheBlueMatt/2023-01-no-debug-panics

Use test/_test_utils to enable single-threaded debug assertions

diff --git a/fuzz/Cargo.toml b/fuzz/Cargo.toml
index b0b6910ae2f035a1eedc3f956ba9e1b9ca5b8cb0..5daebb3506681503e197ba92656eb1ddd4eb4c06 100644 (file)
--- a/fuzz/Cargo.toml
+++ b/fuzz/Cargo.toml
@@ -18,7 +18,7 @@ libfuzzer_fuzz = ["libfuzzer-sys"]
 stdin_fuzz = []
 
 [dependencies]
-lightning = { path = "../lightning", features = ["regex", "hashbrown"] }
+lightning = { path = "../lightning", features = ["regex", "hashbrown", "_test_utils"] }
 lightning-rapid-gossip-sync = { path = "../lightning-rapid-gossip-sync" }
 bitcoin = { version = "0.29.0", features = ["secp-lowmemory"] }
 hex = "0.3"

diff --git a/lightning-block-sync/Cargo.toml b/lightning-block-sync/Cargo.toml
index 8b185ff3babcb4e31f76bfb47260cf088c9ee11c..c605f5ef3a2ef4b3201cec0e7a979b1bda7b9ecc 100644 (file)
--- a/lightning-block-sync/Cargo.toml
+++ b/lightning-block-sync/Cargo.toml
@@ -27,4 +27,5 @@ serde_json = { version = "1.0", optional = true }
 chunked_transfer = { version = "1.4", optional = true }
 
 [dev-dependencies]
+lightning = { version = "0.0.113", path = "../lightning", features = ["_test_utils"] }
 tokio = { version = "~1.14", features = [ "macros", "rt" ] }

diff --git a/lightning-net-tokio/Cargo.toml b/lightning-net-tokio/Cargo.toml
index 07f9f313307c0cd493ab2e2e54b00489aba283c5..8a518638c5f789de307d1fa47cdc3f37422c61ac 100644 (file)
--- a/lightning-net-tokio/Cargo.toml
+++ b/lightning-net-tokio/Cargo.toml
@@ -21,3 +21,4 @@ tokio = { version = "1.0", features = [ "io-util", "macros", "rt", "sync", "net"
 
 [dev-dependencies]
 tokio = { version = "~1.14", features = [ "io-util", "macros", "rt", "rt-multi-thread", "sync", "net", "time" ] }
+lightning = { version = "0.0.113", path = "../lightning", features = ["_test_utils"] }

diff --git a/lightning-net-tokio/src/lib.rs b/lightning-net-tokio/src/lib.rs
index 45fe9b12f61b17a7b078805de1e6344efa91fbf3..38b09a2886c2b825acec3cf4f4f741626f184139 100644 (file)
--- a/lightning-net-tokio/src/lib.rs
+++ b/lightning-net-tokio/src/lib.rs
@@ -301,7 +301,7 @@ pub fn setup_inbound<PM, CMH, RMH, OMH, L, UMH>(
 {
        let remote_addr = get_addr_from_stream(&stream);
        let (reader, write_receiver, read_receiver, us) = Connection::new(stream);
-       #[cfg(debug_assertions)]
+       #[cfg(test)]
        let last_us = Arc::clone(&us);
 
        let handle_opt = if let Ok(_) = peer_manager.new_inbound_connection(SocketDescriptor::new(us.clone()), remote_addr) {
@@ -322,8 +322,8 @@ pub fn setup_inbound<PM, CMH, RMH, OMH, L, UMH>(
                                // socket shutdown(). Still, as a check during testing, to make sure tokio doesn't
                                // keep too many wakers around, this makes sense. The race should be rare (we do
                                // some work after shutdown()) and an error would be a major memory leak.
-                               #[cfg(debug_assertions)]
-                               assert!(Arc::try_unwrap(last_us).is_ok());
+                               #[cfg(test)]
+                               debug_assert!(Arc::try_unwrap(last_us).is_ok());
                        }
                }
        }
@@ -355,7 +355,7 @@ pub fn setup_outbound<PM, CMH, RMH, OMH, L, UMH>(
 {
        let remote_addr = get_addr_from_stream(&stream);
        let (reader, mut write_receiver, read_receiver, us) = Connection::new(stream);
-       #[cfg(debug_assertions)]
+       #[cfg(test)]
        let last_us = Arc::clone(&us);
        let handle_opt = if let Ok(initial_send) = peer_manager.new_outbound_connection(their_node_id, SocketDescriptor::new(us.clone()), remote_addr) {
                Some(tokio::spawn(async move {
@@ -401,8 +401,8 @@ pub fn setup_outbound<PM, CMH, RMH, OMH, L, UMH>(
                                // socket shutdown(). Still, as a check during testing, to make sure tokio doesn't
                                // keep too many wakers around, this makes sense. The race should be rare (we do
                                // some work after shutdown()) and an error would be a major memory leak.
-                               #[cfg(debug_assertions)]
-                               assert!(Arc::try_unwrap(last_us).is_ok());
+                               #[cfg(test)]
+                               debug_assert!(Arc::try_unwrap(last_us).is_ok());
                        }
                }
        }

diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index af83edc5634d37d76d18c95bf61ede56999cdf23..f502a3336cadd935a474b4abe89ef189f8432b0f 100644 (file)
--- a/lightning/src/ln/channelmanager.rs
+++ b/lightning/src/ln/channelmanager.rs
@@ -1153,12 +1153,12 @@ macro_rules! handle_error {
                match $internal {
                        Ok(msg) => Ok(msg),
                        Err(MsgHandleErrInternal { err, chan_id, shutdown_finish }) => {
-                               #[cfg(debug_assertions)]
+                               #[cfg(any(feature = "_test_utils", test))]
                                {
                                        // In testing, ensure there are no deadlocks where the lock is already held upon
                                        // entering the macro.
-                                       assert!($self.pending_events.try_lock().is_ok());
-                                       assert!($self.per_peer_state.try_write().is_ok());
+                                       debug_assert!($self.pending_events.try_lock().is_ok());
+                                       debug_assert!($self.per_peer_state.try_write().is_ok());
                                }
 
                                let mut msg_events = Vec::with_capacity(2);
@@ -1193,7 +1193,7 @@ macro_rules! handle_error {
                                                let mut peer_state = peer_state_mutex.lock().unwrap();
                                                peer_state.pending_msg_events.append(&mut msg_events);
                                        }
-                                       #[cfg(debug_assertions)]
+                                       #[cfg(any(feature = "_test_utils", test))]
                                        {
                                                if let None = per_peer_state.get(&$counterparty_node_id) {
                                                        // This shouldn't occour in tests unless an unkown counterparty_node_id
@@ -1206,10 +1206,10 @@ macro_rules! handle_error {
                                                                => {
                                                                        assert_eq!(*data, expected_error_str);
                                                                        if let Some((err_channel_id, _user_channel_id)) = chan_id {
-                                                                               assert_eq!(*channel_id, err_channel_id);
+                                                                               debug_assert_eq!(*channel_id, err_channel_id);
                                                                        }
                                                                }
-                                                               _ => panic!("Unexpected event"),
+                                                               _ => debug_assert!(false, "Unexpected event"),
                                                        }
                                                }
                                        }
@@ -3565,7 +3565,7 @@ where
        /// Fails an HTLC backwards to the sender of it to us.
        /// Note that we do not assume that channels corresponding to failed HTLCs are still available.
        fn fail_htlc_backwards_internal(&self, source: &HTLCSource, payment_hash: &PaymentHash, onion_error: &HTLCFailReason, destination: HTLCDestination) {
-               #[cfg(debug_assertions)]
+               #[cfg(any(feature = "_test_utils", test))]
                {
                        // Ensure that no peer state channel storage lock is not held when calling this
                        // function.
@@ -3574,7 +3574,7 @@ where
                        // this function with any `per_peer_state` peer lock aquired would.
                        let per_peer_state = self.per_peer_state.read().unwrap();
                        for (_, peer) in per_peer_state.iter() {
-                               assert!(peer.try_lock().is_ok());
+                               debug_assert!(peer.try_lock().is_ok());
                        }
                }