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:
parent
86f0294573
commit
49ae6a7b9d
3 changed files with 21 additions and 6 deletions
|
@ -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];
|
||||||
|
|
||||||
|
|
|
@ -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);
|
||||||
|
|
|
@ -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);
|
||||||
|
|
Loading…
Add table
Add a link
Reference in a new issue