From: Markus Armbruster <armbru@pond.sub.org>
Date: Tue, 27 Mar 2012 17:58:31 +0000 (+0200)
Subject: Fix synchronization between shutdown and player threads
X-Git-Tag: v4.3.30~17
X-Git-Url: http://git.pond.sub.org/?p=empserver;a=commitdiff_plain;h=49ae6a7b9d81d2dfbad8e26ff48d8a3f071f2c66

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.
---

diff --git a/include/server.h b/include/server.h
index 2c2702a38..0729d5542 100644
--- a/include/server.h
+++ b/include/server.h
@@ -39,6 +39,7 @@
 
 extern int shutdown_pending;
 extern empth_rwlock_t *play_lock;
+extern empth_rwlock_t *shutdown_lock;
 extern int update_running;
 extern time_t update_time[UPDATE_TIME_LEN];
 
diff --git a/src/lib/player/login.c b/src/lib/player/login.c
index 743739658..8b285893d 100644
--- a/src/lib/player/login.c
+++ b/src/lib/player/login.c
@@ -48,6 +48,7 @@
 #include "player.h"
 #include "proto.h"
 #include "prototypes.h"
+#include "server.h"
 
 static int client_cmd(void);
 static int coun_cmd(void);
@@ -79,9 +80,7 @@ player_login(void *ud)
     time_t deadline;
     char buf[128];
     char space[128];
-    int ac;
-    int cmd;
-    int res;
+    int res, ac, cmd, prev_state;
 
     player->proc = empth_self();
 
@@ -126,7 +125,10 @@ player_login(void *ud)
 	    break;
 	}
     }
+    prev_state = player->state;
     player->state = PS_SHUTDOWN;
+    if (prev_state == PS_PLAYING)
+	empth_rwlock_unlock(shutdown_lock);
     pr_id(player, C_EXIT, "so long...\n");
     player_delete(player);
     empth_exit();
@@ -360,6 +362,7 @@ play_cmd(void)
     empth_set_name(empth_self(), buf);
     logerror("%s logged in as country #%d", praddr(player), player->cnum);
     pr_id(player, C_INIT, "%d\n", CLIENTPROTO);
+    empth_rwlock_rdlock(shutdown_lock);
     player->state = PS_PLAYING;
     player_main(player);
     logerror("%s logged out, country #%d", praddr(player), player->cnum);
diff --git a/src/server/main.c b/src/server/main.c
index a3010419c..267de1cdb 100644
--- a/src/server/main.c
+++ b/src/server/main.c
@@ -75,10 +75,16 @@ static void loc_NTInit(void);
 #endif
 
 /*
- * Lock to synchronize player threads with update and shutdown.
- * Update and shutdown takes it exclusive, commands take it shared.
+ * Lock to synchronize player threads with update.
+ * Update holds it exclusive, commands hold it shared.
  */
 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";
 
@@ -374,6 +380,10 @@ start_server(int flags)
     if (journal_startup() < 0)
 	exit(1);
 
+    shutdown_lock = empth_rwlock_create("Shutdown");
+    if (!shutdown_lock)
+	exit_nomem();
+
     market_init();
     update_init();
     empth_create(player_accept, 50 * 1024, flags, "AcceptPlayers", NULL);
@@ -423,9 +433,10 @@ shutdwn(int sig)
 	}
 	empth_wakeup(p->proc);
     }
-    empth_rwlock_wrlock(play_lock);
 
+    empth_rwlock_wrlock(shutdown_lock);
     empth_yield();
+
     for (i = 1; i <= 3 && player_next(NULL); i++) {
 	logerror("Waiting for player threads to terminate\n");
 	empth_sleep(now + i);