Fix unwanted player thread blocking on output during shutdown

shutdwn() disables blocking on I/O for all player threads in state
PS_PLAYING, by setting struct player member may_sleep to
PLAYER_SLEEP_NEVER.  This ensures the player threads complete logout
quickly and reliably.  A thread may still block on I/O in io_close()
called from player_delete(), since commit 904822e3, but that's okay,
because it happens after all game state updates.

Bug: if shutdwn() aborts a command, the player thread returns through
dispatch(), which resets may_sleep back to PLAYER_SLEEP_FREELY.  Input
can't block regardless, because the EOF indicator is set, but output
can.  When it happens, the player thread may not complete logout
before shutdwn() terminates the process.

This can make us lose a treasury update (similar to the bug fixed by
commit bdc1c40f; the relevant bug description is in commit note
6f8ca87f), play time update, and log entries.

How?  There are two paths from dispatch() to player_delete().  Here's
the first one:

  1. command()
     Doesn't print since dispatch() returns 0 when it resets may_sleep
  2. player_main()
     Loop and call status()
  3. status()
     If the command set dolcost to a non-trivial amount, print it
     Charge dolcost
     If player went broke or became solvent, notify him
     Charge time used
     Return 0, because shutdwn() set the EOF indicator
  4. player_main()
     Break the loop
     Charge time used
     print Bye-bye
     journal.log the logout
  5. play_cmd()
     server.log the logout
  6. player_login()
     Loop
     Try to flush output
     get EOF, break loop
     print so long
     call player_delete()

Ways the bug can bite:

A. When we block in 4. print Bye-bye, we can fail to log.

B. When we block in 3. print broke/solvent notification, we can
   additionally fail to charge time used.

C. When we block in 3. print dolcost, we can additionally fail to
   charge dolcost.

Note: B. and C. couldn't happen before commit bdc1c40f.  Instead,
something just like C happened always, whether player thread blocked
or not.

The second path:

  1. execute()
     Loop and call status()
  2. status()
     As above
  3. execute()
     break the loop
  4. dispatch()
     Continue with the first path

No additional ways to bite.

Fix by avoiding the may_sleep reset when the player thread is on its
way to terminate: may not sleep and has its EOF indicator set.

Broken in commit 0a4d77e9, v4.3.23.

diff --git a/src/lib/player/dispatch.c b/src/lib/player/dispatch.c
index c576840d7a1839f568a6bd8bb3129c556284cdaf..03b87f795834d4ed0a78a784a630a1fc90124f48 100644 (file)
--- a/src/lib/player/dispatch.c
+++ b/src/lib/player/dispatch.c
@@ -29,7 +29,7 @@
  *  Known contributors to this file:
  *     Dave Pare, 1994
  *     Steve McClure, 1998
- *     Markus Armbruster, 2007-2011
+ *     Markus Armbruster, 2007-2012
  */
 
 #include <config.h>
@@ -112,6 +112,8 @@ dispatch(char *buf, char *redir)
     }
     empth_rwlock_unlock(play_lock);
     player->command = NULL;
-    player->may_sleep = PLAYER_SLEEP_FREELY;
+    if (player->may_sleep != PLAYER_SLEEP_NEVER || !io_eof(player->iop))
+       player->may_sleep = PLAYER_SLEEP_FREELY;
+    /* else we're being kicked out */
     return 0;
 }