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.
This commit is contained in:
Markus Armbruster 2012-01-29 12:06:30 +01:00
parent 8549efbc19
commit 904822e344
Notes: Markus Armbruster 2012-03-28 19:19:57 +02:00
Login command quit has the same problem.  I believe it could always
lose output.

Before Empire 2, quit_cmd() tried to flush output, threw away anything
it couldn't flush, then closed the connection.

Empire 2's quit_cmd() shut down the input direction of the connection
right away.  This terminated the command loop, and led to connection
close.  I believe it also made client input sends fail with ECONNRESET
right away.

Since commit 0a7306a5, the input shutdown is gone.  We get behavior
similar to server shutdown: command loop terminates, which leads to
close of the server's end of the connection.

In all cases, client sending input after input shutdown or connection
close fails with ECONNRESET, and remaining output is lost.
4 changed files with 20 additions and 21 deletions

View file

@ -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)

View file

@ -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;

View file

@ -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*/

View file

@ -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);