Merge pull request #437 from alliedmodders/rm-pausing-12

Fix race between plugin unload and asynchronous query completion.

core/logic/Database.cpp

@@ -693,7 +693,7 @@ void DBManager::OnSourceModIdentityDropped(IdentityToken_t *pToken)
 	s_pAddBlock = NULL;
 }
 
-void DBManager::OnPluginUnloaded(IPlugin *plugin)
+void DBManager::OnPluginWillUnload(IPlugin *plugin)
 {
 	/* Kill the thread so we can flush everything into the think queue... */
 	KillWorkerThread();
@@ -719,9 +719,7 @@ void DBManager::OnPluginUnloaded(IPlugin *plugin)
 		}
 	}
 
-	for (iter = templist.begin();
+	for (iter = templist.begin(); iter != templist.end(); iter++)
-		 iter != templist.end();
-		 iter++)
 	{
 		IDBThreadOperation *op = (*iter);
 		op->RunThinkPart();

core/logic/Database.h

@@ -101,7 +101,7 @@ public: //ke::IRunnable
 	void Run();
 	void ThreadMain();
 public: //IPluginsListener
-	void OnPluginUnloaded(IPlugin *plugin);
+	void OnPluginWillUnload(IPlugin *plugin);
 public:
 	ConfDbInfo *GetDatabaseConf(const char *name);
 	IDBDriver *FindOrLoadDriver(const char *name);

core/logic/PluginSys.cpp

@@ -44,7 +44,8 @@
 #include "Translator.h"
 #include "Logger.h"
 #include "frame_tasks.h"
-#include <am-string.h>
+#include <amtl/am-string.h>
+#include <amtl/am-linkedlist.h>
 #include <bridge/include/IVEngineServerBridge.h>
 #include <bridge/include/CoreProvider.h>
 
@@ -1446,13 +1447,22 @@ bool CPluginManager::ScheduleUnload(CPlugin *pPlugin)
 	if (pPlugin->State() == PluginState::WaitingToUnload)
 		return false;
 
-	IPluginContext *pContext = pPlugin->GetBaseContext();
+	// It is not safe to unload any plugin while another is on the callstack.
-	if (pContext && pContext->IsInExec()) {
+	bool any_active = false;
-		ke::Lambda<void()> callback = [this, pPlugin]() {
+	for (PluginIter iter(m_plugins); !iter.done(); iter.next()) {
-			this->ScheduleUnload(pPlugin);
+		if (IPluginContext *context = (*iter)->GetBaseContext()) {
-		};
+			if (context->IsInExec()) {
+				any_active = true;
+				break;
+			}
+		}
+	}
+
+	if (any_active) {
 		pPlugin->SetWaitingToUnload();
-		ScheduleTaskForNextFrame(ke::Move(callback));
+		ScheduleTaskForNextFrame([this, pPlugin] () -> void {
+			ScheduleUnload(pPlugin);
+		});
 		return false;
 	}
 
@@ -1463,10 +1473,17 @@ bool CPluginManager::ScheduleUnload(CPlugin *pPlugin)
 
 void CPluginManager::Purge(CPlugin *plugin)
 {
-
 	// Go through our libraries and tell other plugins they're gone.
 	plugin->LibraryActions(LibraryAction_Removed);
 
+	// Notify listeners of unloading.
+	if (plugin->EnteredSecondPass()) {
+		for (ListenerIter iter(m_listeners); !iter.done(); iter.next()) {
+			if ((*iter)->GetApiVersion() >= kMinPluginSysApiWithWillUnloadCallback)
+				(*iter)->OnPluginWillUnload(plugin);
+		}
+	}
+
 	// We only pair OnPluginEnd with OnPluginStart if we would have
 	// successfully called OnPluginStart, *and* SetFailState() wasn't called,
 	// which guarantees no further code will execute.
@@ -2232,7 +2249,42 @@ void CPluginManager::ForEachPlugin(ke::Lambda<void(CPlugin *)> callback)
 		callback(*iter);
 }
 
-class OldPluginAPI : public IPluginManager
+class PluginsListenerV1Wrapper final
+	: public IPluginsListener,
+	  public ke::Refcounted<PluginsListenerV1Wrapper>
+{
+public:
+	PluginsListenerV1Wrapper(IPluginsListener_V1 *impl)
+		: impl_(impl)
+	{}
+
+	// The v2 listener was added with API v7, so we pin these wrappers to v6.
+	unsigned int GetApiVersion() const override {
+		return 6;
+	}
+
+	void OnPluginLoaded(IPlugin *plugin) override {
+		impl_->OnPluginLoaded(plugin);
+	}
+	void OnPluginPauseChange(IPlugin *plugin, bool paused) override {
+		impl_->OnPluginPauseChange(plugin, paused);
+	}
+	void OnPluginUnloaded(IPlugin *plugin) override {
+		impl_->OnPluginUnloaded(plugin);
+	}
+	void OnPluginDestroyed(IPlugin *plugin) override {
+		impl_->OnPluginDestroyed(plugin);
+	}
+
+	bool matches(IPluginsListener_V1 *impl) const {
+		return impl_ == impl;
+	}
+
+private:
+	IPluginsListener_V1 *impl_;
+};
+
+class OldPluginAPI final : public IPluginManager
 {
 public:
 	IPlugin *LoadPlugin(const char *path,
@@ -2240,45 +2292,71 @@ public:
 						PluginType type,
 						char error[],
 						size_t maxlength,
-						bool *wasloaded)
+						bool *wasloaded) override
 	{
 		return g_PluginSys.LoadPlugin(path, debug, type, error, maxlength, wasloaded);
 	}
 
-	bool UnloadPlugin(IPlugin *plugin)
+	bool UnloadPlugin(IPlugin *plugin) override
 	{
 		return g_PluginSys.UnloadPlugin(plugin);
 	}
 
-	IPlugin *FindPluginByContext(const sp_context_t *ctx)
+	IPlugin *FindPluginByContext(const sp_context_t *ctx) override
 	{
 		return g_PluginSys.FindPluginByContext(ctx);
 	}
 
-	unsigned int GetPluginCount()
+	unsigned int GetPluginCount() override
 	{
 		return g_PluginSys.GetPluginCount();
 	}
 
-	IPluginIterator *GetPluginIterator()
+	IPluginIterator *GetPluginIterator() override
 	{
 		return g_PluginSys.GetPluginIterator();
 	}
 
-	void AddPluginsListener(IPluginsListener *listener)
+	void AddPluginsListener_V1(IPluginsListener_V1 *listener) override
+	{
+		ke::Ref<PluginsListenerV1Wrapper> wrapper = new PluginsListenerV1Wrapper(listener);
+
+		v1_wrappers_.append(wrapper);
+		g_PluginSys.AddPluginsListener(wrapper);
+	}
+
+	void RemovePluginsListener_V1(IPluginsListener_V1 *listener) override
+	{
+		ke::Ref<PluginsListenerV1Wrapper> wrapper;
+
+		// Find which wrapper has this listener.
+		for (decltype(v1_wrappers_)::iterator iter(v1_wrappers_); !iter.done(); iter.next()) {
+			if ((*iter)->matches(listener)) {
+				wrapper = *iter;
+				iter.remove();
+				break;
+			}
+		}
+		g_PluginSys.RemovePluginsListener(wrapper);
+	}
+
+	IPlugin *PluginFromHandle(Handle_t handle, HandleError *err) override
+	{
+		return g_PluginSys.PluginFromHandle(handle, err);
+	}
+
+	void AddPluginsListener(IPluginsListener *listener) override
 	{
 		g_PluginSys.AddPluginsListener(listener);
 	}
 
-	void RemovePluginsListener(IPluginsListener *listener)
+	void RemovePluginsListener(IPluginsListener *listener) override
 	{
 		g_PluginSys.RemovePluginsListener(listener);
 	}
 
-	IPlugin *PluginFromHandle(Handle_t handle, HandleError *err)
+private:
-	{
+	ReentrantList<ke::Ref<PluginsListenerV1Wrapper>> v1_wrappers_;
-		return g_PluginSys.PluginFromHandle(handle, err);
-	}
 };
 
 static OldPluginAPI sOldPluginAPI;

plugins/include/core.inc

@@ -1,5 +1,5 @@
 /**
- * vim: set ts=4 :
+ * vim: set ts=4 sw=4 tw=99 noet:
  * =============================================================================
  * SourceMod (C)2004-2008 AlliedModders LLC.  All rights reserved.
  * =============================================================================
@@ -92,6 +92,7 @@ enum PluginStatus
 	Plugin_Created,			/**< Plugin is created but not initialized */
 	Plugin_Uncompiled,		/**< Plugin is not yet compiled by the JIT */
 	Plugin_BadLoad,			/**< Plugin failed to load */
+	Plugin_Evicted			/**< Plugin was unloaded due to an error */
 };
 
 /**

public/IPluginSys.h

@@ -38,7 +38,7 @@
 #include <sp_vm_api.h>
 
 #define SMINTERFACE_PLUGINSYSTEM_NAME		"IPluginManager"
-#define SMINTERFACE_PLUGINSYSTEM_VERSION	6
+#define SMINTERFACE_PLUGINSYSTEM_VERSION	8
 
 /** Context user slot 3 is used Core for holding an IPluginContext pointer. */
 #define SM_CONTEXTVAR_USER		3
