win,tcp: make uv_close work like unix

vtjnash · vtjnash · commit 9a5d720da49d · 2020-11-04T10:32:07.000-05:00
The comment here seems mixed up between send and recv buffers. The default behavior on calling `closesocket` is already to do a graceful shutdown (see https://docs.microsoft.com/en-us/windows/win32/api/winsock/nf-winsock-closesocket with default l_onoff=zero) if it is the last open handle. The expect behavior if there are pending reads in flight is to send an RST packet, notifying the client that the server connection was destroyed before acknowledging the EOF. Refs: libuv#3035 Refs: nodejs/node#35946 Refs: nodejs/node#35904 Fixes: libuv#3034 PR-URL:
diff --git a/src/win/tcp.c b/src/win/tcp.c
@@ -237,11 +237,7 @@ void uv_tcp_endgame(uv_loop_t* loop, uv_tcp_t* handle) {
       handle->reqs_pending == 0) {
     assert(!(handle->flags & UV_HANDLE_CLOSED));
 
-    if (!(handle->flags & UV_HANDLE_TCP_SOCKET_CLOSED)) {
-      closesocket(handle->socket);
-      handle->socket = INVALID_SOCKET;
-      handle->flags |= UV_HANDLE_TCP_SOCKET_CLOSED;
-    }
+    assert(handle->flags & UV_HANDLE_TCP_SOCKET_CLOSED);
 
     if (!(handle->flags & UV_HANDLE_CONNECTION) && handle->tcp.serv.accept_reqs) {
       if (handle->flags & UV_HANDLE_EMULATE_IOCP) {
@@ -1442,48 +1438,32 @@ static int uv_tcp_try_cancel_io(uv_tcp_t* tcp) {
 void uv_tcp_close(uv_loop_t* loop, uv_tcp_t* tcp) {
   int close_socket = 1;
 
-  if (tcp->flags & UV_HANDLE_READ_PENDING) {
-    /* In order for winsock to do a graceful close there must not be any any
-     * pending reads, or the socket must be shut down for writing */
-    if (!(tcp->flags & UV_HANDLE_SHARED_TCP_SOCKET)) {
-      /* Just do shutdown on non-shared sockets, which ensures graceful close. */
-      shutdown(tcp->socket, SD_SEND);
-
-    } else if (uv_tcp_try_cancel_io(tcp) == 0) {
-      /* In case of a shared socket, we try to cancel all outstanding I/O,. If
-       * that works, don't close the socket yet - wait for the read req to
-       * return and close the socket in uv_tcp_endgame. */
-      close_socket = 0;
-
-    } else {
-      /* When cancelling isn't possible - which could happen when an LSP is
-       * present on an old Windows version, we will have to close the socket
-       * with a read pending. That is not nice because trailing sent bytes may
-       * not make it to the other side. */
-    }
-
-  } else if ((tcp->flags & UV_HANDLE_SHARED_TCP_SOCKET) &&
-             tcp->tcp.serv.accept_reqs != NULL) {
+  if (tcp->flags & UV_HANDLE_SHARED_TCP_SOCKET) {
     /* Under normal circumstances closesocket() will ensure that all pending
      * accept reqs are canceled. However, when the socket is shared the
      * presence of another reference to the socket in another process will keep
      * the accept reqs going, so we have to ensure that these are canceled. */
-    if (uv_tcp_try_cancel_io(tcp) != 0) {
-      /* When cancellation is not possible, there is another option: we can
-       * close the incoming sockets, which will also cancel the accept
-       * operations. However this is not cool because we might inadvertently
-       * close a socket that just accepted a new connection, which will cause
-       * the connection to be aborted. */
-      unsigned int i;
-      for (i = 0; i < uv_simultaneous_server_accepts; i++) {
-        uv_tcp_accept_t* req = &tcp->tcp.serv.accept_reqs[i];
-        if (req->accept_socket != INVALID_SOCKET &&
-            !HasOverlappedIoCompleted(&req->u.io.overlapped)) {
-          closesocket(req->accept_socket);
-          req->accept_socket = INVALID_SOCKET;
+    if (tcp->tcp.serv.accept_reqs != NULL) {
+      if (uv_tcp_try_cancel_io(tcp) != 0) {
+        /* When cancellation is not possible, there is another option: we can
+         * close the incoming sockets, which will also cancel the accept
+         * operations. However this is not cool because we might inadvertently
+         * close a socket that just accepted a new connection, which will cause
+         * the connection to be aborted. */
+        unsigned int i;
+        for (i = 0; i < uv_simultaneous_server_accepts; i++) {
+          uv_tcp_accept_t* req = &tcp->tcp.serv.accept_reqs[i];
+          if (req->accept_socket != INVALID_SOCKET &&
+              !HasOverlappedIoCompleted(&req->u.io.overlapped)) {
+            closesocket(req->accept_socket);
+            req->accept_socket = INVALID_SOCKET;
+          }
         }
       }
     }
+    else if (tcp->flags & UV_HANDLE_READ_PENDING) {
+      uv_tcp_try_cancel_io(tcp);
+    }
   }
 
   if (tcp->flags & UV_HANDLE_READING) {
@@ -1496,11 +1476,9 @@ void uv_tcp_close(uv_loop_t* loop, uv_tcp_t* tcp) {
     DECREASE_ACTIVE_COUNT(loop, tcp);
   }
 
-  if (close_socket) {
-    closesocket(tcp->socket);
-    tcp->socket = INVALID_SOCKET;
-    tcp->flags |= UV_HANDLE_TCP_SOCKET_CLOSED;
-  }
+  closesocket(tcp->socket);
+  tcp->socket = INVALID_SOCKET;
+  tcp->flags |= UV_HANDLE_TCP_SOCKET_CLOSED;
 
   tcp->flags &= ~(UV_HANDLE_READABLE | UV_HANDLE_WRITABLE);
   uv__handle_closing(tcp);
