Fixed threading issues in gamedata fetcher, new restart mechanism (bug 3351, r=pred).

configs/core.cfg

@@ -94,22 +94,29 @@
 	 *
 	 * The default value is "no". A value of "yes" will block the Auto Updater.
 	 */
-	"DisableAutoUpdate"		"no"
+	"DisableAutoUpdate"			"no"
 
 	/**
-	 * Enables or disables automatic restarting of the server after a successful update.
+	 * If set to yes, a successful gamedata update will attempt to restart SourceMod.
-	 *
+	 * SourceMod is unloaded and reloaded, and the map is changed to the current map.
-	 * The default value is "no". A value of "yes" will let the server automatically restart.
+	 * Since gamedata updates occur when the server loads, impact should be minimal.
+	 * But to be safe, this option is disabled by default.
 	 */
-	"ForceRestartAfterUpdate"		"no"
+	"ForceRestartAfterUpdate"	"no"
 
 	/**
 	 * Sets the server to connect to for auotmatic gamedata updates.
 	 */
-	"AutoUpdateServer"		"smupdate.alliedmods.net"
+	"AutoUpdateServer"			"smupdate.alliedmods.net"
 
 	/**
 	 * Sets the port to connect to on the AutoUpdateServer server
 	 */
 	"AutoUpdatePort"			"6500"
+	
+	/**
+	 * Whether to show debug spew.  
+	 * Currently this will log details about the gamedata updating process.
+	 */
+	"DebugSpew"					"no"
 }

core/GameDataFetcher.cpp

@@ -1,33 +1,33 @@
 /**
-* vim: set ts=4 :
+ * vim: set ts=4 :
-* =============================================================================
+ * =============================================================================
-* SourceMod
+ * SourceMod
-* Copyright (C) 2004-2008 AlliedModders LLC.  All rights reserved.
+ * Copyright (C) 2004-2008 AlliedModders LLC.  All rights reserved.
-* =============================================================================
+ * =============================================================================
-*
+ *
-* This program is free software; you can redistribute it and/or modify it under
+ * This program is free software; you can redistribute it and/or modify it under
-* the terms of the GNU General Public License, version 3.0, as published by the
+ * the terms of the GNU General Public License, version 3.0, as published by the
-* Free Software Foundation.
+ * Free Software Foundation.
-* 
+ * 
-* This program is distributed in the hope that it will be useful, but WITHOUT
+ * This program is distributed in the hope that it will be useful, but WITHOUT
-* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
-* FOR A PARTICULAR PURPOSE.  See the GNU General Public License for more
+ * FOR A PARTICULAR PURPOSE.  See the GNU General Public License for more
-* details.
+ * details.
-*
+ *
-* You should have received a copy of the GNU General Public License along with
+ * You should have received a copy of the GNU General Public License along with
-* this program.  If not, see <http://www.gnu.org/licenses/>.
+ * this program.  If not, see <http://www.gnu.org/licenses/>.
-*
+ *
-* As a special exception, AlliedModders LLC gives you permission to link the
+ * As a special exception, AlliedModders LLC gives you permission to link the
-* code of this program (as well as its derivative works) to "Half-Life 2," the
+ * code of this program (as well as its derivative works) to "Half-Life 2," the
-* "Source Engine," the "SourcePawn JIT," and any Game MODs that run on software
+ * "Source Engine," the "SourcePawn JIT," and any Game MODs that run on software
-* by the Valve Corporation.  You must obey the GNU General Public License in
+ * by the Valve Corporation.  You must obey the GNU General Public License in
-* all respects for all other code used.  Additionally, AlliedModders LLC grants
+ * all respects for all other code used.  Additionally, AlliedModders LLC grants
-* this exception to all derivative works.  AlliedModders LLC defines further
+ * this exception to all derivative works.  AlliedModders LLC defines further
-* exceptions, found in LICENSE.txt (as of this writing, version JULY-31-2007),
+ * exceptions, found in LICENSE.txt (as of this writing, version JULY-31-2007),
-* or <http://www.sourcemod.net/license.php>.
+ * or <http://www.sourcemod.net/license.php>.
-*
+ *
-* Version: $Id$
+ * Version: $Id$
-*/
+ */
 
 #include "GameDataFetcher.h"
 #include "bitbuf.h"
