Merge pull request #46 from elcritch/test-smartptrsleak

Test and Fixed for SharedPtr leak

tests/tsmartptrsleak.nim

@@ -0,0 +1,55 @@
+import threading/smartptrs
+import std/isolation
+import std/locks
+import threading/atomics
+import threading/channels
+
+var
+  freeCounts: Atomic[int]
+
+type
+  TestObj = object
+
+when defined(nimAllowNonVarDestructor):
+  proc `=destroy`(obj: TestObj) =
+    discard freeCounts.fetchAdd(1, Release)
+else:
+  proc `=destroy`(obj: var TestObj) =
+    discard freeCounts.fetchAdd(1, Release)
+
+var
+  thr: array[0..1, Thread[void]]
+  chan = newChan[SharedPtr[TestObj]]()
+
+const
+  # N = 10_000_000
+  # doSleep = false
+  N = 10_000
+  doSleep = true
+
+import std/os
+
+proc threadA() {.thread.} =
+  for i in 1..N:
+    block:
+      var a: SharedPtr[TestObj] = newSharedPtr(unsafeIsolate TestObj())
+      var b = a
+      chan.send(b)
+      assert a.isNil == false # otherwise we don't copy a?
+      when doSleep:
+        os.sleep(1)
+
+proc threadB() {.thread.} =
+  for i in 1..N:
+    block:
+      var b: SharedPtr[TestObj] = chan.recv()
+      when doSleep:
+        os.sleep(1)
+
+createThread(thr[0], threadA)
+createThread(thr[1], threadB)
+joinThreads(thr)
+
+echo "freeCounts: got: ", $int(freeCounts), " expected: ", N
+echo ""
+assert freeCounts.load(Acquire) == N

threading/smartptrs.nim

@@ -89,22 +89,20 @@ type
     ## Shared ownership reference counting pointer.
     val: ptr tuple[value: T, counter: Atomic[int]]
 
+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:
+      `=destroy`(p.val.value)
+      deallocShared(p.val)
+
 when defined(nimAllowNonVarDestructor):
   proc `=destroy`*[T](p: SharedPtr[T]) =
-    if p.val != nil:
-      if p.val.counter.load(Acquire) == 0:
-        `=destroy`(p.val.value)
-        deallocShared(p.val)
-      else:
-        discard fetchSub(p.val.counter, 1, Release)
+    p.decr()
 else:
   proc `=destroy`*[T](p: var SharedPtr[T]) =
-    if p.val != nil:
-      if p.val.counter.load(Acquire) == 0:
-        `=destroy`(p.val.value)
-        deallocShared(p.val)
-      else:
-        discard fetchSub(p.val.counter, 1, Release)
+    p.decr()
 
 proc `=dup`*[T](src: SharedPtr[T]): SharedPtr[T] =
   if src.val != nil: