From cf9cda3c0ba25e88e670050fbc2453e7df1ab5c6 Mon Sep 17 00:00:00 2001 From: Cassie Jones Date: Fri, 1 Jan 2021 20:48:00 -0500 Subject: [PATCH] [rt] Improve memory ordering of incref/decref --- rt/src/lib.rs | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/rt/src/lib.rs b/rt/src/lib.rs index aea68b9..2ca7986 100644 --- a/rt/src/lib.rs +++ b/rt/src/lib.rs @@ -214,14 +214,23 @@ impl Obj { if !self.is_box() { return; } - (*self.header).rc.fetch_add(1, Ordering::SeqCst); + // Ordering::Relaxed is appropriate here, since we assume that each thread with access to a + // reference owns at least one reference (rather than simply borrowing it). Therefore, + // another thread cannot decrement it to 0 while we are performing this increment (since we + // own a reference), so we only need consistency and not ordering. + (*self.header).rc.fetch_add(1, Ordering::Relaxed); } unsafe fn decref(self) { if !self.is_box() { return; } - if (*self.header).rc.fetch_sub(1, Ordering::SeqCst) == 1 { + // Ordering::AcqRel is appropriate here. I believe we need the Acquire in order to ensure + // we see all previous increments/decrements, so we can properly see that the decref is + // decrementing to 0, and we need the Release in order to ensure that we see all writes to + // the memory before we deallocate. + // (Check against 1 instead of 0 since we're loading the old refcount.) + if (*self.header).rc.fetch_sub(1, Ordering::AcqRel) == 1 { self.dealloc(); } } -- 2.47.0