@@ -63,19 +63,49 @@
 #include "compat_wrappers.h"
 #include "sm_stringutil.h"
 #include "md5.h"
+#include "frame_hooks.h"
 
 #define QUERY_MAX_LENGTH 1024
 
-BuildMD5ableBuffer g_MD5Builder;
+static BuildMD5ableBuffer g_MD5Builder;
-FetcherThread g_FetchThread;
+static FetcherThread g_FetchThread;
 
-FILE *logfile = NULL;
+static FILE *logfile = NULL;
 
 bool g_disableGameDataUpdate = false;
+
+/**
+ * Note on this.  If we issue a reload and changelevel, my srcds.exe will emit 
+ *   Assertion Failed: !m_bServiceStarted
+ * on quit.  This seems like a non-issue, because before we just terminated the 
+ * server anyway.  If anyone notices and files a bug, we can look into it further.
+ */
 bool g_restartAfterUpdate = false;
 
-int g_serverPort = 6500;
+static bool was_level_started = false;
-char g_serverAddress[100] = "smupdate.alliedmods.net";
+static int g_serverPort = 6500;
+static char g_serverAddress[100] = "smupdate.alliedmods.net";
+
+static void _ForceRestart(void *data)
+{
+	char cmd[300];
+	g_Logger.LogMessage("Automatically restarting SourceMod after a successful gamedata update.");
+	UTIL_Format(cmd, sizeof(cmd), "meta unload %d\n", g_PLID);
+	engine->ServerCommand(cmd);
+	UTIL_Format(cmd, sizeof(cmd), "changelevel \"%s\"\n", STRING(gpGlobals->mapname));
+	engine->ServerCommand(cmd);
+	UTIL_Format(cmd, sizeof(cmd), "echo SourceMod restarted after gamedata update.\n");
+	engine->ServerCommand(cmd);
+}
+
+static void ForceRestart()
+{
+	FrameAction action;
+
+	action.action = _ForceRestart;
+	action.data = NULL;
+	AddFrameAction(action);
+}
 
 void FetcherThread::RunThread(IThreadHandle *pHandle)
 {
@@ -173,6 +203,11 @@ void FetcherThread::OnTerminate(IThreadHandle *pHandle, bool cancel)
 {
 	g_blockGameDataLoad = false;
 
+	if (cancel)
+	{
+		return;
+	}
+
 	if (wasSuccess)
 	{
 		HandleUpdateStatus(updateStatus, build);
@@ -181,8 +216,10 @@ void FetcherThread::OnTerminate(IThreadHandle *pHandle, bool cancel)
 		{
 			if (g_restartAfterUpdate)
 			{
-				g_Logger.LogMessage("Automatically restarting server after a successful gamedata update!");
+				if (was_level_started)
-				engine->ServerCommand("quit\n");
+				{
+					ForceRestart();
+				}
 			}
 			else
 			{
@@ -642,6 +679,7 @@ void FetcherThread::HandleUpdateStatus(UpdateStatus status, short version[4])
 }
 
 bool g_blockGameDataLoad = false;
+static IThreadHandle *fetch_thread_hndl;
 
 class InitFetch : public SMGlobalClass
 {
@@ -660,7 +698,30 @@ public:
 
 		ThreadParams fetchThreadParams = ThreadParams();
 		fetchThreadParams.prio = ThreadPrio_Low;
-		g_pThreader->MakeThread(&g_FetchThread, &fetchThreadParams);
+		fetch_thread_hndl = g_pThreader->MakeThread(&g_FetchThread, &fetchThreadParams);
+	}
+
+	void OnSourceModShutdown()
+	{
+		fetch_thread_hndl->WaitForThread();
+		fetch_thread_hndl->DestroyThis();
+	}
+
+	void OnSourceModLevelActivated()
+	{
+		was_level_started = true;
+
+		if (g_restartAfterUpdate && 
+			g_FetchThread.wasSuccess && 
+			g_FetchThread.needsRestart)
+		{
+			ForceRestart();
+		}
+	}
+
+	void OnSourceModLevelEnd()
+	{
+		was_level_started = false;
 	}
 
 	ConfigResult OnSourceModConfigChanged(const char *key, 
@@ -820,7 +881,6 @@ CON_COMMAND(sm_gamedata_md5, "Checks the MD5 sum for a given gamedata file")
 
 FetcherThread::~FetcherThread()
 {
-	//delete filenames;
 	SourceHook::CVector<FileData *>::iterator iter = filenames.begin();
 
 	FileData *curData;
@@ -838,5 +898,6 @@ FetcherThread::FetcherThread()
 {
 	memtable = new BaseMemTable(4096);
 	wasSuccess = false;
+	needsRestart = false;
 }
 

core/GameDataFetcher.h

@@ -1,33 +1,33 @@
 /**
-* vim: set ts=4 :
+ * vim: set ts=4 :
-* =============================================================================
+ * =============================================================================
-* SourceMod
+ * SourceMod
-* Copyright (C) 2004-2008 AlliedModders LLC.  All rights reserved.
+ * Copyright (C) 2004-2008 AlliedModders LLC.  All rights reserved.
-* =============================================================================
+ * =============================================================================
-*
+ *
-* This program is free software; you can redistribute it and/or modify it under
+ * This program is free software; you can redistribute it and/or modify it under
-* the terms of the GNU General Public License, version 3.0, as published by the
+ * the terms of the GNU General Public License, version 3.0, as published by the
-* Free Software Foundation.
+ * Free Software Foundation.
-* 
+ * 
-* This program is distributed in the hope that it will be useful, but WITHOUT
+ * This program is distributed in the hope that it will be useful, but WITHOUT
-* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
-* FOR A PARTICULAR PURPOSE.  See the GNU General Public License for more
+ * FOR A PARTICULAR PURPOSE.  See the GNU General Public License for more
-* details.
+ * details.
-*
+ *
-* You should have received a copy of the GNU General Public License along with
+ * You should have received a copy of the GNU General Public License along with
-* this program.  If not, see <http://www.gnu.org/licenses/>.
+ * this program.  If not, see <http://www.gnu.org/licenses/>.
-*
+ *
-* As a special exception, AlliedModders LLC gives you permission to link the
+ * As a special exception, AlliedModders LLC gives you permission to link the
-* code of this program (as well as its derivative works) to "Half-Life 2," the
+ * code of this program (as well as its derivative works) to "Half-Life 2," the
-* "Source Engine," the "SourcePawn JIT," and any Game MODs that run on software
+ * "Source Engine," the "SourcePawn JIT," and any Game MODs that run on software
-* by the Valve Corporation.  You must obey the GNU General Public License in
+ * by the Valve Corporation.  You must obey the GNU General Public License in
-* all respects for all other code used.  Additionally, AlliedModders LLC grants
+ * all respects for all other code used.  Additionally, AlliedModders LLC grants
-* this exception to all derivative works.  AlliedModders LLC defines further
+ * this exception to all derivative works.  AlliedModders LLC defines further
-* exceptions, found in LICENSE.txt (as of this writing, version JULY-31-2007),
+ * exceptions, found in LICENSE.txt (as of this writing, version JULY-31-2007),
-* or <http://www.sourcemod.net/license.php>.
+ * or <http://www.sourcemod.net/license.php>.
-*
+ *
-* Version: $Id$
+ * Version: $Id$
-*/
+ */
 
 #ifndef _INCLUDE_SOURCEMOD_GAMEDATAFETCHER_H_
 #define _INCLUDE_SOURCEMOD_GAMEDATAFETCHER_H_
@@ -92,9 +92,9 @@ private:
 	void HandleUpdateStatus(UpdateStatus status, short version[4]);
 public:
 	SourceHook::CVector<FileData *> filenames;
-private:
-	bool wasSuccess;
 	bool needsRestart;
+	bool wasSuccess;
+private:
 	UpdateStatus updateStatus;
 	BaseMemTable *memtable;
 	short build[4];

core/PlayerManager.cpp

@@ -272,6 +272,13 @@ void PlayerManager::OnServerActivate(edict_t *pEdictList, int edictCount, int cl
 
 	g_OnMapStarted = true;
 
+	SMGlobalClass *cls = SMGlobalClass::head;
+	while (cls)
+	{
+		cls->OnSourceModLevelActivated();
+		cls = cls->m_pGlobalClassNext;
+	}
+
 	SM_ExecuteAllConfigs();
 }
 

core/frame_hooks.cpp

@@ -36,13 +36,64 @@
 #include "MenuStyle_Radio.h"
 #include "PlayerManager.h"
 #include "CoreConfig.h"
+#include <sm_queue.h>
+#include "ThreadSupport.h"
 
-float g_LastMenuTime = 0.0f;
+static IMutex *frame_mutex;
-float g_LastAuthCheck = 0.0f;
+static Queue<FrameAction> *frame_queue;
+static Queue<FrameAction> *frame_actions;
+static float g_LastMenuTime = 0.0f;
+static float g_LastAuthCheck = 0.0f;
 bool g_PendingInternalPush = false;
 
+class FrameActionInit : public SMGlobalClass
+{
+public:
+	void OnSourceModAllInitialized()
+	{
+		frame_queue = new Queue<FrameAction>();
+		frame_actions = new Queue<FrameAction>();
+		frame_mutex = g_pThreader->MakeMutex();
+	}
+
+	void OnSourceModShutdown()
+	{
+		delete frame_queue;
+		delete frame_actions;
+		frame_mutex->DestroyThis();
+	}
+} s_FrameActionInit;
+
+void AddFrameAction(const FrameAction & action)
+{
+	frame_mutex->Lock();
+	frame_queue->push(action);
+	frame_mutex->Unlock();
+}
+
 void RunFrameHooks(bool simulating)
 {
+	/* It's okay if this check races. */
+	if (frame_queue->size())
+	{
+		Queue<FrameAction> *temp;
+
+		/* Very quick lock to move queue/actions back and forth */
+		frame_mutex->Lock();
+		temp = frame_queue;
+		frame_queue = frame_actions;
+		frame_actions = temp;
+		frame_mutex->Unlock();
+
+		/* The server will now be adding to the other queue, so we can process events. */
+		while (!frame_actions->empty())
+		{
+			FrameAction &item = frame_actions->first();
+			frame_actions->pop();
+			item.action(item.data);
+		}
+	}
+
 	/* Frame based hooks */
 	g_DBMan.RunFrame();
 	g_HL2.ProcessFakeCliCmdQueue();

core/frame_hooks.h

@@ -32,8 +32,17 @@
 #ifndef _INCLUDE_SOURCEMOD_FRAME_HOOKS_H_
 #define _INCLUDE_SOURCEMOD_FRAME_HOOKS_H_
 
+typedef void (*FRAMEACTION)(void *data);
+
+struct FrameAction
+{
+	void *data;
+	FRAMEACTION action;
+};
+
 extern bool g_PendingInternalPush;
 
+void AddFrameAction(const FrameAction & action);
 void RunFrameHooks(bool simulating);
 
 #endif //_INCLUDE_SOURCEMOD_FRAME_HOOKS_H_

core/sm_globals.h

@@ -134,6 +134,13 @@ public:
 	{
 	}
 
+	/**
+	 * @brief Called when the level has activated.
+	 */
+	virtual void OnSourceModLevelActivated()
+	{
+	}
+
 	/**
 	 * @brief Called when the level ends.
 	 */