nim-lang / Nim

Nim is a statically typed compiled systems programming language. It combines successful concepts from mature languages like Python, Ada and Modula. Its design focuses on efficiency, expressiveness, and elegance (in that order of priority).
https://nim-lang.org
Other
16.48k stars 1.47k forks source link

OSError: Bad file descriptor in asyncnet server #12382

Open iffy opened 4 years ago

iffy commented 4 years ago

An unexpected OSError "Bad file descriptor" exception is raised during what I think is normal use of asyncnet.

Example

## Run this in one session, then run the following in another:
##
##  echo "hello" | nc 127.0.0.1 9001
##
import asyncdispatch
import asyncnet

proc serve(port: Port, address = "") {.async.} =
  var server = newAsyncSocket()
  server.setSockOpt(OptReuseAddr, true)
  server.bindAddr(port, address)
  server.listen()

  let client = await server.accept()

  # application code
  let msg1 = client.recv(1024)
  yield msg1
  asyncCheck client.send("message 1")
  client.close()
  # end application code

if isMainModule:
  asyncCheck serve(9001.Port, "127.0.0.1")
  while true:
    poll()

Run this Nim program in one terminal, then (as noted in the comment at the top of the file) run this in another shell:

echo "hello" | nc 127.0.0.1 9001

Current Output

/private/tmp/badserver.nim(25) badserver
/Users/matt/.choosenim/toolchains/nim-0.20.2/lib/pure/asyncdispatch.nim(1562) poll
/Users/matt/.choosenim/toolchains/nim-0.20.2/lib/pure/asyncdispatch.nim(1328) runOnce
/Users/matt/.choosenim/toolchains/nim-0.20.2/lib/pure/asyncdispatch.nim(210) processPendingCallbacks
/Users/matt/.choosenim/toolchains/nim-0.20.2/lib/pure/asyncmacro.nim(34) recvNimAsyncContinue
/Users/matt/.choosenim/toolchains/nim-0.20.2/lib/pure/asyncnet.nim(309) recvIter
/Users/matt/.choosenim/toolchains/nim-0.20.2/lib/pure/asyncfutures.nim(374) read
[[reraised from:
/private/tmp/badserver.nim(25) badserver
/Users/matt/.choosenim/toolchains/nim-0.20.2/lib/pure/asyncdispatch.nim(1562) poll
/Users/matt/.choosenim/toolchains/nim-0.20.2/lib/pure/asyncdispatch.nim(1328) runOnce
/Users/matt/.choosenim/toolchains/nim-0.20.2/lib/pure/asyncdispatch.nim(210) processPendingCallbacks
/Users/matt/.choosenim/toolchains/nim-0.20.2/lib/pure/asyncfutures.nim(422) asyncCheckCallback
]]
Error: unhandled exception: Bad file descriptor
Async traceback:
  /private/tmp/badserver.nim(25)                                                badserver
  /Users/matt/.choosenim/toolchains/nim-0.20.2/lib/pure/asyncdispatch.nim(1562) poll
  /Users/matt/.choosenim/toolchains/nim-0.20.2/lib/pure/asyncdispatch.nim(1328) runOnce
  /Users/matt/.choosenim/toolchains/nim-0.20.2/lib/pure/asyncdispatch.nim(210)  processPendingCallbacks
  /Users/matt/.choosenim/toolchains/nim-0.20.2/lib/pure/asyncmacro.nim(34)      recvNimAsyncContinue
  /Users/matt/.choosenim/toolchains/nim-0.20.2/lib/pure/asyncnet.nim(309)       recvIter
  /Users/matt/.choosenim/toolchains/nim-0.20.2/lib/pure/asyncfutures.nim(374)   read
Exception message: Bad file descriptor
Exception type: [OSError]
Error: execution of an external program failed: '/private/tmp/badserver '

Expected Output

As written, I'd expect no output from the Nim program.

Possible Solution

I don't have a good solution. In Python, "Bad file descriptor" is raised if you attempt to close a file descriptor twice. I suspect that might be happening here, with the first close from my application code and the second from somewhere in asyncnet or asyncdispatch.

Or I may be using asyncnet wrong. If so, maybe the bug is the error message?

Additional Information

$ nim -v
Nim Compiler Version 0.20.2 [MacOSX: amd64]
Compiled at 2019-07-17
Copyright (c) 2006-2019 by Andreas Rumpf

git hash: 88a0edba4b1a3d535b54336fd589746add54e937
active boot switches: -d:release
Araq commented 4 years ago

So what happens if you leave out the client.close() call?

iffy commented 4 years ago

With client.close() commented out, this is the output:

/private/tmp/badserver.nim(26) badserver
/Users/matt/.choosenim/toolchains/nim-0.20.2/lib/pure/asyncdispatch.nim(1562) poll
/Users/matt/.choosenim/toolchains/nim-0.20.2/lib/pure/asyncdispatch.nim(1280) runOnce
Error: unhandled exception: No handles or timers registered in dispatcher. [ValueError]
Error: execution of an external program failed: '/private/tmp/badserver '
iffy commented 4 years ago

I should note that in my actual complicated code, the close call doesn't happen immediately like this, but somewhere nested deeply in other code. Is it wrong to close the socket myself?

iffy commented 4 years ago

Following the traceback through, I now think the problem is this that there's this call being attempted after the socket has been closed:

https://github.com/nim-lang/Nim/blob/84c956d9dadfac0fa149dbc1eb7adef6888c95dc/lib/pure/asyncnet.nim#L446

And from my code snippet, that makes sense (because client.close() happens right after client.send("message 1") has been initiated but probably not complete).

But I'm not sure what the resolution should be. "Bad file descriptor" isn't very helpful, but I'm not sure if it's asyncnet's job to provide a different error or not.

AboveAverageDeveloper commented 4 years ago

had the same error myself. Replacing

asyncCheck client.send("message 1")

with

asyncCheck client.send("message 1")

did the trick

Araq commented 4 years ago

So ... you replaced the line of code with itself?

AboveAverageDeveloper commented 4 years ago

oh this is embarrassing. so sorry, I meant to say that I replaced the line with

await client.send("message 1")
ghost commented 4 years ago

@AboveAverageDeveloper then it's understandable, in the original example the code is not really correct since asyncCheck starts running a new async procedure concurrently, so it might send AFTER you did client.close()

ghost commented 4 years ago

And with "await" you explicitly wait until the data was sent and only close after that

ghost commented 4 years ago

So I guess the only issue here is the (subjectively) confusing error message

AboveAverageDeveloper commented 4 years ago

mhm the conclusion that I came to when trying to solve was that asyncCheck didn't wait for the return so it would close the client before it writes to the file descriptor

ghost commented 4 years ago

@AboveAverageDeveloper yes, because asyncCheck doesn't have to wait for anything - it sets the callback of the future you want to call to check for error and starts the future without blocking.