[PATCH xserver] os: Only mark_client_ready on write failure when client isn't being closed

[Keith Packard]

AbortClient was always calling mark_client_ready so that DIX would
go look at the client again and clean it up. However, if the
precipitating write failure happened while closing the client, the
client would get put into the ready clients list and not taken back
out as the CloseDownClient finished work.

Avoid this by moving the mark_client_ready call out of AbortClient and
stick it only where it should be called. A potentially simpler fix
would be to move the call to mark_client_not_ready in CloseDownClient
after the call to CloseDownConnection.

Signed-off-by: Keith Packard <keithp at keithp.com>
---
 os/connection.c |  9 +++++++--
 os/io.c         | 21 ++++++++++++++-------
 2 files changed, 21 insertions(+), 9 deletions(-)

diff --git a/os/connection.c b/os/connection.c
index 62e298072..de207891a 100644
--- a/os/connection.c
+++ b/os/connection.c
@@ -708,6 +708,7 @@ ClientReady(int fd, int xevents, void *data)
         CloseDownClient(client);
         return;
     }
+    assert (!client->clientGone);
     if (xevents & X_NOTIFY_READ)
         mark_client_ready(client);
     if (xevents & X_NOTIFY_WRITE) {
@@ -910,8 +911,12 @@ CloseDownConnection(ClientPtr client)
     if (FlushCallback)
         CallCallbacks(&FlushCallback, client);
 
-    if (oc->output)
-	FlushClient(client, oc, (char *) NULL, 0);
+    if (oc->output) {
+        int ret;
+	ret = FlushClient(client, oc, (char *) NULL, 0);
+        if (ret < 0)
+            ErrorF("FlushClient returns -1 in CloseDownConnection\n");
+    }
 #ifdef XDMCP
     XdmcpCloseDisplay(oc->fd);
 #endif
diff --git a/os/io.c b/os/io.c
index e1b90d17e..37241b8c7 100644
--- a/os/io.c
+++ b/os/io.c
@@ -613,7 +613,8 @@ FlushAllOutput(void)
             continue;
         if (!client_is_ready(client)) {
             oc = (OsCommPtr) client->osPrivate;
-            (void) FlushClient(client, oc, (char *) NULL, 0);
+            if (FlushClient(client, oc, (char *) NULL, 0) < 0)
+                mark_client_ready(client);
         } else
             NewOutputPending = TRUE;
     }
@@ -635,10 +636,7 @@ SetCriticalOutputPending(void)
 /*****************
  * AbortClient:
  *    When a write error occurs to a client, close
- *    the connection and clean things up. Mark
- *    the client as 'ready' so that the server will
- *    try to read from it again, notice that the fd is
- *    closed and clean up from there.
+ *    the connection and clean things up.
  *****************/
 
 static void
@@ -650,7 +648,7 @@ AbortClient(ClientPtr client)
         _XSERVTransDisconnect(oc->trans_conn);
         _XSERVTransClose(oc->trans_conn);
         oc->trans_conn = NULL;
-        mark_client_ready(client);
+        ErrorF("AbortClient %d\n", client->index);
     }
 }
 
@@ -731,6 +729,7 @@ WriteToClient(ClientPtr who, int count, const void *__buf)
         else if (!(oco = AllocateOutputBuffer())) {
             AbortClient(who);
             MarkClientException(who);
+            mark_client_ready(who);
             return -1;
         }
         oc->output = oco;
@@ -785,7 +784,15 @@ WriteToClient(ClientPtr who, int count, const void *__buf)
             NewOutputPending = FALSE;
         }
 
-        return FlushClient(who, oc, buf, count);
+        count = FlushClient(who, oc, buf, count);
+
+        /* Get the server to try to read from this client again so
+         * that it notices and closes it down
+         */
+        if (count < 0)
+            mark_client_ready(who);
+
+        return count;
     }
 
     NewOutputPending = TRUE;
-- 
2.11.0