Skip to content

Commit

Permalink
nim-lang#55 Remove smart-ptr data-race
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
PhilippMDoerner committed Jan 10, 2024
1 parent 92cd319 commit 4999602
Showing 1 changed file with 2 additions and 2 deletions.
4 changes: 2 additions & 2 deletions threading/smartptrs.nim
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand All @@ -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] =
Expand Down

0 comments on commit 4999602

Please sign in to comment.