Add test coverage for atomics (#15193)

* Add test coverage for atomics

Signed-off-by: Elijah Shaw-Rutschman <elijahr@gmail.com>

* Fix compareExchange bugs for non-trivial objects

Bugs fixed:

1. compareExchange would not set the desired value in the success case.
2. compareExchange would not set var expected to the found value in the failure case.
3. withLock would spin forever running the unit tests. try..body..finally prevents this. Not sure why this makes a difference, since an exception wasn’t being raised, but clearing the guard in a finally block seems correct anyways.

Signed-off-by: Elijah Shaw-Rutschman <elijahr@gmail.com>

1 files changed, 7 insertions, 7 deletions

diff --git a/lib/pure/concurrency/atomics.nim b/lib/pure/concurrency/atomics.nim
index 29491f68c..eaa811aa6 100644
--- a/lib/pure/concurrency/atomics.nim
+++ b/lib/pure/concurrency/atomics.nim
@@ -326,8 +326,10 @@ else:
 
   template withLock[T: not Trivial](location: var Atomic[T]; order: MemoryOrder; body: untyped): untyped =
     while location.guard.testAndSet(moAcquire): discard
-    body
-    location.guard.clear(moRelease)
+    try:
+      body
+    finally:
+      location.guard.clear(moRelease)
 
   proc load*[T: not Trivial](location: var Atomic[T]; order: MemoryOrder = moSequentiallyConsistent): T {.inline.} =
     withLock(location, order):
@@ -345,16 +347,14 @@ else:
   proc compareExchange*[T: not Trivial](location: var Atomic[T]; expected: var T; desired: T; success, failure: MemoryOrder): bool {.inline.} =
     withLock(location, success):
       if location.nonAtomicValue != expected:
+        expected = location.nonAtomicValue
         return false
+      expected = desired
       swap(location.nonAtomicValue, expected)
       return true
 
   proc compareExchangeWeak*[T: not Trivial](location: var Atomic[T]; expected: var T; desired: T; success, failure: MemoryOrder): bool {.inline.} =
-    withLock(location, success):
-      if location.nonAtomicValue != expected:
-        return false
-      swap(location.nonAtomicValue, expected)
-      return true
+    compareExchange(location, expected, desired, success, failure)
 
   proc compareExchange*[T: not Trivial](location: var Atomic[T]; expected: var T; desired: T; order: MemoryOrder = moSequentiallyConsistent): bool {.inline.} =
     compareExchange(location, expected, desired, order, order)