From 7c74afc35a235d27946872ea41acccf8b227e1d8 Mon Sep 17 00:00:00 2001 From: Greg Fitzgerald Date: Thu, 28 Jun 2018 11:20:08 -0600 Subject: [PATCH] Relax recycler Instead of asserting ref count is 1 before recycling, allow users to recycle items early. If it turns out that was too early, and allocate() wants to return it, then boot it and take a memory allocation performance hit instead. --- src/packet.rs | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/src/packet.rs b/src/packet.rs index 99382caee8..caa98d6819 100644 --- a/src/packet.rs +++ b/src/packet.rs @@ -163,13 +163,25 @@ impl Clone for Recycler { impl Recycler { pub fn allocate(&self) -> Arc> { let mut gc = self.gc.lock().expect("recycler lock in pb fn allocate"); - gc.pop() - .unwrap_or_else(|| Arc::new(RwLock::new(Default::default()))) + let x = gc.pop() + .unwrap_or_else(|| Arc::new(RwLock::new(Default::default()))); + + // Only return the item if this recycler is the last reference to it. + // Remove this check once `T` holds a Weak reference back to this + // recycler and implements `Drop`. At the time of this writing, Weak can't + // be passed across threads ('alloc' is a nightly-only API), and so our + // reference-counted recyclables are awkwardly being recycled by hand, + // which allows this race condition to exist. + if Arc::strong_count(&x) > 1 { + warn!("Recycled item still in use. Booting it."); + self.allocate() + } else { + x + } } - pub fn recycle(&self, msgs: Arc>) { - assert_eq!(Arc::strong_count(&msgs), 1); // Ensure this function holds that last reference. + pub fn recycle(&self, x: Arc>) { let mut gc = self.gc.lock().expect("recycler lock in pub fn recycle"); - gc.push(msgs); + gc.push(x); } }