Channel.trySend/tryRecv contains a data-race according to thread-sanitizer

[PhilippMDoerner]

Description

Heyho, while working to clear up my own package of data-races I noticed that this example will declare a data-race in thread-sanitizer (tsan):

type Thing = ref object

var chan = Channel[Thing]()
chan.open()
var thr: Thread[void]

proc loop() =
  while true:
    let resp = chan.tryRecv()
    if resp.dataAvailable:
      break

proc main() =
  createThread(thr, loop)
  discard chan.trySend(Thing())

  joinThread(thr)
main()

Compiled with nim r --cc:clang --mm:arc -d:release -d:useMalloc -f --passc:"-fsanitize=thread -fno-omit-frame-pointer -mno-omit-leaf-frame-pointer" --debugger:native --passl:"-fsanitize=thread -fno-omit-frame-pointer -mno-omit-leaf-frame-pointer" src/playground.nim

tsan sees a data-race here between trySend with rawSend in one thread and tryRecv with channelReceive in the other. That is because both access the mask property on RawChannel and tryRecv does it before the lock on the channel has been acquired:

proc tryRecv*[TMsg](c: var Channel[TMsg]): tuple[dataAvailable: bool,
                                                  msg: TMsg] =
  var q = cast[PRawChannel](addr(c))
  if q.mask != ChannelDeadMask: # This is the apparently problematic line
    if tryAcquireSys(q.lock):
      if q.count > 0:
        llRecv(q, addr(result.msg), cast[PNimType](getTypeInfo(result.msg)))
        result.dataAvailable = true
      releaseSys(q.lock)

Even if the data-race does not pose a real problem, I would want to solve it because users of the lib (like me) would want to tsan-check their own applications to make them as stable as can be.

Nim Version

Nim Compiler Version 2.0.2 [Linux: amd64]
Compiled at 2023-12-15
Copyright (c) 2006-2023 by Andreas Rumpf

git hash: c4c44d1
active boot switches: -d:release

Current Output

