Evict plugins that fail to load.

core/logic/PluginSys.cpp

@@ -63,9 +63,9 @@ CPlugin::CPlugin(const char *file)
    m_LibraryMissing(false),
    m_pContext(nullptr),
    m_MaxClientsVar(nullptr),
-   m_ident(nullptr),
    m_bGotAllLoaded(false),
    m_FileVersion(0),
+   m_ident(nullptr),
    m_LastFileModTime(0),
    m_handle(BAD_HANDLE)
 {
@@ -83,16 +83,7 @@ CPlugin::CPlugin(const char *file)
 
 CPlugin::~CPlugin()
 {
-	if (m_handle)
+	DestroyIdentity();
-	{
-		HandleSecurity sec;
-		sec.pOwner = g_PluginSys.GetIdentity();
-		sec.pIdentity = sec.pOwner;
-
-		handlesys->FreeHandle(m_handle, &sec);
-		g_ShareSys.DestroyIdentity(m_ident);
-	}
-
 	for (size_t i=0; i<m_configs.size(); i++)
 		delete m_configs[i];
 	m_configs.clear();
@@ -100,13 +91,71 @@ CPlugin::~CPlugin()
 
 void CPlugin::InitIdentity()
 {
-	if (!m_handle)
+	if (m_handle)
-	{
+		return;
-		m_ident = g_ShareSys.CreateIdentity(g_PluginIdent, this);
+
-		m_handle = handlesys->CreateHandle(g_PluginType, this, g_PluginSys.GetIdentity(), g_PluginSys.GetIdentity(), NULL);
+	m_ident = g_ShareSys.CreateIdentity(g_PluginIdent, this);
-		m_pRuntime->GetDefaultContext()->SetKey(1, m_ident);
+	m_handle = handlesys->CreateHandle(g_PluginType, this, g_PluginSys.GetIdentity(), g_PluginSys.GetIdentity(), NULL);
-		m_pRuntime->GetDefaultContext()->SetKey(2, (IPlugin *)this);
+	m_pRuntime->GetDefaultContext()->SetKey(1, m_ident);
+	m_pRuntime->GetDefaultContext()->SetKey(2, (IPlugin *)this);
+}
+
+void CPlugin::DestroyIdentity()
+{
+	if (m_handle) {
+		HandleSecurity sec(g_PluginSys.GetIdentity(), g_PluginSys.GetIdentity());
+		handlesys->FreeHandle(m_handle, &sec);
+		m_handle = BAD_HANDLE;
 	}
+	if (m_ident) {
+		g_ShareSys.DestroyIdentity(m_ident);
+		m_ident = nullptr;
+	}
+}
+
+bool CPlugin::IsEvictionCandidate() const
+{
+	switch (Status()) {
+		case Plugin_Running:
+		case Plugin_Loaded:
+			// These states are valid, we should never evict.
+			return false;
+		case Plugin_BadLoad:
+		case Plugin_Uncompiled:
+			// These states imply that the plugin never loaded to begin with,
+			// so we have nothing to evict.
+			return false;
+		case Plugin_Evicted:
+			// We cannot be evicted twice.
+			return false;
+		default:
+			return true;
+	}
+}
+
+void CPlugin::FinishEviction()
+{
+	assert(IsEvictionCandidate());
+
+	// Revoke our identity and handle. This could maybe be seen as bad faith,
+	// since other plugins could be holding the handle and will now error. But
+	// this was already an existing problem. We need a listener API to solve
+	// it.
+	DestroyIdentity();
+
+	// Note that we do not set our status to Plugin_Evicted. This is a pseudo-status
+	// so consumers won't attempt to read the context or runtime. The real state
+	// is reflected here.
+	m_state = PluginState::Evicted;
+	m_pRuntime = nullptr;
+	m_pPhrases = nullptr;
+	m_pContext = nullptr;
+	m_MaxClientsVar = nullptr;
+	m_Props.clear();
+	m_configs.clear();
+	m_Libraries.clear();
+	m_bGotAllLoaded = false;
+	m_FileVersion = 0;
 }
 
 unsigned int CPlugin::CalcMemUsage()
@@ -489,6 +538,15 @@ unsigned int CPlugin::GetSerial()
 
 PluginStatus CPlugin::GetStatus()
 {
+	return Status();
+}
+
+PluginStatus CPlugin::Status() const
+{
+	// Even though we're evicted, we previously guaranteed a valid runtime
+	// for error/fail states. A new failure case above BadLoad solves this.
+	if (m_state == PluginState::Evicted)
+		return Plugin_Evicted;
 	return m_status;
 }
 
@@ -981,6 +1039,13 @@ void CPluginManager::AddPlugin(CPlugin *pPlugin)
 
 	for (ListenerIter iter(m_listeners); !iter.done(); iter.next())
 		(*iter)->OnPluginCreated(pPlugin);
+
+	if (pPlugin->IsEvictionCandidate()) {
+		// If we get here, and the plugin isn't running, we evict it. This
+		// should be safe since our call stack should be empty.
+		Purge(pPlugin);
+		pPlugin->FinishEviction();
+	}
 }
 
 void CPluginManager::LoadAll_SecondPass()
@@ -991,7 +1056,8 @@ void CPluginManager::LoadAll_SecondPass()
 			char error[256] = {0};
 			if (!RunSecondPass(pPlugin)) {
 				g_Logger.LogError("[SM] Unable to load plugin \"%s\": %s", pPlugin->GetFilename(), pPlugin->GetErrorMsg());
-				pPlugin->EvictWithError(Plugin_BadLoad, "%s", error);
+				Purge(pPlugin);
+				pPlugin->FinishEviction();
 			}
 		}
 	}
