From 043015e829b76e23c97ab6029a61618a8e2d4a6e Mon Sep 17 00:00:00 2001
From: Markus Armbruster <armbru@pond.sub.org>
Date: Wed, 25 Jul 2007 06:10:10 +0000
Subject: [PATCH] (player_kill_idle): Don't kill hung player threads.  That
 code was flawed.  Firstly, when player_kill_idle() ran before the player
 thread could react to being aborted by update or shutdown, player_kill_idle()
 incorrectly diagnosed it as hung.  Secondly, terminating hung threads leaks
 resources and can leave a stale play_lock behind.  It could perhaps even
 corrupt game state.  It might salvage some scenarios, but makes others worse.
  Not worth it.

---
 src/server/idle.c | 18 ++++--------------
 src/server/main.c |  1 -
 2 files changed, 4 insertions(+), 15 deletions(-)

diff --git a/src/server/idle.c b/src/server/idle.c
index d291b265..873261f3 100644
--- a/src/server/idle.c
+++ b/src/server/idle.c
@@ -51,29 +51,19 @@ player_kill_idle(void *unused)
     while (1) {
 	empth_sleep(now + 60);
 	time(&now);
-	/*if (update_pending) */
-	/*continue; */
 	for (p = player_next(0); p != 0; p = player_next(p)) {
 	    if (p->state == PS_SHUTDOWN) {
-		/* player thread hung */
-		/* FIXME but for how long?  unknown if shut down elsewhere! */
-		/* FIXME this can leave stale update_lock behind! */
-		empth_terminate(p->proc);
-		p = player_delete(p);
+		/*
+		 * Player thread hung or just aborted by update or
+		 * shutdown, we can't tell.
+		 */
 		continue;
 	    }
 	    if (p->curup + max_idle * 60 < now) {
 		p->state = PS_SHUTDOWN;
-		/* giving control to another thread while
-		 * in the middle of walking player list is
-		 * not a good idea at all. Sasha */
 		p->aborted++;
 		pr_flash(p, "idle connection terminated\n");
 		empth_wakeup(p->proc);
-		/* go to sleep because player thread
-		   could vandalize a player list */
-
-		break;
 	    }
 	}
     }
diff --git a/src/server/main.c b/src/server/main.c
index 1141008c..5e7c2054 100644
--- a/src/server/main.c
+++ b/src/server/main.c
@@ -385,7 +385,6 @@ shutdwn(int sig)
 	empth_wakeup(p->proc);
     }
     empth_rwlock_wrlock(play_lock);
-    /* rely on player_kill_idle() for killing hung player threads */
     if (sig)
 	logerror("Server shutting down on signal %d", sig);
     else