==================
WARNING: ThreadSanitizer: data race (pid=2330564)
  Write of size 8 at 0x557bc96914f8 by main thread (mutexes: write M0):
    #0 rawSend__system_u5512 /home/philipp/.choosenim/toolchains/nim-2.0.2/lib/system/channels_builtin.nim:327:16 (playground_ADBE1768068E727FD97CC3D798B60A4157F8DA2B+0xeb35d) (BuildId: b7cb7edeeb18d9425e27e9a005739638323b0344)
    #1 sendImpl__system_u5552 /home/philipp/.choosenim/toolchains/nim-2.0.2/lib/system/channels_builtin.nim:364:2 (playground_ADBE1768068E727FD97CC3D798B60A4157F8DA2B+0xeb54a) (BuildId: b7cb7edeeb18d9425e27e9a005739638323b0344)
    #2 trySend__playground_u85 /home/philipp/.choosenim/toolchains/nim-2.0.2/lib/system/channels_builtin.nim:383:11 (playground_ADBE1768068E727FD97CC3D798B60A4157F8DA2B+0xeb878) (BuildId: b7cb7edeeb18d9425e27e9a005739638323b0344)
    #3 main__playground_u77 /home/philipp/dev/playground/src/playground.nim:15:268 (playground_ADBE1768068E727FD97CC3D798B60A4157F8DA2B+0xeb878)
    #4 NimMainModule /home/philipp/dev/playground/src/playground.nim:18:2 (playground_ADBE1768068E727FD97CC3D798B60A4157F8DA2B+0xec021) (BuildId: b7cb7edeeb18d9425e27e9a005739638323b0344)
    #5 NimMainInner /home/philipp/dev/playground/src/playground.nim:42:2 (playground_ADBE1768068E727FD97CC3D798B60A4157F8DA2B+0xec021)
    #6 NimMain /home/philipp/dev/playground/src/playground.nim:53:2 (playground_ADBE1768068E727FD97CC3D798B60A4157F8DA2B+0xec021)
    #7 main /home/philipp/dev/playground/src/playground.nim:61:2 (playground_ADBE1768068E727FD97CC3D798B60A4157F8DA2B+0xec021)

  Previous read of size 8 at 0x557bc96914f8 by thread T1:
    #0 tryRecv__playground_u13 /home/philipp/.choosenim/toolchains/nim-2.0.2/lib/system/channels_builtin.nim:419:18 (playground_ADBE1768068E727FD97CC3D798B60A4157F8DA2B+0xeb04a) (BuildId: b7cb7edeeb18d9425e27e9a005739638323b0344)
    #1 loop__playground_u12 /home/philipp/dev/playground/src/playground.nim:9:116 (playground_ADBE1768068E727FD97CC3D798B60A4157F8DA2B+0xeb709) (BuildId: b7cb7edeeb18d9425e27e9a005739638323b0344)
    #2 threadProcWrapDispatch__stdZtypedthreads_u105 /home/philipp/.choosenim/toolchains/nim-2.0.2/lib/system/threadimpl.nim:66:2 (playground_ADBE1768068E727FD97CC3D798B60A4157F8DA2B+0xeab49) (BuildId: b7cb7edeeb18d9425e27e9a005739638323b0344)
    #3 threadProcWrapStackFrame__stdZtypedthreads_u95 /home/philipp/.choosenim/toolchains/nim-2.0.2/lib/system/threadimpl.nim:95:2 (playground_ADBE1768068E727FD97CC3D798B60A4157F8DA2B+0xeab49)
    #4 threadProcWrapper__stdZtypedthreads_u81 /home/philipp/.choosenim/toolchains/nim-2.0.2/lib/system/threadimpl.nim:101:2 (playground_ADBE1768068E727FD97CC3D798B60A4157F8DA2B+0xe7375) (BuildId: b7cb7edeeb18d9425e27e9a005739638323b0344)

  Location is global 'chan__playground_u5' of size 152 at 0x557bc96914e0 (playground_ADBE1768068E727FD97CC3D798B60A4157F8DA2B+0x14974f8)

  Mutex M0 (0x557bc9691510) created at:
    #0 pthread_mutex_init <null> (playground_ADBE1768068E727FD97CC3D798B60A4157F8DA2B+0x99f78) (BuildId: b7cb7edeeb18d9425e27e9a005739638323b0344)
    #1 initRawChannel__system_u5469 /home/philipp/.choosenim/toolchains/nim-2.0.2/lib/system/channels_builtin.nim:163:2 (playground_ADBE1768068E727FD97CC3D798B60A4157F8DA2B+0xe9ca0) (BuildId: b7cb7edeeb18d9425e27e9a005739638323b0344)
    #2 open__playground_u6 /home/philipp/.choosenim/toolchains/nim-2.0.2/lib/system/channels_builtin.nim:447:2 (playground_ADBE1768068E727FD97CC3D798B60A4157F8DA2B+0xe9ca0)
    #3 NimMainModule /home/philipp/dev/playground/src/playground.nim:4:2 (playground_ADBE1768068E727FD97CC3D798B60A4157F8DA2B+0xec01c) (BuildId: b7cb7edeeb18d9425e27e9a005739638323b0344)
    #4 NimMainInner /home/philipp/dev/playground/src/playground.nim:42:2 (playground_ADBE1768068E727FD97CC3D798B60A4157F8DA2B+0xec01c)
    #5 NimMain /home/philipp/dev/playground/src/playground.nim:53:2 (playground_ADBE1768068E727FD97CC3D798B60A4157F8DA2B+0xec01c)
    #6 main /home/philipp/dev/playground/src/playground.nim:61:2 (playground_ADBE1768068E727FD97CC3D798B60A4157F8DA2B+0xec01c)

  Thread T1 (tid=2330566, running) created by main thread at:
    #0 pthread_create <null> (playground_ADBE1768068E727FD97CC3D798B60A4157F8DA2B+0x64a36) (BuildId: b7cb7edeeb18d9425e27e9a005739638323b0344)
    #1 createThread__stdZtypedthreads_u60 /home/philipp/.choosenim/toolchains/nim-2.0.2/lib/std/typedthreads.nim:246:103 (playground_ADBE1768068E727FD97CC3D798B60A4157F8DA2B+0xe74a9) (BuildId: b7cb7edeeb18d9425e27e9a005739638323b0344)
    #2 createThread__stdZtypedthreads_u51 /home/philipp/.choosenim/toolchains/nim-2.0.2/lib/std/typedthreads.nim:262:2 (playground_ADBE1768068E727FD97CC3D798B60A4157F8DA2B+0xe75a0) (BuildId: b7cb7edeeb18d9425e27e9a005739638323b0344)
    #3 main__playground_u77 /home/philipp/dev/playground/src/playground.nim:14:2 (playground_ADBE1768068E727FD97CC3D798B60A4157F8DA2B+0xeb828) (BuildId: b7cb7edeeb18d9425e27e9a005739638323b0344)
    #4 NimMainModule /home/philipp/dev/playground/src/playground.nim:18:2 (playground_ADBE1768068E727FD97CC3D798B60A4157F8DA2B+0xec021) (BuildId: b7cb7edeeb18d9425e27e9a005739638323b0344)
    #5 NimMainInner /home/philipp/dev/playground/src/playground.nim:42:2 (playground_ADBE1768068E727FD97CC3D798B60A4157F8DA2B+0xec021)
    #6 NimMain /home/philipp/dev/playground/src/playground.nim:53:2 (playground_ADBE1768068E727FD97CC3D798B60A4157F8DA2B+0xec021)
    #7 main /home/philipp/dev/playground/src/playground.nim:61:2 (playground_ADBE1768068E727FD97CC3D798B60A4157F8DA2B+0xec021)

SUMMARY: ThreadSanitizer: data race /home/philipp/.choosenim/toolchains/nim-2.0.2/lib/system/channels_builtin.nim:327:16 in rawSend__system_u5512
==================
ThreadSanitizer: reported 1 warnings

Expected Output

Nothing

Possible Solution

No response

Additional Information

This issue seems to be pretty similar/analogous to the one I raised in the threading repo.

Maybe they have a similar root?

As for why this set-up:

It seems sensible to have a a thread in a while-loop poll a channel for messages dedicated to it. This is a minimal example of such a setup.