From 0aaa659e29ae73c5333a26fe4d25c5cbfa364a89 Mon Sep 17 00:00:00 2001
From: David Anderson <dvander@alliedmods.net>
Date: Fri, 14 Aug 2015 18:19:36 -0700
Subject: [PATCH] Fix how the mark-serial is used.

The mark-serial is a generation number to optimize dependency tracking. It did not actually get
applied correctly, meaning that in rare cases we could miss dependencies. This patch removes the
incorrect serial propagation and ensures that we don't double-count a dependent plugin.

Additionally, this patch ensures that all callers of BindNativeToPlugin() will update the mark
serial, as is required to correctly track dependencies.
---
 core/logic/NativeOwner.cpp | 18 ++----------------
 core/logic/NativeOwner.h   |  1 -
 core/logic/PluginSys.cpp   |  1 +
 core/logic/ShareSys.cpp    | 31 ++++++++++++++-----------------
 core/logic/ShareSys.h      |  1 +
 5 files changed, 18 insertions(+), 34 deletions(-)

diff --git a/core/logic/NativeOwner.cpp b/core/logic/NativeOwner.cpp
index e6adcb2e..a4b092e6 100644
--- a/core/logic/NativeOwner.cpp
+++ b/core/logic/NativeOwner.cpp
@@ -48,7 +48,8 @@ unsigned int CNativeOwner::GetMarkSerial()
 
 void CNativeOwner::AddDependent(CPlugin *pPlugin)
 {
-	m_Dependents.push_back(pPlugin);
+	if (m_Dependents.find(pPlugin) == m_Dependents.end())
+		m_Dependents.push_back(pPlugin);
 }
 
 void CNativeOwner::AddWeakRef(const WeakNative & ref)
@@ -64,23 +65,8 @@ void CNativeOwner::AddNatives(const sp_nativeinfo_t *natives)
 	m_natives.append(natives);
 }
 
-void CNativeOwner::PropagateMarkSerial(unsigned int serial)
-{
-	CNativeOwner *pOwner;
-	List<CPlugin *>::iterator iter;
-
-	for (iter = m_Dependents.begin();
-		 iter != m_Dependents.end();
-		 iter++)
-	{
-		pOwner = (*iter);
-		pOwner->SetMarkSerial(serial);
-	}
-}
-
 void CNativeOwner::UnbindWeakRef(const WeakNative &ref)
 {
-	sp_native_t *native;
 	IPluginContext *pContext;
 
 	pContext = ref.pl->GetBaseContext();
diff --git a/core/logic/NativeOwner.h b/core/logic/NativeOwner.h
index e8037827..f55d954b 100644
--- a/core/logic/NativeOwner.h
+++ b/core/logic/NativeOwner.h
@@ -69,7 +69,6 @@ public:
 public:
 	void SetMarkSerial(unsigned int serial);
 	unsigned int GetMarkSerial();
-	void PropagateMarkSerial(unsigned int serial);
 public:
 	void AddDependent(CPlugin *pPlugin);
 	void AddWeakRef(const WeakNative & ref);
diff --git a/core/logic/PluginSys.cpp b/core/logic/PluginSys.cpp
index 0d2edf92..dfcfa665 100644
--- a/core/logic/PluginSys.cpp
+++ b/core/logic/PluginSys.cpp
@@ -1381,6 +1381,7 @@ bool CPluginManager::RunSecondPass(CPlugin *pPlugin, char *error, size_t maxleng
 					  || pOther->GetStatus() == Plugin_Paused)
 					 && pOther != pPlugin)
 			{
+				g_ShareSys.BeginBindingFor(pPlugin);
 				for (size_t i = 0; i < pPlugin->m_fakes.length(); i++)
 					g_ShareSys.BindNativeToPlugin(pOther, pPlugin->m_fakes[i]);
 			}
diff --git a/core/logic/ShareSys.cpp b/core/logic/ShareSys.cpp
index 16067b43..3119f36c 100644
--- a/core/logic/ShareSys.cpp
+++ b/core/logic/ShareSys.cpp
@@ -269,19 +269,19 @@ PassRef<Native> ShareSystem::FindNative(const char *name)
 	return *r;
 }
 
+void ShareSystem::BeginBindingFor(CPlugin *pPlugin)
+{
+	g_mark_serial++;
+}
+
 void ShareSystem::BindNativesToPlugin(CPlugin *pPlugin, bool bCoreOnly)
 {
-	uint32_t i, native_count;
-	IPluginContext *pContext;
+	IPluginContext *pContext = pPlugin->GetBaseContext();
 
-	pContext = pPlugin->GetBaseContext();
+	BeginBindingFor(pPlugin);
 
-	/* Generate a new serial ID, mark our dependencies with it. */
-	g_mark_serial++;
-	pPlugin->PropagateMarkSerial(g_mark_serial);
-
-	native_count = pContext->GetNativesNum();
-	for (i = 0; i < native_count; i++)
+	uint32_t native_count = pContext->GetNativesNum();
+	for (uint32_t i = 0; i < native_count; i++)
 	{
 		const sp_native_t *native = pContext->GetRuntime()->GetNative(i);
 		if (!native)
@@ -349,15 +349,12 @@ void ShareSystem::BindNativeToPlugin(CPlugin *pPlugin, const sp_native_t *native
 		/* Otherwise, we're a strong dependent and not a weak one */
 		else
 		{
-			/* See if this has already been marked as a dependent.
-			 * If it has, it means this relationship has already occurred, 
-			 * and there is no reason to do it again.
-			 */
-			if (pEntry->owner != pPlugin->ToNativeOwner() 
-				&& pEntry->owner->GetMarkSerial() != g_mark_serial)
+			// If this plugin is not binding to itself, and it hasn't been marked as a
+			// dependency already, then add it now. We use the mark serial to track
+			// which plugins we already consider a dependency.
+			if (pEntry->owner != pPlugin->ToNativeOwner() &&
+			    pEntry->owner->GetMarkSerial() != g_mark_serial)
 			{
-				/* This has not been marked as a dependency yet */
-				//pPlugin->AddDependency(pEntry->owner);
 				pEntry->owner->AddDependent(pPlugin);
 				pEntry->owner->SetMarkSerial(g_mark_serial);
 			}
diff --git a/core/logic/ShareSys.h b/core/logic/ShareSys.h
index 456e2c59..18eaf956 100644
--- a/core/logic/ShareSys.h
+++ b/core/logic/ShareSys.h
@@ -119,6 +119,7 @@ public:
 		return &m_IdentRoot;
 	}
 public:
+	void BeginBindingFor(CPlugin *pPlugin);
 	void BindNativesToPlugin(CPlugin *pPlugin, bool bCoreOnly);
 	void BindNativeToPlugin(CPlugin *pPlugin, const ke::Ref<Native> &pEntry);
 	ke::PassRef<Native> AddFakeNative(IPluginFunction *pFunc, const char *name, SPVM_FAKENATIVE_FUNC func);