status-im / nim-chronos

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

`SIGSEGV` in `asyncTest`-derived scenario (also, in `asyncTest`) #518

Open tersec opened 3 months ago

tersec commented 3 months ago
import chronos    # only depends on asyncloop's transitive closure

proc t(): Future[int] {.async: (raw: true, raises: [ValueError]).} =
  var retFuture = newFuture[int]("")
  retFuture.fail(newException(ValueError, ""))
  return retFuture

proc m(transp: int): Future[void] {.async: (raw: true, raises: []).} =
  var retFuture = newFuture[void]("")
  retFuture.complete()
  return retFuture

proc p(self: int) {.async.} = await self.m()

try:
  defer: waitFor p(0)    # teardown
  waitFor((proc() {.async.} = discard await t())())  # asyncTest
except CatchableError:
  discard

Across all of Nim 1.6 refc and Nim 2.0 and devel refc and ORC:

rm ~/.cache/nim/ -rf && ~/nim16/bin/nim c -r /tmp/w; ~/nim20/bin/nim c -r --hints:off --mm:refc /tmp/w; ~/nim20/bin/nim c -r --hints:off --mm:orc /tmp/w; ~/nimdevel/bin/nim c -r --hints:off --mm:refc /tmp/w; ~/nimdevel/bin/nim c -r --hints:off --mm:orc /tmp/w
Traceback (most recent call last)
/tmp/w.nim(16)   w
SIGSEGV: Illegal storage access. (Attempt to read from nil?)
Segmentation fault
Error: execution of an external program failed: '/tmp/w'
Traceback (most recent call last)
/tmp/w.nim(16)   w
SIGSEGV: Illegal storage access. (Attempt to read from nil?)
Segmentation fault
Error: execution of an external program failed: '/tmp/w'
Traceback (most recent call last)
.nimble/pkgs2/chronos-4.0.0-2b7b9c774b0fb8c3b1d415da6148c8973c4ba81c/chronos/internal/asyncfutures.nim(217) w
SIGSEGV: Illegal storage access. (Attempt to read from nil?)
Segmentation fault
Error: execution of an external program failed: '/tmp/w'
Traceback (most recent call last)
/tmp/w.nim(16)   w
SIGSEGV: Illegal storage access. (Attempt to read from nil?)
Segmentation fault
Error: execution of an external program failed: '/tmp/w`
Traceback (most recent call last)
.nimble/pkgs2/chronos-4.0.0-2b7b9c774b0fb8c3b1d415da6148c8973c4ba81c/chronos/internal/asyncfutures.nim(217) hmm
SIGSEGV: Illegal storage access. (Attempt to read from nil?)
Segmentation fault
Error: execution of an external program failed: '/tmp/w'

Nim versions:

Nim Compiler Version 1.6.19 [Linux: amd64]
Compiled at 2024-03-03
Copyright (c) 2006-2023 by Andreas Rumpf

git hash: 51c86fdae77b6ebd9c9caf4924ec1edfe7883933
active boot switches: -d:release
Nim Compiler Version 2.0.3 [Linux: amd64]
Compiled at 2024-03-03
Copyright (c) 2006-2023 by Andreas Rumpf

git hash: e03667e92040bae8f8a7f817679a1abb4d19f4eb
active boot switches: -d:release
Nim Compiler Version 2.1.1 [Linux: amd64]
Compiled at 2024-03-03
Copyright (c) 2006-2024 by Andreas Rumpf

git hash: 1e7ca2dc789eafccdb44304f7e42206c3702fc13
active boot switches: -d:release

And this does trigger with the Chronos commit in nimbus-eth2, as well as the one one gets from cloning this repo in a default way.

tersec commented 3 months ago

Slightly less minimized version, showing the uinttest2 aspects more explicitly:

import ./asyncloop
import unittest2

proc connect(): Future[int] {.
             async: (raw: true, raises: [ValueError]).} =
  var retFuture = newFuture[int]("stream.transport.connect")
  retFuture.fail(newException(ValueError,
    "connect local address domain is not equal to target address domain"))
  return retFuture

proc closeWait(transp: int): Future[void] {.async: (raw: true, raises: []).} =
  var retFuture = newFuture[void]("stream.transport.join")
  retFuture.complete()
  return retFuture

type
  TcpTransport = object
    client: int

proc new(T: typedesc[TcpTransport]): T =
  return default(TcpTransport)

method stop(self: TcpTransport) {.async.} =
  discard await allFinished(@[self.client.closeWait()])

var stub: TcpTransport
suite "Tor transport":
  setup:
    stub = TcpTransport.new()
  teardown:
    waitFor stub.stop()

  test "test start and dial using ipv4":
    # asyncTest
    waitFor((
      proc() {.async.} =
        discard await connect()
    )())

But see above, unittest and its fun use of gcsafe and threadvar are not required to repro this.

cheatfate commented 3 months ago

The problem is not in unittest2, because last example is impossible to reproduce for me, but initial one is possible to reproduce on Linux and Windows with Nim 1.6.18.

cheatfate commented 3 months ago

I could also confirm that this (asyncdispatch version)

import asyncdispatch

proc t(): Future[int] =
  var retFuture = newFuture[int]("")
  retFuture.fail(newException(ValueError, ""))
  return retFuture

proc m(transp: int): Future[void] =
  var retFuture = newFuture[void]("")
  retFuture.complete()
  return retFuture

proc p(self: int) {.async.} =
  await self.m()

try:
  defer: waitFor p(0)    # teardown
  waitFor((proc() {.async.} = discard await t())())  # asyncTest
except CatchableError:
  discard

is working cc @arnetheduck

arnetheduck commented 3 months ago

simplified:

import chronos    # only depends on asyncloop's transitive closure

proc t(): Future[void] =
  var retFuture = newFuture[void]("")
  retFuture.fail(newException(ValueError, ""))
  return retFuture

proc m(transp: int): Future[void] =
  var retFuture = newFuture[void]("")
  retFuture.complete()
  return retFuture

proc p(self: int) {.async.} = await self.m()

try:
  try:
    waitFor(t())  # asyncTest
  finally:
    waitFor p(0)    # teardown
except CatchableError:
  discard
arnetheduck commented 3 months ago

Upstream bug: https://github.com/nim-lang/Nim/issues/23390