asynchttpserver "Bad file descriptor" crashes

[zedeus]

asynchttpserver crashes due to cancelled(?) requests:

/home/zed/src/dev/nim/nitter/src/nitter.nim(30) nitter
/home/zed/.nimble/pkgs/jester-0.4.3/jester.nim(499) serve
/home/zed/.choosenim/toolchains/nim-1.0.0/lib/pure/asyncdispatch.nim(1874) runForever
/home/zed/.choosenim/toolchains/nim-1.0.0/lib/pure/asyncdispatch.nim(1569) poll
/home/zed/.choosenim/toolchains/nim-1.0.0/lib/pure/asyncdispatch.nim(1335) runOnce
/home/zed/.choosenim/toolchains/nim-1.0.0/lib/pure/asyncdispatch.nim(210) processPendingCallbacks
/home/zed/.choosenim/toolchains/nim-1.0.0/lib/pure/asyncmacro.nim(34) sendNimAsyncContinue
/home/zed/.choosenim/toolchains/nim-1.0.0/lib/pure/asyncnet.nim(446) sendIter
/home/zed/.choosenim/toolchains/nim-1.0.0/lib/pure/asyncfutures.nim(383) read
[[reraised from:
/home/zed/src/dev/nim/nitter/src/nitter.nim(30) nitter
/home/zed/.nimble/pkgs/jester-0.4.3/jester.nim(499) serve
/home/zed/.choosenim/toolchains/nim-1.0.0/lib/pure/asyncdispatch.nim(1874) runForever
/home/zed/.choosenim/toolchains/nim-1.0.0/lib/pure/asyncdispatch.nim(1569) poll
/home/zed/.choosenim/toolchains/nim-1.0.0/lib/pure/asyncdispatch.nim(1335) runOnce
/home/zed/.choosenim/toolchains/nim-1.0.0/lib/pure/asyncdispatch.nim(210) processPendingCallbacks
/home/zed/.choosenim/toolchains/nim-1.0.0/lib/pure/asyncfutures.nim(431) asyncCheckCallback
]]
Error: unhandled exception: Bad file descriptor [OSError]

There's a few ways to trigger this. My use case is a website, where navigating around quickly can cause cancelled requests which triggers the error. Another way is scrubbing around a video stream to parts not buffered yet (causing previous buffering to get cancelled), example repo here for reproduction.

It gets weird though, when run behind nginx every request triggers the error. I haven't had time to investigate this, but it occurred on multiple machines with different nginx configurations, which certainly shouldn't cause all requests to get cancelled.

Related: #12325

[zedeus]

I did some investigation and it may be an issue with Jester, but I'm not sure.
Using this simple reproducible example with nginx.

nginx config:

http {
   server {
      listen 80;
      listen [::]:80;
   
      server_name localhost;
   
      location / {
         proxy_pass http://localhost:5000/;
      }
   }
}

Server:

import jester

routes:
  get "/":
    resp Http200

Run:

sudo nginx # -s reload if already running
nim c -d:useStdLib -r server.nim
curl http://localhost

This immediately results in the bad file descriptor exception, so I did some digging, and it appears to happen because the socket is closed before the response gets sent, due to Jester calling asyncCheck from its send proc here.

Likewise, if we replace the server code with the following, where asyncCheck is used inside the request callback, the same exception happens:

import asynchttpserver, asyncdispatch

var server = newAsyncHttpServer()
proc cb(req: Request) {.async.} =
  asyncCheck req.respond(Http200, "Hello World")

waitFor server.serve(Port(5000), cb)

Jester's send proc is not async (neither is most of the call stack above it) so I'm not sure how to go about fixing this

[dom96]

So there is at least two issues here:

• asynchttpserver's respond proc probably shouldn't crash when the socket disconnects (we have a flag called SafeDisconn which should prevent this, not sure why it's not in this case, maybe it's not enabled for send?)

• anything that runs asyncCheck will crash the process when the future fails, this probably isn't ideal and we may wish to enable users to set a global async exception handling callback

[disruptek]

I don't want to leave my server loop to handle an exception, as that is disruptive to all other clients and whatever state I'm working with in the server loop. Hence #12325.

[zedeus]

To mitigate this so I can use Jester with asynchttpserver, I've hacked together a fork that removes support for httpbeast and awaits the requests properly. There might be a way to keep httpbeast support and await asynchttpserver's requests, but this works for now.
https://github.com/zedeus/jester/tree/asyncify

[dom96]

There certainly is a way to keep httpbeast: httpbeast is enabled statically so there is absolutely no reason you would need to remove httpbeast support for this. I'm curious what the actual fix is and would like to merge it into Jester.

[zedeus]

It's not so much supporting httpbeast, it's the critical path (the send procs etc) that require {.async.} with asynchttpserver which afaik httpbeast doesn't. As described in my post with the nginx example, the fix is just changing it to call things async properly instead of asyncCheck'ing the requests. The two issues you mentioned seem orthogonal to me.

Oh also, the leak/race condition I mentioned on IRC went away when I switched to asynchttpserver. Unfortunately I don't have an easy way to reproduce it apart from running httpbeast on nitter.net with its large amount of traffic that makes it easier to trigger. A more analytical approach might be better, it seems to be a file descriptor leak but that's just a guess

[disruptek]

If the repository Zed created doesn't help, and #12325 doesn't help, and Zed's
new branch doesn't help, hit me up in person this weekend and we can walk through
it. I feel like something must be getting lost in translation via irc/GH...

[zedeus]

Turns out supporting both with async was very easy, opened dom96/jester#236