summary refs log tree commit diff stats
diff options
context:
space:
mode:
authorflywind <xzsflywind@gmail.com>2021-06-16 17:31:20 +0800
committerGitHub <noreply@github.com>2021-06-16 11:31:20 +0200
commitc51680e7012bff32156623fad0996d62f7918222 (patch)
tree6f6f8e12e02272c76966d26c42ea0f8d4c09b641
parentc5cf21c0c4f314e4128658d35000339d67e4824b (diff)
downloadNim-c51680e7012bff32156623fad0996d62f7918222.tar.gz
fixes #17696 (#18276)
-rw-r--r--lib/pure/collections/sharedlist.nim7
-rw-r--r--tests/stdlib/tsharedlist.nim53
2 files changed, 47 insertions, 13 deletions
diff --git a/lib/pure/collections/sharedlist.nim b/lib/pure/collections/sharedlist.nim
index c72477675..79f2391f6 100644
--- a/lib/pure/collections/sharedlist.nim
+++ b/lib/pure/collections/sharedlist.nim
@@ -35,8 +35,10 @@ template withLock(t, x: untyped) =
   release(t.lock)
 
 proc iterAndMutate*[A](x: var SharedList[A]; action: proc(x: A): bool) =
-  ## iterates over the list. If 'action' returns true, the
+  ## Iterates over the list. If 'action' returns true, the
   ## current item is removed from the list.
+  ##
+  ## .. warning:: It may not preserve the element order after some modifications.
   withLock(x):
     var n = x.head
     while n != nil:
@@ -47,8 +49,9 @@ proc iterAndMutate*[A](x: var SharedList[A]; action: proc(x: A): bool) =
         if action(n.d[i]):
           acquire(x.lock)
           let t = x.tail
+          dec t.dataLen # TODO considering t.dataLen == 0,
+                        # probably the module should be refactored using doubly linked lists
           n.d[i] = t.d[t.dataLen]
-          dec t.dataLen
         else:
           acquire(x.lock)
           inc i
diff --git a/tests/stdlib/tsharedlist.nim b/tests/stdlib/tsharedlist.nim
index a795be0f3..693f1018e 100644
--- a/tests/stdlib/tsharedlist.nim
+++ b/tests/stdlib/tsharedlist.nim
@@ -1,17 +1,48 @@
-import sharedlist
+discard """
+  matrix: "--threads:on"
+"""
 
-var
-  list: SharedList[int]
-  count: int
+import std/sharedlist
 
-init(list)
+block:
+  var
+    list: SharedList[int]
+    count: int
 
-for i in 1 .. 250:
-  list.add i
+  init(list)
 
-for i in list:
-  inc count
+  for i in 1 .. 250:
+    list.add i
 
-doAssert count == 250
+  for i in list:
+    inc count
 
-deinitSharedList(list)
+  doAssert count == 250
+
+  deinitSharedList(list)
+
+
+block: # bug #17696
+  var keysList = SharedList[string]()
+  init(keysList)
+
+  keysList.add("a")
+  keysList.add("b")
+  keysList.add("c")
+  keysList.add("d")
+  keysList.add("e")
+  keysList.add("f")
+
+
+  # Remove element "b" and "d" from the list. 
+  keysList.iterAndMutate(proc (key: string): bool =
+    if key == "b" or key == "d": # remove only "b" and "d"
+      return true
+    return false
+  )
+
+  var results: seq[string]
+  for key in keysList.items:
+    results.add key
+
+  doAssert results == @["a", "f", "c", "e"]