server: Give client EOF when we are done writing

If we detect a fatal error but do not close the socket back to the
client, then we can create deadlock where we are stuck waiting to read
NBD_CMD_DISC from the client but where the client is stuck waiting to
read our reply.  Try harder to explicitly inform the client any time
that we have detected when our connection has degraded enough to
warrant that we won't do any more writing, even if we still hang on to
the socket for a bit longer for further reads.

State-wise, all clients will exit through one of two paths:

normal -> conn->close(SHUT_WR) -> conn->close(SHUT_RDWR)
normal -> conn->close(SHUT_RDWR)

That is, SHUT_WR is an optional state which exists to help avoid
deadlocks, but which does not cause an fd leak of sockin because we
will always reach SHUT_RDWR.

Message-Id: <[email protected]>
(cherry picked from commit bf8653b)

Author: ebblake

server/connections.c

@@ -62,7 +62,7 @@ static int raw_send_socket (const void *buf, size_t len, int flags);
 #ifndef WIN32
 static int raw_send_other (const void *buf, size_t len, int flags);
 #endif
-static void raw_close (void);
+static void raw_close (int how);
 
 conn_status
 connection_get_status (void)
@@ -97,6 +97,8 @@ connection_set_status (conn_status value)
       if (write (conn->status_pipe[1], &c, 1) != 1 && errno != EAGAIN)
         debug ("failed to notify pipe-to-self: %m");
     }
+    if (conn->status >= STATUS_CLIENT_DONE && value < STATUS_CLIENT_DONE)
+      conn->close (SHUT_WR);
     conn->status = value;
   }
   if (conn->nworkers &&
@@ -348,7 +350,7 @@ free_connection (struct connection *conn)
   if (!conn)
     return;
 
-  conn->close ();
+  conn->close (SHUT_RDWR);
 
   /* Don't call the plugin again if quit has been set because the main
    * thread will be in the process of unloading it.  The plugin.unload
@@ -397,6 +399,7 @@ raw_send_socket (const void *vbuf, size_t len, int flags)
   ssize_t r;
   int f = 0;
 
+  assert (sock >= 0);
 #ifdef MSG_MORE
   if (flags & SEND_MORE)
     f |= MSG_MORE;
@@ -427,6 +430,7 @@ raw_send_other (const void *vbuf, size_t len, int flags)
   const char *buf = vbuf;
   ssize_t r;
 
+  assert (sock >= 0);
   while (len > 0) {
     r = write (sock, buf, len);
     if (r == -1) {
@@ -489,12 +493,21 @@ raw_recv (void *vbuf, size_t len)
  * close, so this function ignores errors.
  */
 static void
-raw_close (void)
+raw_close (int how)
 {
   GET_CONN;
 
-  if (conn->sockin >= 0)
-    closesocket (conn->sockin);
-  if (conn->sockout >= 0 && conn->sockin != conn->sockout)
-    closesocket (conn->sockout);
+  if (conn->sockout >= 0 && how == SHUT_WR) {
+    if (conn->sockin == conn->sockout)
+      shutdown (conn->sockout, how);
+    else
+      closesocket (conn->sockout);
+    conn->sockout = -1;
+  }
+  else {
+    if (conn->sockin >= 0)
+      closesocket (conn->sockin);
+    if (conn->sockout >= 0 && conn->sockin != conn->sockout)
+      closesocket (conn->sockout);
+  }
 }

server/crypto.c

@@ -412,25 +412,29 @@ crypto_send (const void *vbuf, size_t len, int flags)
  * close, so this function ignores errors.
  */
 static void
-crypto_close (void)
+crypto_close (int how)
 {
   GET_CONN;
   gnutls_session_t session = conn->crypto_session;
   int sockin, sockout;
 
   assert (session != NULL);
 
-  gnutls_transport_get_int2 (session, &sockin, &sockout);
+  if (how == SHUT_WR)
+    gnutls_bye (session, GNUTLS_SHUT_WR);
+  else {
+    gnutls_transport_get_int2 (session, &sockin, &sockout);
 
-  gnutls_bye (session, GNUTLS_SHUT_RDWR);
+    gnutls_bye (session, GNUTLS_SHUT_RDWR);
 
-  if (sockin >= 0)
-    closesocket (sockin);
-  if (sockout >= 0 && sockin != sockout)
-    closesocket (sockout);
+    if (sockin >= 0)
+      closesocket (sockin);
+    if (sockout >= 0 && sockin != sockout)
+      closesocket (sockout);
 
-  gnutls_deinit (session);
-  conn->crypto_session = NULL;
+    gnutls_deinit (session);
+    conn->crypto_session = NULL;
+  }
 }
 
 #ifdef WIN32

server/internal.h

@@ -194,7 +194,7 @@ typedef int (*connection_recv_function) (void *buf, size_t len)
 typedef int (*connection_send_function) (const void *buf, size_t len,
                                          int flags)
   __attribute__((__nonnull__ (1)));
-typedef void (*connection_close_function) (void);
+typedef void (*connection_close_function) (int how);
 
 /* struct context stores data per connection and backend.  Primarily
  * this is the filter or plugin handle, but other state is also stored