@@ -1397,7 +1463,6 @@ bool CPluginManager::ScheduleUnload(CPlugin *pPlugin)
 
 void CPluginManager::Purge(CPlugin *plugin)
 {
-	assert(plugin->State() != PluginState::Unregistered);
 
 	// Go through our libraries and tell other plugins they're gone.
 	plugin->LibraryActions(LibraryAction_Removed);
@@ -1424,7 +1489,11 @@ void CPluginManager::UnloadPluginImpl(CPlugin *pPlugin)
 {
 	m_plugins.remove(pPlugin);
 	m_LoadLookup.remove(pPlugin->GetFilename());
-	Purge(pPlugin);
+
+	// Evicted plugins were already purged from external systems.
+	if (pPlugin->State() != PluginState::Evicted)
+		Purge(pPlugin);
+
 	delete pPlugin;
 }
 
@@ -1635,7 +1704,7 @@ void CPluginManager::OnRootConsoleCommand(const char *cmdname, const ICommandArg
 				const sm_plugininfo_t *info = pl->GetPublicInfo();
 				if (pl->GetStatus() != Plugin_Running && !pl->IsSilentlyFailed())
 				{
-					len += ke::SafeSprintf(buffer, sizeof(buffer), "  %02d <%s>", id, GetStatusText(pl->GetStatus()));
+					len += ke::SafeSprintf(buffer, sizeof(buffer), "  %02d <%s>", id, GetStatusText(pl->GetDisplayStatus()));
 
 					/* Plugin has failed to load. */
 					fail_list.append(pl);
@@ -1644,7 +1713,7 @@ void CPluginManager::OnRootConsoleCommand(const char *cmdname, const ICommandArg
 				{
 					len += ke::SafeSprintf(buffer, sizeof(buffer), "  %02d", id);
 				}
-				if (pl->GetStatus() < Plugin_Created)
+				if (pl->GetStatus() < Plugin_Created || pl->GetStatus() == Plugin_Evicted)
 				{
 					if (pl->IsSilentlyFailed())
 						len += ke::SafeSprintf(&buffer[len], sizeof(buffer)-len, " Disabled:");

core/logic/PluginSys.h

@@ -84,6 +84,9 @@ enum class PluginState
 	// The plugin is a member of the global plugin list.
 	Registered,
 
+	// The plugin has been evicted.
+	Evicted,
+
 	// The plugin is waiting to be unloaded.
 	WaitingToUnload
 };
@@ -103,6 +106,7 @@ public:
 	const char *GetFilename();
 	bool IsDebugging();
 	PluginStatus GetStatus();
+	PluginStatus Status() const;
 	bool IsSilentlyFailed();
 	const sm_plugininfo_t *GetPublicInfo();
 	bool SetPauseState(bool paused);
@@ -146,6 +150,7 @@ public:
 public:
 	// Evicts the plugin from memory and sets an error state.
 	void EvictWithError(PluginStatus status, const char *error_fmt, ...);
+	void FinishEviction();
 
 	// Initializes the plugin's identity information
 	void InitIdentity();
@@ -173,6 +178,11 @@ public:
 	void SetRegistered();
 	void SetWaitingToUnload();
 
+	PluginStatus GetDisplayStatus() const {
+		return m_status;
+	}
+	bool IsEvictionCandidate() const;
+
 public:
 	// Returns true if the plugin was running, but is now invalid.
 	bool WasRunning();
@@ -235,11 +245,12 @@ public:
 	void BindFakeNativesTo(CPlugin *other);
 
 protected:
-	bool ReadInfo();
 	void DependencyDropped(CPlugin *pOwner);
 
 private:
 	time_t GetFileTimeStamp();
+	bool ReadInfo();
+	void DestroyIdentity();
 
 private:
 	// This information is static for the lifetime of the plugin.
@@ -264,13 +275,13 @@ private:
 	sp_pubvar_t *m_MaxClientsVar;
 	StringHashMap<void *> m_Props;
 	CVector<AutoConfig *> m_configs;
-	IdentityToken_t *m_ident;
+	List<String> m_Libraries;
 	bool m_bGotAllLoaded;
 	int m_FileVersion;
 
 	// Information that survives past eviction.
 	List<String> m_RequiredLibs;
-	List<String> m_Libraries;
+	IdentityToken_t *m_ident;
 	time_t m_LastFileModTime;
 	Handle_t m_handle;
 	char m_DateTime[256];

public/IPluginSys.h

@@ -38,7 +38,7 @@
 #include <sp_vm_api.h>
 
 #define SMINTERFACE_PLUGINSYSTEM_NAME		"IPluginManager"
-#define SMINTERFACE_PLUGINSYSTEM_VERSION	5
+#define SMINTERFACE_PLUGINSYSTEM_VERSION	6
 
 /** Context user slot 3 is used Core for holding an IPluginContext pointer. */
 #define SM_CONTEXTVAR_USER		3
@@ -96,7 +96,13 @@ namespace SourceMod
 		// @brief The plugin could not be loaded. Either its file was missing
 		// or could not be recognized as a valid SourcePawn binary. Plugins
 		// in this state do not have a context or runtime.
-		Plugin_BadLoad
+		Plugin_BadLoad,
+
+		// @brief The plugin was once in a state <= Created, but has since
+		// been destroyed. We have left its IPlugin instance around for
+		// informational purposes. Plugins in this state do not have a
+		// context or runtime.
+		Plugin_Evicted
 	};
 
 
@@ -275,38 +281,35 @@ namespace SourceMod
 	class IPluginsListener
 	{
 	public:
-		/**
+		// @brief This callback should not be used since plugins may be in
-		 * @brief Called when a plugin is created/mapped into memory.
+		// an unusable state.
-		 */
+		virtual void OnPluginCreated(IPlugin *plugin) final
-		virtual void OnPluginCreated(IPlugin *plugin)
 		{
 		}
 
-		/**
+		// @brief Called when a plugin's required dependencies and natives have
-		 * @brief Called when a plugin is fully loaded successfully.
+		// been bound. Plugins at this phase may be in any state Failed or
-		 */
+		// lower. This is invoked immediately before OnPluginStart, and sometime
+		// after OnPluginCreated.
 		virtual void OnPluginLoaded(IPlugin *plugin)
 		{
 		}
 
-		/**
+		// @brief Called when a plugin is paused or unpaused.
-		* @brief Called when a plugin is paused or unpaused.
-		*/
 		virtual void OnPluginPauseChange(IPlugin *plugin, bool paused)
 		{
 		}
 
-		/**
+		// @brief Called when a plugin is about to be unloaded. This is called for
-		 * @brief Called when a plugin is unloaded (only if fully loaded).
+		// any plugin for which OnPluginLoaded was called, and is invoked
-		 */
+		// immediately after OnPluginEnd(). The plugin may be in any state Failed
+		// or lower.
 		virtual void OnPluginUnloaded(IPlugin *plugin)
 		{
 		}
 
-		/**
+		// @brief Called when a plugin is destroyed. This is called on all plugins for
-		 * @brief Called when a plugin is destroyed.
+		// which OnPluginCreated was called. The plugin may be in any state.
-		 * NOTE: Always called if Created, even if load failed.
-		 */
 		virtual void OnPluginDestroyed(IPlugin *plugin)
 		{
 		}