From a7df3ffbe0ed6d10cc952e47f4e92a69403363dc Mon Sep 17 00:00:00 2001 From: Kay Zheng Date: Sun, 12 Apr 2015 12:59:56 +0800 Subject: [PATCH 1/4] Ignore EvError in `asyncdispatch.poll(...)` for non-windows systems, so that exceptions can be raised from `send(...)` and `recv(...)` --- lib/pure/asyncdispatch.nim | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/pure/asyncdispatch.nim b/lib/pure/asyncdispatch.nim index 27f77cef21..6ed7825453 100644 --- a/lib/pure/asyncdispatch.nim +++ b/lib/pure/asyncdispatch.nim @@ -882,9 +882,9 @@ else: let data = PData(info.key.data) assert data.fd == info.key.fd.TAsyncFD #echo("In poll ", data.fd.cint) - if EvError in info.events: - closeSocket(data.fd) - continue + # There may be EvError here, but we handle them in callbacks, + # so that exceptions can be raised from `send(...)` and + # `recv(...)` routines. if EvRead in info.events: # Callback may add items to ``data.readCBs`` which causes issues if From 371d2739249ad33267e0b330e582a04d7d28fd98 Mon Sep 17 00:00:00 2001 From: Kay Zheng Date: Fri, 17 Apr 2015 22:24:06 +0800 Subject: [PATCH 2/4] Add a test case for the EvError handling issue --- tests/async/tasynceverror.nim | 62 +++++++++++++++++++++++++++++++++++ 1 file changed, 62 insertions(+) create mode 100644 tests/async/tasynceverror.nim diff --git a/tests/async/tasynceverror.nim b/tests/async/tasynceverror.nim new file mode 100644 index 0000000000..564df569e2 --- /dev/null +++ b/tests/async/tasynceverror.nim @@ -0,0 +1,62 @@ +discard """ + file: "tasynceverror.nim" + exitcode: 1 + outputsub: "Error: unhandled exception: Connection reset by peer [Exception]" +""" + +import + asyncdispatch, + asyncnet, + rawsockets, + os + + +const + testHost = "127.0.0.1" + testPort = Port(17357) + + +proc createListenSocket(host: string, port: Port): TAsyncFD = + result = newAsyncRawSocket() + + SocketHandle(result).setSockOptInt(SOL_SOCKET, SO_REUSEADDR, 1) + + var aiList = getAddrInfo(host, port, AF_INET) + if SocketHandle(result).bindAddr(aiList.ai_addr, aiList.ai_addrlen.Socklen) < 0'i32: + dealloc(aiList) + raiseOSError(osLastError()) + dealloc(aiList) + + if SocketHandle(result).listen(1) < 0'i32: + raiseOSError(osLastError()) + + +proc testAsyncSend() {.async.} = + var + ls = createListenSocket(testHost, testPort) + s = newAsyncSocket() + + await s.connect(testHost, testPort) + + var ps = await ls.accept() + SocketHandle(ls).close() + + await ps.send("test 1", flags={}) + s.close() + # This send should raise EPIPE + await ps.send("test 2", flags={}) + SocketHandle(ps).close() + + +# The bug was, when the poll function handled EvError for us, +# our callbacks may never get executed, thus making the event +# loop block indefinitely. This is a timer to keep everything +# rolling. 400 ms is an arbitrary value, should be enough though. +proc timer() {.async.} = + await sleepAsync(400) + echo("Timer expired.") + quit(2) + + +asyncCheck(testAsyncSend()) +waitFor(timer()) From 3125058b6ab4bd9c3ad2cb103bb326463af0a7ff Mon Sep 17 00:00:00 2001 From: Kay Zheng Date: Fri, 17 Apr 2015 22:40:53 +0800 Subject: [PATCH 3/4] Only run the test for the fixed version of poll(...) --- tests/async/tasynceverror.nim | 69 ++++++++++++++++++----------------- 1 file changed, 36 insertions(+), 33 deletions(-) diff --git a/tests/async/tasynceverror.nim b/tests/async/tasynceverror.nim index 564df569e2..3b81680cbd 100644 --- a/tests/async/tasynceverror.nim +++ b/tests/async/tasynceverror.nim @@ -16,47 +16,50 @@ const testPort = Port(17357) -proc createListenSocket(host: string, port: Port): TAsyncFD = - result = newAsyncRawSocket() +when defined(windows) or defined(nimdoc): + discard +else: + proc createListenSocket(host: string, port: Port): TAsyncFD = + result = newAsyncRawSocket() - SocketHandle(result).setSockOptInt(SOL_SOCKET, SO_REUSEADDR, 1) + SocketHandle(result).setSockOptInt(SOL_SOCKET, SO_REUSEADDR, 1) - var aiList = getAddrInfo(host, port, AF_INET) - if SocketHandle(result).bindAddr(aiList.ai_addr, aiList.ai_addrlen.Socklen) < 0'i32: - dealloc(aiList) - raiseOSError(osLastError()) - dealloc(aiList) + var aiList = getAddrInfo(host, port, AF_INET) + if SocketHandle(result).bindAddr(aiList.ai_addr, aiList.ai_addrlen.Socklen) < 0'i32: + dealloc(aiList) + raiseOSError(osLastError()) + dealloc(aiList) - if SocketHandle(result).listen(1) < 0'i32: - raiseOSError(osLastError()) + if SocketHandle(result).listen(1) < 0'i32: + raiseOSError(osLastError()) -proc testAsyncSend() {.async.} = - var - ls = createListenSocket(testHost, testPort) - s = newAsyncSocket() + proc testAsyncSend() {.async.} = + var + ls = createListenSocket(testHost, testPort) + s = newAsyncSocket() - await s.connect(testHost, testPort) - - var ps = await ls.accept() - SocketHandle(ls).close() + await s.connect(testHost, testPort) + + var ps = await ls.accept() + SocketHandle(ls).close() - await ps.send("test 1", flags={}) - s.close() - # This send should raise EPIPE - await ps.send("test 2", flags={}) - SocketHandle(ps).close() + await ps.send("test 1", flags={}) + s.close() + # This send should raise EPIPE + await ps.send("test 2", flags={}) + SocketHandle(ps).close() -# The bug was, when the poll function handled EvError for us, -# our callbacks may never get executed, thus making the event -# loop block indefinitely. This is a timer to keep everything -# rolling. 400 ms is an arbitrary value, should be enough though. -proc timer() {.async.} = - await sleepAsync(400) - echo("Timer expired.") - quit(2) + # The bug was, when the poll function handled EvError for us, + # our callbacks may never get executed, thus making the event + # loop block indefinitely. This is a timer to keep everything + # rolling. 400 ms is an arbitrary value, should be enough though. + proc timer() {.async.} = + await sleepAsync(400) + echo("Timer expired.") + quit(2) -asyncCheck(testAsyncSend()) -waitFor(timer()) + asyncCheck(testAsyncSend()) + waitFor(timer()) From a11a2f0fdb3ecdd995e4fc1a6cb41de4e7fc12f2 Mon Sep 17 00:00:00 2001 From: Kay Zheng Date: Sat, 18 Apr 2015 10:27:35 +0800 Subject: [PATCH 4/4] Check for async errors in --- lib/pure/asyncdispatch.nim | 14 +++++++++++--- tests/async/tasyncconnect.nim | 33 +++++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 3 deletions(-) create mode 100644 tests/async/tasyncconnect.nim diff --git a/lib/pure/asyncdispatch.nim b/lib/pure/asyncdispatch.nim index 6ed7825453..ca5b13c78e 100644 --- a/lib/pure/asyncdispatch.nim +++ b/lib/pure/asyncdispatch.nim @@ -923,9 +923,17 @@ else: var retFuture = newFuture[void]("connect") proc cb(fd: TAsyncFD): bool = - # We have connected. - retFuture.complete() - return true + var ret = SocketHandle(fd).getSockOptInt(cint(SOL_SOCKET), cint(SO_ERROR)) + if ret == 0: + # We have connected. + retFuture.complete() + return true + elif ret == EINTR: + # interrupted, keep waiting + return false + else: + retFuture.fail(newException(OSError, osErrorMsg(OSErrorCode(ret)))) + return true var aiList = getAddrInfo(address, port, af) var success = false diff --git a/tests/async/tasyncconnect.nim b/tests/async/tasyncconnect.nim new file mode 100644 index 0000000000..bc63b8e82a --- /dev/null +++ b/tests/async/tasyncconnect.nim @@ -0,0 +1,33 @@ +discard """ + file: "tasyncconnect.nim" + exitcode: 1 + outputsub: "Error: unhandled exception: Connection refused [Exception]" +""" + +import + asyncdispatch, + posix + + +const + testHost = "127.0.0.1" + testPort = Port(17357) + + +when defined(windows) or defined(nimdoc): + discard +else: + proc testAsyncConnect() {.async.} = + var s = newAsyncRawSocket() + + await s.connect(testHost, testPort) + + var peerAddr: SockAddr + var addrSize = Socklen(sizeof(peerAddr)) + var ret = SocketHandle(s).getpeername(addr(peerAddr), addr(addrSize)) + + if ret < 0: + echo("`connect(...)` failed but no exception was raised.") + quit(2) + + waitFor(testAsyncConnect())