status-im / nim-chronos

Chronos - An efficient library for asynchronous programming
https://status-im.github.io/nim-chronos/
Apache License 2.0
362 stars 51 forks source link

SIGSEGV with defer/waitFor/AsyncQueue #342

Closed iffy closed 1 year ago

iffy commented 1 year ago

On my macOS machine, this minimal code produces the following result:

import chronos

type
  Obj = ref object
    queue: AsyncQueue[string]

proc newQueue(o: Obj): AsyncQueue[string] =
  o.queue = newAsyncQueue[string]()
  return o.queue

block:
  var o = Obj()
  var oq = o.newQueue()
  defer: waitFor o.queue.addLast("foo")
  assert false

Result of nim c -r tests/test_tmp:

Traceback (most recent call last)
/Users/matt/m/buckets.git/ccore/tests/test_tmp.nim(14) test_tmp
SIGSEGV: Illegal storage access. (Attempt to read from nil?)

If I take any line out, it doesn't segfault.

  1. Does this fail for you?
  2. Is this a bug with defer, waitFor, AsyncQueue or my code?
arnetheduck commented 1 year ago

There are several bugs here at once:

testit.nim(15) testit
Nim/lib/system/excpt.nim(417) nimLeaveFinally
Nim/lib/system/excpt.nim(408) reportUnhandledError
Nim/lib/system/excpt.nim(360) reportUnhandledErrorAux
Nim/lib/system/excpt.nim(631) signalHandler

indicating that this is a bug in nim itself.

iffy commented 1 year ago

Okay, thank you! I'll file upstream.

  1. How did you get the call stack? I've been going crazy trying to track down what's happening.
  2. If I change assert false to raise ValueError.newException("foo") it still segfaults. Is my understanding correct that raising ValueError is not a Defect?
arnetheduck commented 1 year ago

How did you get the call stack? I've been going crazy trying to track down what's happening.

by running it in the nimbus build environment which does magic things for call stacks that I'm not entirely sure how they work (git clone nimbus-eth2; make; ./env.sh bash)

still segfaults

this is the Nim bug that the call stack hints at - better repro code is:


import chronos

type
  Obj = ref object
    queue: AsyncQueue[string]

proc newQueue(o: Obj): AsyncQueue[string] =
  o.queue = newAsyncQueue[string]()
  return o.queue

try:
  var o = Obj()
  var oq = o.newQueue()
  defer: waitFor o.queue.addLast("foo")
  raise (ref ValueError)(msg: "test")
except ValueError:
  echo "test"

ie the code triggers an error even when the exception is actually caught - this is likely a closure transformation bug in the exception handling

iffy commented 1 year ago

Thanks! I've filed upstream, so I'll close this as it's not a chronos issue.