@@ -278,7 +278,7 @@ namespace SourceMod
 	/**
 	 * @brief Listens for plugin-oriented events.
 	 */
-	class IPluginsListener
+	class IPluginsListener_V1
 	{
 	public:
 		// @brief This callback should not be used since plugins may be in
@@ -304,6 +304,11 @@ namespace SourceMod
 		// any plugin for which OnPluginLoaded was called, and is invoked
 		// immediately after OnPluginEnd(). The plugin may be in any state Failed
 		// or lower.
+		//
+		// This function must not cause the plugin to re-enter script code. If
+		// you wish to be notified of when a plugin is unloading, and to forbid
+		// future calls on that plugin, use OnPluginWillUnload and use a
+		// plugin property to block future calls.
 		virtual void OnPluginUnloaded(IPlugin *plugin)
 		{
 		}
@@ -315,6 +320,27 @@ namespace SourceMod
 		}
 	};
 
+	// @brief Listens for plugin-oriented events. Extends the V1 listener class.
+	class IPluginsListener : public IPluginsListener_V1
+	{
+	public:
+		virtual unsigned int GetApiVersion() const {
+			return SMINTERFACE_PLUGINSYSTEM_VERSION;
+		}
+
+		// @brief Called when a plugin is about to be unloaded, before its
+		// OnPluginEnd callback is fired. This can be used to ensure that any
+		// asynchronous operations are flushed and no further operations can
+		// be started (via SetProperty).
+		//
+		// Like OnPluginUnloaded, this is only called for plugins which
+		// OnPluginLoaded was called.
+		virtual void OnPluginWillUnload(IPlugin *plugin)
+		{
+		}
+	};
+
+	static const unsigned int kMinPluginSysApiWithWillUnloadCallback = 8;
 
 	/**
 	 * @brief Manages the runtime loading and unloading of plugins.
@@ -380,6 +406,29 @@ namespace SourceMod
 		 */
 		virtual IPluginIterator *GetPluginIterator() =0;
 
