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.

diff --git a/src/lib/empthread/io.c b/src/lib/empthread/io.c
index ef285a74c4a67f1104b8339e9e70a83e43527a88..47a2b1093e4de2a9cdb188131270077a7e4e276a 100644 (file)
--- 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 70133ff806f171e300ec73e19a5d8b823129697d..87eb9124d44a27b9e76632ffee1a4222f77ab56b 100644 (file)
--- 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 fc002149dfbecface3bf4e023222a71e31c4553f..1f5400c9e4c9720439971f58856dfae58a34e2a9 100644 (file)
--- 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 becfeb34ae1ae10d9e29177ac084ce1ed417a6f7..da02ccdf3edb609c38188b57df23ffab053a100a 100644 (file)
--- 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);