Async try transformation fails on `finally`

[Beluki]

To learn a bit more about Nim, I wrote a small script that checks website links:

import asyncdispatch
import httpclient
import htmlparser
import strtabs
import streams
import strutils
import xmltree

var running = 0

proc testLink(uri: string) {.async.} =  
    try:
        running += 1

        let client = newAsyncHttpClient()
        let response = await client.get(uri)

        if response.status.startsWith("200"):
            echo("OK: ", uri)
        else:
            echo("FAILED: ", uri)

    except:
        echo("ERROR: ", uri)

    finally:
        running -= 1
        client.close()

proc testLinks(uri: string) =
    let data = getContent(uri)
    let html = parseHTML(newStringStream(data))

    for a in html.findAll("a"):
        let href = a.attrs["href"]

        if not href.isNil and href.startsWith("http"):
            asyncCheck testLink(href)


testLinks("http://www.reddit.com/")

while running != 0:
    poll()

Sometimes, it works. Most of the time it crashes with the following stack trace (or something similar depending on how long it takes for it to crash):

Traceback (most recent call last)
test.nim(46)             test
asyncdispatch.nim(406)   poll
system.nim(2266)         :anonymous
asyncdispatch.nim(206)   fail
asyncdispatch.nim(1211)  cb
asyncdispatch.nim(206)   fail
asyncdispatch.nim(1211)  cb
asyncdispatch.nim(206)   fail
asyncdispatch.nim(1211)  cb
asyncdispatch.nim(206)   fail
asyncdispatch.nim(1211)  cb
SIGSEGV: Illegal storage access. (Attempt to read from nil?)

Compiled with: nim -d:ssl compile test.nim
Nim version: Nim Compiler Version 0.11.0 (2015-04-30) [Windows: i386]
GCC from mingw-w64: gcc (GCC) 4.8.3 32 bit

[Araq]

For me it now fails with a stack overflow in debug mode and seems to work in release mode.

[dom96]

Anything that uses try in an async proc I consider dangerous now. My try transformation is shit.

[Beluki]

Testing on the latest devel,

Whenever there's an error, the finally: running -= 1... part does not execute and the program ends up in an infinite loop (the while running != 0 never finishes). I have to stop the program with control+c.

It doesn't crash anymore though.

This happens in both debug and release builds.

@dom96 so such code should simply be made synchronous for now?

[dom96]

@exetoc no, just don't use await in a try statement. There are many ways around it. A future has an error field.

[dom96]

I think there may be two bugs triggering in @Beluki's code.

One is likely that the try and finally transformation performed by the async macro is incorrect, whereas the other has something to do with async in general or perhaps just the httpclient.

[dom96]

Indeed. Because @Beluki's code is creating so many connections the OS gives up with a "Too many open files" error. I modified the code slightly to remove the try statement, in order to see the exceptions which are being thrown and got this:

true: https://www.reddit.com/user/Blassreiter
    nodename nor servname provided, or not known Too many open files
  connect's lead up to read of failed Future:
    Traceback (most recent call last)
    a2.nim(57)               a2
   ....
    asyncnet.nim(187)        connectIter
    asyncdispatch.nim(1075)  connect
    nativesockets.nim(218)   getAddrInfo
    os.nim(135)              raiseOSError
  newConnection's lead up to read of failed Future:
 ....

Don't mind the first line (just some debugging info, which actually inspired me to listen to Blassreiter's soundtrack... 😄 ).

The line nodename nor servname provided, or not known Too many open files is the important bit. This is a concatenation of two errors, one from getAddrInfo which calls raiseOSError with both an error code and another parameter called additionalInfo.

First things first, I will make the error message for this better because currently it is just a mashup of two error messages.

[dom96]

I should clarify that the code no longer crashes with a SIGSEGV, nor a stack overflow. I guess that was fixed since then.

[dom96]

The other problems are indeed caused by the try transformation. I'm afraid that attempting to fix it would just be a waste of time, it's already convoluted and very wrong as it is.

For a workaround you can do something like the following:

import asyncdispatch
import httpclient
import htmlparser
import strtabs
import streams
import strutils
import xmltree

var running = 0

proc testLink(uri: string) {.async.} =  
        running += 1

        let client = newAsyncHttpClient()
        echo("Requesting: ", uri)
        let response = await client.get(uri)

        if response.status.startsWith("200"):
            echo("OK: ", uri)
        else:
            echo("FAILED: ", uri)
        client.close()

proc createCB(link: string): proc (fut: Future[void]) {.closure, gcsafe.} =
  return
    proc (fut: Future[void]) =
      echo(fut.failed, ": ", link)
      if fut.failed:
        echo("    ", fut.error.msg)
      running -= 1

proc testLinks(uri: string) =
    let data = getContent(uri)
    let html = parseHTML(newStringStream(data))

    for a in html.findAll("a"):
        let href = a.attrs.getOrDefault("href")

        if not href.isNil and href.startsWith("http"):
            var fut = testLink(href)
            fut.callback = createCB(href)

testLinks("http://www.reddit.com/")

while running != 0:
    poll()

The code doesn't close the AsyncHttpClient always, but i'll leave that for you to fix. You will also need to limit the amount of requests made and possibly group them together by domain name (the AsyncHttpClient has been designed to be used to make multiple requests to the same domain).