Fix Windows accept() to fail future instead of raising

Resolves: #5279
2025-12-28 17:04:41 +00:00 · 2017-01-30 13:18:32 +07:00
parent c57fcf42df
commit fb8168d338
6 changed files with 107 additions and 44 deletions
--- a/lib/pure/asyncdispatch.nim
+++ b/lib/pure/asyncdispatch.nim
@@ -753,26 +753,6 @@ when defined(windows) or defined(nimdoc):
    let dwLocalAddressLength = Dword(sizeof(Sockaddr_in) + 16)
    let dwRemoteAddressLength = Dword(sizeof(Sockaddr_in) + 16)

-    template completeAccept() {.dirty.} =
-      var listenSock = socket
-      let setoptRet = setsockopt(clientSock, SOL_SOCKET,
-          SO_UPDATE_ACCEPT_CONTEXT, addr listenSock,
-          sizeof(listenSock).SockLen)
-      if setoptRet != 0: raiseOSError(osLastError())
-
-      var localSockaddr, remoteSockaddr: ptr SockAddr
-      var localLen, remoteLen: int32
-      getAcceptExSockaddrs(addr lpOutputBuf[0], dwReceiveDataLength,
-                           dwLocalAddressLength, dwRemoteAddressLength,
-                           addr localSockaddr, addr localLen,
-                           addr remoteSockaddr, addr remoteLen)
-      register(clientSock.AsyncFD)
-      # TODO: IPv6. Check ``sa_family``. http://stackoverflow.com/a/9212542/492186
-      retFuture.complete(
-        (address: $inet_ntoa(cast[ptr Sockaddr_in](remoteSockAddr).sin_addr),
-         client: clientSock.AsyncFD)
-      )
-
    template failAccept(errcode) =
      if flags.isDisconnectionError(errcode):
        var newAcceptFut = acceptAddr(socket, flags)
@@ -785,6 +765,29 @@ when defined(windows) or defined(nimdoc):
      else:
        retFuture.fail(newException(OSError, osErrorMsg(errcode)))

+    template completeAccept() {.dirty.} =
+      var listenSock = socket
+      let setoptRet = setsockopt(clientSock, SOL_SOCKET,
+          SO_UPDATE_ACCEPT_CONTEXT, addr listenSock,
+          sizeof(listenSock).SockLen)
+      if setoptRet != 0:
+        let errcode = osLastError()
+        checkCloseError clientSock.closeSocket()
+        failAccept(errcode)
+      else:
+        var localSockaddr, remoteSockaddr: ptr SockAddr
+        var localLen, remoteLen: int32
+        getAcceptExSockaddrs(addr lpOutputBuf[0], dwReceiveDataLength,
+                             dwLocalAddressLength, dwRemoteAddressLength,
+                             addr localSockaddr, addr localLen,
+                             addr remoteSockaddr, addr remoteLen)
+        register(clientSock.AsyncFD)
+        # TODO: IPv6. Check ``sa_family``. http://stackoverflow.com/a/9212542/492186
+        retFuture.complete(
+          (address: $inet_ntoa(cast[ptr Sockaddr_in](remoteSockAddr).sin_addr),
+          client: clientSock.AsyncFD)
+        )
+
    var ol = PCustomOverlapped()
    GC_ref(ol)
    ol.data = CompletionData(fd: socket, cb:
--- a/lib/pure/nativesockets.nim
+++ b/lib/pure/nativesockets.nim
@@ -22,11 +22,12 @@ const useWinVersion = defined(Windows) or defined(nimdoc)
 when useWinVersion:
  import winlean
  export WSAEWOULDBLOCK, WSAECONNRESET, WSAECONNABORTED, WSAENETRESET,
+         WSANOTINITIALISED, WSAENOTSOCK, WSAEINPROGRESS, WSAEINTR,
         WSAEDISCON, ERROR_NETNAME_DELETED
 else:
  import posix
  export fcntl, F_GETFL, O_NONBLOCK, F_SETFL, EAGAIN, EWOULDBLOCK, MSG_NOSIGNAL,
-    EINTR, EINPROGRESS, ECONNRESET, EPIPE, ENETRESET
+    EINTR, EINPROGRESS, ECONNRESET, EPIPE, ENETRESET, EBADF
  export Sockaddr_storage, Sockaddr_un, Sockaddr_un_path_length

 export SocketHandle, Sockaddr_in, Addrinfo, INADDR_ANY, SockAddr, SockLen,
--- a/lib/pure/net.nim
+++ b/lib/pure/net.nim
@@ -191,6 +191,31 @@ proc isDisconnectionError*(flags: set[SocketFlag],
    SocketFlag.SafeDisconn in flags and
      lastError.int32 in {ECONNRESET, EPIPE, ENETRESET}

+proc checkCloseError*(ret: cint) =
+  ## Asserts that the return value of close() or closeSocket() syscall
+  ## does not indicate a programming error (such as invalid descriptor).
+  ## This must only be used when an error has already occurred and
+  ## you are performing a cleanup.
+  ## Otherwise, error handling must be performed as usual.
+  ##
+  ## This procedure must be called right after perfoming the syscall. Example:
+  ##
+  ## .. code-block:: nim
+  ##
+  ##  let ret = someSysCall()
+  ##  if ret != 0:
+  ##    let errcode = osLastError()
+  ##    checkCloseError sock.closeSocket()
+  ##    raise newException(OSError, osErrorMsg(errcode))
+
+  if ret != 0:
+    let errcode = osLastError()
+    when useWinVersion:
+      doAssert(errcode.int32 notin {WSANOTINITIALISED, WSAENOTSOCK,
+                                    WSAEINPROGRESS, WSAEINTR, WSAEWOULDBLOCK})
+    else:
+      doAssert(errcode.int32 notin {EBADF})
+
 proc toOSFlags*(socketFlags: set[SocketFlag]): cint =
  ## Converts the flags into the underlying OS representation.
  for f in socketFlags:
--- a/lib/upcoming/asyncdispatch.nim
+++ b/lib/upcoming/asyncdispatch.nim
@@ -738,26 +738,6 @@ when defined(windows) or defined(nimdoc):
    let dwLocalAddressLength = Dword(sizeof(Sockaddr_in) + 16)
    let dwRemoteAddressLength = Dword(sizeof(Sockaddr_in) + 16)

-    template completeAccept() {.dirty.} =
-      var listenSock = socket
-      let setoptRet = setsockopt(clientSock, SOL_SOCKET,
-          SO_UPDATE_ACCEPT_CONTEXT, addr listenSock,
-          sizeof(listenSock).SockLen)
-      if setoptRet != 0: raiseOSError(osLastError())
-
-      var localSockaddr, remoteSockaddr: ptr SockAddr
-      var localLen, remoteLen: int32
-      getAcceptExSockaddrs(addr lpOutputBuf[0], dwReceiveDataLength,
-                           dwLocalAddressLength, dwRemoteAddressLength,
-                           addr localSockaddr, addr localLen,
-                           addr remoteSockaddr, addr remoteLen)
-      register(clientSock.AsyncFD)
-      # TODO: IPv6. Check ``sa_family``. http://stackoverflow.com/a/9212542/492186
-      retFuture.complete(
-        (address: $inet_ntoa(cast[ptr Sockaddr_in](remoteSockAddr).sin_addr),
-         client: clientSock.AsyncFD)
-      )
-
    template failAccept(errcode) =
      if flags.isDisconnectionError(errcode):
        var newAcceptFut = acceptAddr(socket, flags)
@@ -770,6 +750,29 @@ when defined(windows) or defined(nimdoc):
      else:
        retFuture.fail(newException(OSError, osErrorMsg(errcode)))

+    template completeAccept() {.dirty.} =
+      var listenSock = socket
+      let setoptRet = setsockopt(clientSock, SOL_SOCKET,
+          SO_UPDATE_ACCEPT_CONTEXT, addr listenSock,
+          sizeof(listenSock).SockLen)
+      if setoptRet != 0:
+        let errcode = osLastError()
+        checkCloseError clientSock.closeSocket()
+        failAccept(errcode)
+      else:
+        var localSockaddr, remoteSockaddr: ptr SockAddr
+        var localLen, remoteLen: int32
+        getAcceptExSockaddrs(addr lpOutputBuf[0], dwReceiveDataLength,
+                             dwLocalAddressLength, dwRemoteAddressLength,
+                             addr localSockaddr, addr localLen,
+                             addr remoteSockaddr, addr remoteLen)
+        register(clientSock.AsyncFD)
+        # TODO: IPv6. Check ``sa_family``. http://stackoverflow.com/a/9212542/492186
+        retFuture.complete(
+          (address: $inet_ntoa(cast[ptr Sockaddr_in](remoteSockAddr).sin_addr),
+          client: clientSock.AsyncFD)
+        )
+
    var ol = PCustomOverlapped()
    GC_ref(ol)
    ol.data = CompletionData(fd: socket, cb:
--- a/lib/windows/winlean.nim
+++ b/lib/windows/winlean.nim
@@ -419,9 +419,6 @@ const

  ws2dll = "Ws2_32.dll"

-  WSAEWOULDBLOCK* = 10035
-  WSAEINPROGRESS* = 10036
-
 proc wsaGetLastError*(): cint {.importc: "WSAGetLastError", dynlib: ws2dll.}

 type
@@ -760,6 +757,11 @@ const
  WSAEDISCON* = 10101
  WSAENETRESET* = 10052
  WSAETIMEDOUT* = 10060
+  WSANOTINITIALISED* = 10093
+  WSAENOTSOCK* = 10038
+  WSAEINPROGRESS* = 10036
+  WSAEINTR* = 10004
+  WSAEWOULDBLOCK* = 10035
  ERROR_NETNAME_DELETED* = 64
  STATUS_PENDING* = 0x103

--- a/tests/async/tacceptcloserace.nim
+++ b/tests/async/tacceptcloserace.nim
@@ -0,0 +1,29 @@
+import asyncdispatch, net, os, nativesockets
+
+# bug: https://github.com/nim-lang/Nim/issues/5279
+
+proc setupServerSocket(hostname: string, port: Port): AsyncFD =
+  let fd = newNativeSocket()
+  setSockOptInt(fd, SOL_SOCKET, SO_REUSEADDR, 1)
+  var aiList = getAddrInfo(hostname, port)
+  if bindAddr(fd, aiList.ai_addr, aiList.ai_addrlen.Socklen) < 0'i32:
+    freeAddrInfo(aiList)
+    raiseOSError(osLastError())
+  freeAddrInfo(aiList)
+  if listen(fd) != 0:
+    raiseOSError(osLastError())
+  setBlocking(fd, false)
+  result = fd.AsyncFD
+  register(result)
+
+const port = Port(5614)
+for i in 0..100:
+  let serverFd = setupServerSocket("localhost", port)
+  serverFd.accept().callback = proc(fut: Future[AsyncFD]) =
+    if not fut.failed:
+      fut.read().closeSocket()
+
+  var fd = newAsyncNativeSocket()
+  waitFor fd.connect("localhost", port)
+  serverFd.closeSocket()
+  fd.closeSocket()