nim-lang / RFCs

A repository for your Nim proposals.
135 stars 26 forks source link

Atomics: remove support for non-trivial types #445

Open mratsim opened 2 years ago

mratsim commented 2 years ago

See https://forum.nim-lang.org/t/8830

Apparently it's possible to create an Atomic[seq[int]] with the layout:

https://github.com/nim-lang/Nim/blob/7cafd22/lib/pure/concurrency/atomics.nim#L303-L311

However it's currently broken due to using method call syntax of a private proc in a template. So I assume no one ever tried.

I think we should altogether remove that capability because that spinlock

  template withLock[T: not Trivial](location: var Atomic[T]; order: MemoryOrder; body: untyped): untyped =
    while location.guard.testAndSet(moAcquire): discard
    try:
      body
    finally:
      location.guard.clear(moRelease)

suffers from the ABA problem and will create problems down the line.

mratsim commented 2 years ago

Sidenote, Trivial can be expanded with char and byte.

planetis-m commented 2 years ago

So what aspect of threading/atomics need to be improved?

mratsim commented 2 years ago

I think the library as is achieves the main goal as serving as a building block for concurrency primitives.

I personally don't need anything else from it at the moment.

Varriount commented 2 years ago

@mratsim Out of curiousity, I have seen that C++ has an atomic type. Does that also suffer from the ABA problem? If it doesn't, why not?

mratsim commented 2 years ago

@mratsim Out of curiousity, I have seen that C++ has an atomic type. Does that also suffer from the ABA problem? If it doesn't, why not?

  1. As soon as you use compare-and-swap you need to deal with ABA.

  2. C++ atomics require trivially copiable type so you can apply them to std::vector