fixes #17696 (#18276)

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

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"]