From 68fc92110539402daf4a70c450ac28be7fb3e04d Mon Sep 17 00:00:00 2001 From: Antonis Geralis <43617260+planetis-m@users.noreply.github.com> Date: Wed, 8 Dec 2021 10:34:45 +0200 Subject: [PATCH 1/2] fix atomics memory ordering for sharedptr Consume was used in the original pr https://github.com/nim-lang/Nim/pull/10485, haven't questioned before https://github.com/nim-lang/Nim/pull/19212 --- threading/smartptrs.nim | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/threading/smartptrs.nim b/threading/smartptrs.nim index 25480b7..c6aebe1 100644 --- a/threading/smartptrs.nim +++ b/threading/smartptrs.nim @@ -81,15 +81,13 @@ type proc `=destroy`*[T](p: var SharedPtr[T]) = if p.val != nil: - if p.val[].counter.load(Consume) == 0: + if fetchSub(p.val[].counter, 1, AcqRel) == 0: `=destroy`(p.val[]) deallocShared(p.val) - else: - atomicDec(p.val[].counter) proc `=copy`*[T](dest: var SharedPtr[T], src: SharedPtr[T]) = if src.val != nil: - atomicInc(src.val[].counter) + discard fetchAdd(src.val[].counter, 1, Relaxed) if dest.val != nil: `=destroy`(dest) dest.val = src.val From 3f605ce300dbc2563ddaf22426a3162fbdd78389 Mon Sep 17 00:00:00 2001 From: Antonis Geralis <43617260+planetis-m@users.noreply.github.com> Date: Fri, 10 Dec 2021 19:37:33 +0200 Subject: [PATCH 2/2] Prefer the original logic over simplification --- threading/smartptrs.nim | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/threading/smartptrs.nim b/threading/smartptrs.nim index c6aebe1..ab579a5 100644 --- a/threading/smartptrs.nim +++ b/threading/smartptrs.nim @@ -81,9 +81,11 @@ type proc `=destroy`*[T](p: var SharedPtr[T]) = if p.val != nil: - if fetchSub(p.val[].counter, 1, AcqRel) == 0: + if p.val[].counter.load(Acquire) == 0: `=destroy`(p.val[]) deallocShared(p.val) + else: + discard fetchSub(p.val[].counter, 1, Release) proc `=copy`*[T](dest: var SharedPtr[T], src: SharedPtr[T]) = if src.val != nil: