From 4999602ac034d097acda83b4a8d168a83ddb090f Mon Sep 17 00:00:00 2001 From: Philipp Doerner Date: Wed, 10 Jan 2024 17:22:43 +0100 Subject: [PATCH] #55 Remove smart-ptr data-race Using the memory order "Release" here enables a data-race according to tsan. The data-race being *potentially* a false-positive. The race it identifies is between 2 threads both trying to destroy the smartPtr at the same time. They'll both call the decr proc in smartptrs.nim:92 and get past the nil check. Then one of them might get past the second if check and deallocate the ptr. However, that should be impossible to lead to a data race. The count of 0 can only be reached *once* if you're the last thread. I can only assume compiler-order-reshuffling may enable a race here. Using the default mode (SeqCst) as well as Acquire Release (AcqRel) gets rid of said data-race. --- threading/smartptrs.nim | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/threading/smartptrs.nim b/threading/smartptrs.nim index 6a077f5..fb62540 100644 --- a/threading/smartptrs.nim +++ b/threading/smartptrs.nim @@ -93,7 +93,7 @@ proc decr[T](p: SharedPtr[T]) {.inline.} = if p.val != nil: # this `fetchSub` returns current val then subs # so count == 0 means we're the last - if p.val.counter.fetchSub(1, Release) == 0: + if p.val.counter.fetchSub(1, AcqRel) == 0: `=destroy`(p.val.value) deallocShared(p.val) @@ -119,7 +119,7 @@ proc newSharedPtr*[T](val: sink Isolated[T]): SharedPtr[T] {.nodestroy.} = ## Returns a shared pointer which shares ## ownership of the object by reference counting. result.val = cast[typeof(result.val)](allocShared(sizeof(result.val[]))) - int(result.val.counter) = 0 + result.val.counter.store(0) result.val.value = extract val template newSharedPtr*[T](val: T): SharedPtr[T] =