Fixes the order in which FutureVar and return completions are made.

This caused a pretty bad and subtle bug in the asynchttpserver.
As far as I can understand, the fact that the returned future was
being completed first meant that the underlying async procedure
could continue running and thus clean() the FutureVar
and request new data. The control then went back and the
FutureVar was completed again causing an error.
This commit is contained in:
Dominik Picheta 2017-03-27 21:11:48 +02:00
commit e0bb65e45c

View file

@ -40,6 +40,8 @@ template createCb(retFutureSym, iteratorNameSym,
else: else:
next.callback = cb next.callback = cb
except: except:
futureVarCompletions
if retFutureSym.finished: if retFutureSym.finished:
# Take a look at tasyncexceptions for the bug which this fixes. # Take a look at tasyncexceptions for the bug which this fixes.
# That test explains it better than I can here. # That test explains it better than I can here.
@ -47,7 +49,6 @@ template createCb(retFutureSym, iteratorNameSym,
else: else:
retFutureSym.fail(getCurrentException()) retFutureSym.fail(getCurrentException())
futureVarCompletions
cb() cb()
#{.pop.} #{.pop.}
proc generateExceptionCheck(futSym, proc generateExceptionCheck(futSym,
@ -123,12 +124,16 @@ template createVar(result: var NimNode, futSymName: string,
result.add newVarStmt(futSym, asyncProc) # -> var future<x> = y result.add newVarStmt(futSym, asyncProc) # -> var future<x> = y
useVar(result, futSym, valueReceiver, rootReceiver, fromNode) useVar(result, futSym, valueReceiver, rootReceiver, fromNode)
proc createFutureVarCompletions(futureVarIdents: seq[NimNode]): NimNode proc createFutureVarCompletions(futureVarIdents: seq[NimNode],
{.compileTime.} = fromNode: NimNode): NimNode {.compileTime.} =
result = newStmtList() result = newNimNode(nnkStmtList, fromNode)
# Add calls to complete each FutureVar parameter. # Add calls to complete each FutureVar parameter.
for ident in futureVarIdents: for ident in futureVarIdents:
# Only complete them if they have not been completed already by the user. # Only complete them if they have not been completed already by the user.
# TODO: Once https://github.com/nim-lang/Nim/issues/5617 is fixed.
# TODO: Add line info to the complete() call!
# In the meantime, this was really useful for debugging :)
#result.add(newCall(newIdentNode("echo"), newStrLitNode(fromNode.lineinfo)))
result.add newIfStmt( result.add newIfStmt(
( (
newCall(newIdentNode("not"), newCall(newIdentNode("not"),
@ -145,6 +150,10 @@ proc processBody(node, retFutureSym: NimNode,
case node.kind case node.kind
of nnkReturnStmt: of nnkReturnStmt:
result = newNimNode(nnkStmtList, node) result = newNimNode(nnkStmtList, node)
# As I've painfully found out, the order here really DOES matter.
result.add createFutureVarCompletions(futureVarIdents, node)
if node[0].kind == nnkEmpty: if node[0].kind == nnkEmpty:
if not subTypeIsVoid: if not subTypeIsVoid:
result.add newCall(newIdentNode("complete"), retFutureSym, result.add newCall(newIdentNode("complete"), retFutureSym,
@ -158,8 +167,6 @@ proc processBody(node, retFutureSym: NimNode,
else: else:
result.add newCall(newIdentNode("complete"), retFutureSym, x) result.add newCall(newIdentNode("complete"), retFutureSym, x)
result.add createFutureVarCompletions(futureVarIdents)
result.add newNimNode(nnkReturnStmt, node).add(newNilLit()) result.add newNimNode(nnkReturnStmt, node).add(newNilLit())
return # Don't process the children of this return stmt return # Don't process the children of this return stmt
of nnkCommand, nnkCall: of nnkCommand, nnkCall:
@ -347,6 +354,8 @@ proc asyncSingleProc(prc: NimNode): NimNode {.compileTime.} =
futureVarIdents, nil) futureVarIdents, nil)
# don't do anything with forward bodies (empty) # don't do anything with forward bodies (empty)
if procBody.kind != nnkEmpty: if procBody.kind != nnkEmpty:
procBody.add(createFutureVarCompletions(futureVarIdents, nil))
if not subtypeIsVoid: if not subtypeIsVoid:
procBody.insert(0, newNimNode(nnkPragma).add(newIdentNode("push"), procBody.insert(0, newNimNode(nnkPragma).add(newIdentNode("push"),
newNimNode(nnkExprColonExpr).add(newNimNode(nnkBracketExpr).add( newNimNode(nnkExprColonExpr).add(newNimNode(nnkBracketExpr).add(
@ -366,8 +375,6 @@ proc asyncSingleProc(prc: NimNode): NimNode {.compileTime.} =
# -> complete(retFuture) # -> complete(retFuture)
procBody.add(newCall(newIdentNode("complete"), retFutureSym)) procBody.add(newCall(newIdentNode("complete"), retFutureSym))
procBody.add(createFutureVarCompletions(futureVarIdents))
var closureIterator = newProc(iteratorNameSym, [newIdentNode("FutureBase")], var closureIterator = newProc(iteratorNameSym, [newIdentNode("FutureBase")],
procBody, nnkIteratorDef) procBody, nnkIteratorDef)
closureIterator[4] = newNimNode(nnkPragma, prc[6]).add(newIdentNode("closure")) closureIterator[4] = newNimNode(nnkPragma, prc[6]).add(newIdentNode("closure"))
@ -377,7 +384,7 @@ proc asyncSingleProc(prc: NimNode): NimNode {.compileTime.} =
#var cbName = newIdentNode("cb") #var cbName = newIdentNode("cb")
var procCb = getAst createCb(retFutureSym, iteratorNameSym, var procCb = getAst createCb(retFutureSym, iteratorNameSym,
newStrLitNode(prc[0].getName), newStrLitNode(prc[0].getName),
createFutureVarCompletions(futureVarIdents)) createFutureVarCompletions(futureVarIdents, nil))
outerProcBody.add procCb outerProcBody.add procCb
# -> return retFuture # -> return retFuture
@ -398,7 +405,7 @@ proc asyncSingleProc(prc: NimNode): NimNode {.compileTime.} =
if procBody.kind != nnkEmpty: if procBody.kind != nnkEmpty:
result[6] = outerProcBody result[6] = outerProcBody
#echo(treeRepr(result)) #echo(treeRepr(result))
#if prc[0].getName == "beta": #if prc[0].getName == "recvLineInto":
# echo(toStrLit(result)) # echo(toStrLit(result))
macro async*(prc: untyped): untyped = macro async*(prc: untyped): untyped =