Fix synchronization between shutdown and player threads

shutdwn() sets the EOF indicator, aborts the running command, if any,
forbids sleeping on I/O and wakes up the player thread, for all player
threads in state PS_PLAYING.  It takes play_lock to prevent new
commands from running.  It then waits up to 3s for player threads to
terminate, by polling player_next(), to let output buffers drain.

Issues:

1. Polling is lame.

2. New player threads can still enter state PS_PLAYING.  They'll block
   as soon as they try to run a command.  Somehwat unclean.

3. We can exit before all player threads left state PS_PLAYING, losing
   a treasury update, play time update, and log entries.  Could happen
   when player threads blocked on output until commit 90b3abc5 fixed
   that; its commit message describes the bug's impact in more detail.
   Since then, the bug shouldn't bite in practice, because player
   threads should leave state PS_PLAYING quickly.

Fix by introducing shutdown_lock: player threads in state PS_PLAYING
hold it shared, shutdwn() takes it exclusive, instead of play_lock.
Takes care of the issues as follows:

3. shutdwn() waits until all player threads left state PS_PLAYING, no
   matter how long it takes them.

2. New player threads block before entering state PS_PLAYING.

1. shutdwn() still polls up to 3s for player threads to terminate.
   Still lame.  Left for another day.
This commit is contained in:
Markus Armbruster 2012-03-27 19:58:31 +02:00
parent 86f0294573
commit 49ae6a7b9d
3 changed files with 21 additions and 6 deletions

View file

@ -39,6 +39,7 @@
extern int shutdown_pending; extern int shutdown_pending;
extern empth_rwlock_t *play_lock; extern empth_rwlock_t *play_lock;
extern empth_rwlock_t *shutdown_lock;
extern int update_running; extern int update_running;
extern time_t update_time[UPDATE_TIME_LEN]; extern time_t update_time[UPDATE_TIME_LEN];

View file

@ -48,6 +48,7 @@
#include "player.h" #include "player.h"
#include "proto.h" #include "proto.h"
#include "prototypes.h" #include "prototypes.h"
#include "server.h"
static int client_cmd(void); static int client_cmd(void);
static int coun_cmd(void); static int coun_cmd(void);
@ -79,9 +80,7 @@ player_login(void *ud)
time_t deadline; time_t deadline;
char buf[128]; char buf[128];
char space[128]; char space[128];
int ac; int res, ac, cmd, prev_state;
int cmd;
int res;
player->proc = empth_self(); player->proc = empth_self();
@ -126,7 +125,10 @@ player_login(void *ud)
break; break;
} }
} }
prev_state = player->state;
player->state = PS_SHUTDOWN; player->state = PS_SHUTDOWN;
if (prev_state == PS_PLAYING)
empth_rwlock_unlock(shutdown_lock);
pr_id(player, C_EXIT, "so long...\n"); pr_id(player, C_EXIT, "so long...\n");
player_delete(player); player_delete(player);
empth_exit(); empth_exit();
@ -360,6 +362,7 @@ play_cmd(void)
empth_set_name(empth_self(), buf); empth_set_name(empth_self(), buf);
logerror("%s logged in as country #%d", praddr(player), player->cnum); logerror("%s logged in as country #%d", praddr(player), player->cnum);
pr_id(player, C_INIT, "%d\n", CLIENTPROTO); pr_id(player, C_INIT, "%d\n", CLIENTPROTO);
empth_rwlock_rdlock(shutdown_lock);
player->state = PS_PLAYING; player->state = PS_PLAYING;
player_main(player); player_main(player);
logerror("%s logged out, country #%d", praddr(player), player->cnum); logerror("%s logged out, country #%d", praddr(player), player->cnum);

View file

@ -75,10 +75,16 @@ static void loc_NTInit(void);
#endif #endif
/* /*
* Lock to synchronize player threads with update and shutdown. * Lock to synchronize player threads with update.
* Update and shutdown takes it exclusive, commands take it shared. * Update holds it exclusive, commands hold it shared.
*/ */
empth_rwlock_t *play_lock; empth_rwlock_t *play_lock;
/*
* Lock to synchronize player threads with shutdown.
* Shutdown holds it exclusive, player threads in state PS_PLAYING
* hold it shared.
*/
empth_rwlock_t *shutdown_lock;
static char pidfname[] = "server.pid"; static char pidfname[] = "server.pid";
@ -374,6 +380,10 @@ start_server(int flags)
if (journal_startup() < 0) if (journal_startup() < 0)
exit(1); exit(1);
shutdown_lock = empth_rwlock_create("Shutdown");
if (!shutdown_lock)
exit_nomem();
market_init(); market_init();
update_init(); update_init();
empth_create(player_accept, 50 * 1024, flags, "AcceptPlayers", NULL); empth_create(player_accept, 50 * 1024, flags, "AcceptPlayers", NULL);
@ -423,9 +433,10 @@ shutdwn(int sig)
} }
empth_wakeup(p->proc); empth_wakeup(p->proc);
} }
empth_rwlock_wrlock(play_lock);
empth_rwlock_wrlock(shutdown_lock);
empth_yield(); empth_yield();
for (i = 1; i <= 3 && player_next(NULL); i++) { for (i = 1; i <= 3 && player_next(NULL); i++) {
logerror("Waiting for player threads to terminate\n"); logerror("Waiting for player threads to terminate\n");
empth_sleep(now + i); empth_sleep(now + i);