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 2c2702a380a7d48a9e09a55a1671aa1af39f706b..0729d554233ccd7d91102c6c35b74000bfb02759 100644 (file)
--- a/include/server.h
+++ b/include/server.h
@@ -39,6 +39,7 @@
 
 extern int shutdown_pending;
 extern empth_rwlock_t *play_lock;
 
 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];
 
 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 743739658c3942e418208e920f4cf99fec98f7e0..8b285893d5131642f5e0a711d3bf55188607b6ca 100644 (file)
--- 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 "player.h"
 #include "proto.h"
 #include "prototypes.h"

+#include "server.h"

 
 static int client_cmd(void);
 static int coun_cmd(void);
 
 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];
     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();
 
 
     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");
     player_delete(player);
     empth_exit();
     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_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);
     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 a3010419ca2386ce9a1a6fa6a0b2b3272735f09f..267de1cdb37d75cb7e6173065d64c704ff3574c1 100644 (file)
--- a/src/server/main.c
+++ b/src/server/main.c
@@ -75,10 +75,16 @@ static void loc_NTInit(void);
 #endif
 
 /*
 #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;
  */
 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)
        exit(1);
 
     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);
     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_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++) {
        logerror("Waiting for player threads to terminate\n");
        empth_sleep(now + i);
     for (i = 1; i <= 3 && player_next(NULL); i++) {
        logerror("Waiting for player threads to terminate\n");
        empth_sleep(now + i);