+		/** 
+		 * @brief Adds a V1 plugin manager listener.
+		 *
+		 * @param listener	Pointer to a listener.
+		 */
+		virtual void AddPluginsListener_V1(IPluginsListener_V1 *listener) =0;
+
+		/**
+		 * @brief Removes a V1 plugin listener.
+		 * 
+		 * @param listener	Pointer to a listener.
+		 */
+		virtual void RemovePluginsListener_V1(IPluginsListener_V1 *listener) =0;
+
+		/**
+		 * @brief Converts a Handle to an IPlugin if possible.
+		 *
+		 * @param handle	Handle.
+		 * @param err		Error, set on failure (otherwise undefined).
+		 * @return			IPlugin pointer, or NULL on failure.
+		 */
+		virtual IPlugin *PluginFromHandle(Handle_t handle, HandleError *err) =0;
+
 		/** 
 		 * @brief Adds a plugin manager listener.
 		 *
@@ -393,15 +442,6 @@ namespace SourceMod
 		 * @param listener	Pointer to a listener.
 		 */
 		virtual void RemovePluginsListener(IPluginsListener *listener) =0;
-
-		/**
-		 * @brief Converts a Handle to an IPlugin if possible.
-		 *
-		 * @param handle	Handle.
-		 * @param err		Error, set on failure (otherwise undefined).
-		 * @return			IPlugin pointer, or NULL on failure.
-		 */
-		virtual IPlugin *PluginFromHandle(Handle_t handle, HandleError *err) =0;
 	};
 }