Possible memory leak triggered by async

[dom96]

This is a fun one and I caught it thanks to my work on httpbeast.

I was able to systematically create a short sample reproducing the issue with no dependencies (except on the stdlib).

WARNING: This allocates memory very quickly.

import selectors, asyncdispatch
import future

type
  Data = object

type
  OnRequest* = proc (): Future[void] {.gcsafe.}

proc onRequestFutureComplete(theFut: Future[void],
                             selector: Selector[Data]) =
  discard

proc eventLoop(onRequest: OnRequest) =

  let selector = newSelector[Data]()

  let disp = getGlobalDispatcher() # This needs to be here <---

  while true:
    let fut = onRequest()
    if not fut.isNil:
      fut.callback =
        (theFut: Future[void]) =>
          (onRequestFutureComplete(theFut, selector))

when isMainModule:
  proc onRequestAsync(): Future[void] {.gcsafe.} =
    var retFuture255001 = newFuture[void]("onRequest")
    iterator onRequestIter255002(): FutureBase {.closure.} =
      complete(retFuture255001)

    var nameIterVar255005 = onRequestIter255002
    var next255007 = nameIterVar255005()

    return retFuture255001

  eventLoop(onRequestAsync)

Click to expand old reproduction that depends on httpbeast

I wasn't able to reproduce this outside my project, so here is the code needed to reproduce it with ``httpbeast`` as a dependency:

import options, asyncdispatch, strutils

import httpbeast

proc onRequest(req: Request): Future[void] {.async.} =
  if req.httpMethod == some(HttpGet):
    case req.path.get()
    of "/":
      req.send("Hello World")
    else:
      req.send(Http404)

proc onRequestAsync(req: Request): Future[void] =
  var retFuture255001 = newFuture[void]("onRequest")
  iterator onRequestIter255002(): FutureBase {.closure.} =
    if req.httpMethod == some(HttpGet):
      case req.path.get()
      of "/":
        req.send("Hello World")
      else:
        req.send(Http404)
    complete(retFuture255001)

  var nameIterVar255005 = onRequestIter255002
  proc onRequest_continue255003() {.closure.} =
    try:
      if not nameIterVar255005.finished:
        var next255007 = nameIterVar255005()
        while (not next255007.isNil) and
            next255007.finished:
          next255007 = nameIterVar255005()
          if nameIterVar255005.finished:
            break
        if next255007 ==
            nil:
          if not retFuture255001.finished:
            let msg255009 = "Async procedure ($1) yielded `nil`, are you await\'ing a " &
                "`nil` Future?"
            raise newException(AssertionError, msg255009 % "onRequest")
        else:
          {.gcsafe.}:
            {.push, hint[ConvFromXtoItselfNotNeeded]: off.}
            next255007.callback = (proc () {.closure, gcsafe.})(onRequest_continue255003)
            {.pop.}
    except:
      if retFuture255001.finished:
        raise
      else:
        retFuture255001.fail(getCurrentException())

  onRequest_continue255003()
  return retFuture255001

run(onRequestAsync)

Tip: The onRequestAsync procedure is just what the {.async.} pragma produces out of the onRequest procedure defined above.

Here is how I solved it, PR will be coming soon to generate this code:

import options, asyncdispatch, strutils

import httpbeast

proc onRequestAsync(req: Request): Future[void] =
  var retFuture255001 = newFuture[void]("onRequest")
  iterator onRequestIter255002(): FutureBase {.closure.} =
    if req.httpMethod == some(HttpGet):
      case req.path.get()
      of "/":
        req.send("Hello World")
      else:
        req.send(Http404)
    # complete(retFuture255001)

  var nameIterVar255005 = onRequestIter255002
  proc onRequest_continue255003() {.closure.} =
    try:
      if not nameIterVar255005.finished:
        var next255007 = nameIterVar255005()
        while (not next255007.isNil) and
            next255007.finished:
          next255007 = nameIterVar255005()
          if nameIterVar255005.finished:
            break

        if likely(next255007 != nil):
          {.gcsafe.}:
            {.push, hint[ConvFromXtoItselfNotNeeded]: off.}
            next255007.callback = (proc () {.closure, gcsafe.})(onRequest_continue255003)
            {.pop.}
        else:
          let msg255009 = "Async procedure ($1) yielded `nil`, are you await\'ing a `nil` Future?"
          assert retFuture255001.finished, msg255009
    except:
      if retFuture255001.finished:
        raise
      else:
        retFuture255001.fail(getCurrentException())

  onRequest_continue255003()
  return retFuture255001

run(onRequestAsync)

[dom96]

I played around with dumpNumberOfInstances, it seems that the Futures are not being deallocated.

[Heap] Future[system.void]: #48481; bytes: 4654176

[Araq]

That's not even a bug afaict. You call callback= which registers the future as "callsoon" as it's completed already. You then never give the event loop the chance to run these, so disp.callbacks continues to grow and it also keeps the futures alive (these are part of the closure's environment).

[dom96]

Discussion: https://irclogs.nim-lang.org/14-04-2018.html#11:39:27

[Araq]

Fixed in httpbeast.