From 1691060a22a93cb6fa3a5e3eff41b0774b1be627 Mon Sep 17 00:00:00 2001 From: Greg Fitzgerald Date: Mon, 25 Jun 2018 17:13:26 -0600 Subject: [PATCH] Assert recycler is given last reference to data This patch likely fixes the sporadic failures in the following tests: ``` test server::tests::validator_exit ... FAILED test streamer::test::streamer_send_test ... FAILED test thin_client::tests::test_bad_sig ... FAILED test drone::tests::test_send_airdrop ... FAILED test thin_client::tests::test_thin_client ... FAILED ``` --- src/ncp.rs | 3 --- src/packet.rs | 1 + src/streamer.rs | 24 ++++++++++++------------ 3 files changed, 13 insertions(+), 15 deletions(-) diff --git a/src/ncp.rs b/src/ncp.rs index bf19f7c138..b4f8b759fb 100644 --- a/src/ncp.rs +++ b/src/ncp.rs @@ -64,10 +64,7 @@ mod tests { use std::sync::{Arc, RwLock}; #[test] - #[ignore] // test that stage will exit when flag is set - // TODO: Troubleshoot Docker-based coverage build and re-enabled - // this test. It is probably failing due to too many threads. fn test_exit() { let exit = Arc::new(AtomicBool::new(false)); let tn = TestNode::new(); diff --git a/src/packet.rs b/src/packet.rs index 5dfde8c807..7f117bd338 100644 --- a/src/packet.rs +++ b/src/packet.rs @@ -166,6 +166,7 @@ impl Recycler { .unwrap_or_else(|| Arc::new(RwLock::new(Default::default()))) } pub fn recycle(&self, msgs: Arc>) { + assert_eq!(Arc::strong_count(&msgs), 1); // Ensure this function holds that last reference. let mut gc = self.gc.lock().expect("recycler lock in pub fn recycle"); gc.push(msgs); } diff --git a/src/streamer.rs b/src/streamer.rs index 958724b828..b6654905c9 100644 --- a/src/streamer.rs +++ b/src/streamer.rs @@ -29,19 +29,18 @@ fn recv_loop( ) -> Result<()> { loop { let msgs = re.allocate(); - let msgs_ = msgs.clone(); loop { - match msgs.write() + let result = msgs.write() .expect("write lock in fn recv_loop") - .recv_from(sock) - { + .recv_from(sock); + match result { Ok(()) => { - channel.send(msgs_)?; + channel.send(msgs)?; break; } Err(_) => { if exit.load(Ordering::Relaxed) { - re.recycle(msgs_); + re.recycle(msgs); return Ok(()); } } @@ -760,12 +759,13 @@ mod test { let mut msgs = VecDeque::new(); for i in 0..10 { let b = resp_recycler.allocate(); - let b_ = b.clone(); - let mut w = b.write().unwrap(); - w.data[0] = i as u8; - w.meta.size = PACKET_DATA_SIZE; - w.meta.set_addr(&addr); - msgs.push_back(b_); + { + let mut w = b.write().unwrap(); + w.data[0] = i as u8; + w.meta.size = PACKET_DATA_SIZE; + w.meta.set_addr(&addr); + } + msgs.push_back(b); } s_responder.send(msgs).expect("send"); let mut num = 0;