EVM: Move where `continuation` is cleared to fix a potential stall

This fixes a bug spotted by @mjfh that was introduced by commit 2a7ccceb:

    try:
      if not c.continuation.isNil:
        (c.continuation)()
        c.continuation = nil
      c.selectVM(fork)
    except CatchableError as e:
      ...

The call to `(c.continuation)()` was moved by 2a7ccceb inside the `try` so
that, like all the Op functions do already, if the continuation raises, the
interpreter's general catch turns the exception into a an error status result.

But if the continuation raises an exception, `continuation` is not cleared in
the next line, and at the next resumption the continuation is called again.
It may loop doing this.

This doesn't currently happen because the continuations don't really raise, but
it's still a correctness issue.

This fix also allows a continuation to spawn a second continuation, if it
encounters a second suspension point.  This also doesn't happen currently,
but the pattern will become useful with async EVM.

Signed-off-by: Jamie Lokier <jamie@shareable.org>
This commit is contained in:
Jamie Lokier 2021-05-11 15:51:31 +01:00
parent f4de9d3c80
commit 537cac1bf5
No known key found for this signature in database
GPG Key ID: CBC25C68435C30A2
2 changed files with 1 additions and 1 deletions

View File

@ -307,6 +307,7 @@ proc afterExec(c: Computation) {.noinline.} =
template chainTo*(c: Computation, toChild: typeof(c.child), after: untyped) =
c.child = toChild
c.continuation = proc() =
c.continuation = nil
after
when vm_use_recursion:

View File

@ -395,7 +395,6 @@ proc executeOpcodes(c: Computation) =
try:
if not c.continuation.isNil:
(c.continuation)()
c.continuation = nil
c.selectVM(fork)
except CatchableError as e:
c.setError(&"Opcode Dispatch Error msg={e.msg}, depth={c.msg.depth}", true)