From 904822e3449f35ab1dd7a0cbaa924b4e0a406a0a Mon Sep 17 00:00:00 2001
From: Markus Armbruster <armbru@pond.sub.org>
Date: Sun, 29 Jan 2012 12:06:30 +0100
Subject: [PATCH] Fix server shutdown to let player output drain properly

Commit 1e1dfc86 (v4.3.23) attempted to do this, but it's flawed.

Server shutdown makes the player command loops terminate.  Each player
thread then flushes buffered output and closes the server's end of the
connection.  The client eventually gets EOF, and closes its end of the
connection.  All is well.

However, if the client sends more input after the server closed its
end of the connection, but before it completed receiving server
output, it gets ECONNRESET, and the remaining output is lost.

Instead of closing the server's end of the connection, we need to shut
down its transmission direction, then wait for the client to close its
end, by receiving client input until EOF.  Do that in io_close().

The output flushing in player_login() is now superfluous.  Remove it.

Make shutdwn() wait for the io_close() to complete instead of output
queues to drain.  Without that, we could still close the server's end
of the connection prematurely, through program termination.  Change
player_delete() to keep the player in Players until after io_close()
completes, so that shutdwn() can detect completion.
---
 src/lib/empthread/io.c  | 17 +++++++++++++----
 src/lib/player/accept.c |  6 +++---
 src/lib/player/login.c  |  1 -
 src/server/main.c       | 19 +++++--------------
 4 files changed, 21 insertions(+), 22 deletions(-)

diff --git a/src/lib/empthread/io.c b/src/lib/empthread/io.c
index ef285a74c..47a2b1093 100644
--- a/src/lib/empthread/io.c
+++ b/src/lib/empthread/io.c
@@ -108,7 +108,16 @@ io_open(int fd, int flags, int bufsize, struct timeval timeout)
 void
 io_close(struct iop *iop)
 {
-
+    char buf[IO_BUFSIZE];
+    int ret;
+
+    while (io_output(iop, 1) > 0) ;
+    shutdown(iop->fd, SHUT_WR);
+    while (empth_select(iop->fd, EMPTH_FD_READ, &iop->input_timeout) > 0) {
+	ret = read(iop->fd, buf, sizeof(buf));
+	if (ret <= 0)
+	    break;
+    }
     if (iop->input)
 	ioq_destroy(iop->input);
     if (iop->output)
@@ -201,15 +210,15 @@ io_output(struct iop *iop, int wait)
     if (wait)
 	ef_make_stale();
 
-    if (!ioq_qsize(iop->output))
-	return 0;
-
     if ((iop->flags & IO_WRITE) == 0)
 	return -1;
 
     if (iop->flags & IO_ERROR)
 	return -1;
 
+    if (!ioq_qsize(iop->output))
+	return 0;
+
     if (wait) {
 	res = empth_select(iop->fd, EMPTH_FD_WRITE, NULL);
 	if (res == 0)
diff --git a/src/lib/player/accept.c b/src/lib/player/accept.c
index 70133ff80..87eb9124d 100644
--- a/src/lib/player/accept.c
+++ b/src/lib/player/accept.c
@@ -100,14 +100,14 @@ player_delete(struct player *lp)
 {
     struct player *back;
 
-    back = (struct player *)lp->queue.q_back;
-    if (back)
-	emp_remque(&lp->queue);
     if (lp->iop) {
 	/* it's a real player */
 	io_close(lp->iop);
 	lp->iop = NULL;
     }
+    back = (struct player *)lp->queue.q_back;
+    if (back)
+	emp_remque(&lp->queue);
     free(lp);
     /* XXX may need to free bigmap here */
     return back;
diff --git a/src/lib/player/login.c b/src/lib/player/login.c
index fc002149d..1f5400c9e 100644
--- a/src/lib/player/login.c
+++ b/src/lib/player/login.c
@@ -122,7 +122,6 @@ player_login(void *ud)
     }
     player->state = PS_SHUTDOWN;
     pr_id(player, C_EXIT, "so long...\n");
-    while (io_output(player->iop, 1) > 0) ;
     player_delete(player);
     empth_exit();
     /*NOTREACHED*/
diff --git a/src/server/main.c b/src/server/main.c
index becfeb34a..da02ccdf3 100644
--- a/src/server/main.c
+++ b/src/server/main.c
@@ -411,7 +411,7 @@ shutdwn(int sig)
 {
     struct player *p;
     time_t now;
-    int i, queues_drained;
+    int i;
 
     logerror("Shutdown commencing (cleaning up threads.)");
 
@@ -431,22 +431,13 @@ shutdwn(int sig)
 
     now = time(NULL);
     empth_yield();
-    for (i = 1; i <= 3; i++) {
-	queues_drained = 1;
-	for (p = player_next(NULL); p; p = player_next(p)) {
-	    if (io_outputwaiting(p->iop))
-		queues_drained = 0;
-	}
-	if (queues_drained)
-	    break;
-	logerror("Waiting for player output to drain\n");
+    for (i = 1; i <= 3 && player_next(NULL); i++) {
+	logerror("Waiting for player threads to terminate\n");
 	empth_sleep(now + i);
     }
 
-    for (p = player_next(NULL); p; p = player_next(p)) {
-	if (io_outputwaiting(p->iop))
-	    logerror("Output for player %d lost", p->cnum);
-    }
+    for (p = player_next(NULL); p; p = player_next(p))
+	logerror("Player %d lingers, output might be lost", p->cnum);
 
     if (sig)
 	logerror("Server shutting down on signal %d", sig);
-- 
2.43.0