From 9f1d9042788d08d977c2a776c807b7950df9f8ff Mon Sep 17 00:00:00 2001
From: Ykkrosh <philip@wildfiregames.com>
Date: Sun, 18 Apr 2010 16:35:06 +0000
Subject: [PATCH] # Fix use-of-freed-memory bug.

Don't call xmlCleanupParser after parsing documents.
("This function name is somewhat misleading. It does not clean up parser
state, it cleans up memory allocated by the library itself.")

This was SVN commit r7465.
---
 source/collada/StdSkeletons.cpp               |  2 --
 source/graphics/tests/test_MeshManager.h      | 20 ++++++++++++
 .../tests/test_CmpTemplateManager.h           | 31 ++++++++++++++++++-
 3 files changed, 50 insertions(+), 3 deletions(-)

diff --git a/source/collada/StdSkeletons.cpp b/source/collada/StdSkeletons.cpp
index 8665b3f51e..78b137d9ff 100644
--- a/source/collada/StdSkeletons.cpp
+++ b/source/collada/StdSkeletons.cpp
@@ -208,14 +208,12 @@ void Skeleton::LoadSkeletonDataFromXml(const char* xmlData, size_t xmlLength, st
 			xmlFreeDoc(doc);
 			doc = NULL;
 		}
-		xmlCleanupParser();
 		xmlSetGenericErrorFunc(NULL, NULL);
 	}
 	catch (const ColladaException&)
 	{
 		if (doc)
 			xmlFreeDoc(doc);
-		xmlCleanupParser();
 		xmlSetGenericErrorFunc(NULL, NULL);
 		throw;
 	}
diff --git a/source/graphics/tests/test_MeshManager.h b/source/graphics/tests/test_MeshManager.h
index 225b021246..96ce64742c 100644
--- a/source/graphics/tests/test_MeshManager.h
+++ b/source/graphics/tests/test_MeshManager.h
@@ -25,6 +25,7 @@
 #include "graphics/ModelDef.h"
 
 #include "ps/CLogger.h"
+#include "ps/XML/RelaxNG.h"
 
 static fs::wpath MOD_PATH(DataDir()/L"mods/_test.mesh");
 static fs::wpath CACHE_PATH(DataDir()/L"_testcache");
@@ -215,6 +216,25 @@ public:
 		TS_ASSERT(! modeldef);
 	}
 
+	void test_load_across_relaxng()
+	{
+		// Verify that loading meshes doesn't invalidate other users of libxml2 by calling xmlCleanupParser
+		// (Run this in Valgrind and check for use-of-freed-memory errors)
+
+		RelaxNGValidator v;
+		TS_ASSERT(v.LoadGrammar("<element xmlns='http://relaxng.org/ns/structure/1.0' datatypeLibrary='http://www.w3.org/2001/XMLSchema-datatypes' name='test'><data type='decimal'/></element>"));
+		TS_ASSERT(v.Validate(L"doc", L"<test>2.0</test>"));
+
+		copyFile(srcDAE, testDAE);
+		copyFile(srcSkeletonDefs, testSkeletonDefs);
+		CModelDefPtr modeldef = meshManager->GetMesh(testDAE);
+		TS_ASSERT(modeldef);
+		if (modeldef) TS_ASSERT_WSTR_EQUALS(modeldef->GetName().string(), testBase);
+
+		TS_ASSERT(v.Validate(L"doc", L"<test>2.0</test>"));
+	}
+
+
 	//////////////////////////////////////////////////////////////////////////
 
 	// Tests based on real DAE files:
diff --git a/source/simulation2/tests/test_CmpTemplateManager.h b/source/simulation2/tests/test_CmpTemplateManager.h
index 98c372e562..391807553a 100644
--- a/source/simulation2/tests/test_CmpTemplateManager.h
+++ b/source/simulation2/tests/test_CmpTemplateManager.h
@@ -24,7 +24,9 @@
 #include "simulation2/MessageTypes.h"
 #include "simulation2/system/ParamNode.h"
 #include "simulation2/system/SimContext.h"
+#include "simulation2/Simulation2.h"
 
+#include "graphics/Terrain.h"
 #include "ps/Filesystem.h"
 #include "ps/CLogger.h"
 #include "ps/XML/Xeromyces.h"
@@ -35,7 +37,6 @@ public:
 	void setUp()
 	{
 		g_VFS = CreateVfs(20 * MiB);
-		TS_ASSERT_OK(g_VFS->Mount(L"", DataDir()/L"mods/_test.sim"));
 		CXeromyces::Startup();
 	}
 
@@ -47,6 +48,8 @@ public:
 
 	void test_LoadTemplate()
 	{
+		TS_ASSERT_OK(g_VFS->Mount(L"", DataDir()/L"mods/_test.sim"));
+
 		CSimContext context;
 		CComponentManager man(context);
 		man.LoadComponentTypes();
@@ -90,6 +93,8 @@ public:
 
 	void test_LoadTemplate_errors()
 	{
+		TS_ASSERT_OK(g_VFS->Mount(L"", DataDir()/L"mods/_test.sim"));
+
 		CSimContext context;
 		CComponentManager man(context);
 		man.LoadComponentTypes();
@@ -121,6 +126,8 @@ public:
 
 	void test_LoadTemplate_multiple()
 	{
+		TS_ASSERT_OK(g_VFS->Mount(L"", DataDir()/L"mods/_test.sim"));
+
 		CSimContext context;
 		CComponentManager man(context);
 		man.LoadComponentTypes();
@@ -156,4 +163,26 @@ public:
 		TS_ASSERT(tempMan->LoadTemplate(ent2, L"inherit-broken", -1) == NULL);
 		TS_ASSERT(tempMan->LoadTemplate(ent2, L"inherit-broken", -1) == NULL);
 	}
+
+	void test_load_all_DISABLED() // disabled since it's a bit slow
+	{
+		TS_ASSERT_OK(g_VFS->Mount(L"", DataDir()/L"mods/public"));
+
+		CTerrain dummy;
+		CSimulation2 sim(NULL, &dummy);
+		sim.LoadDefaultScripts();
+		sim.ResetState();
+
+		CmpPtr<ICmpTemplateManager> cmpTempMan(sim, SYSTEM_ENTITY);
+		TS_ASSERT(!cmpTempMan.null());
+
+		std::vector<std::wstring> templates = cmpTempMan->FindAllTemplates();
+		for (size_t i = 0; i < templates.size(); ++i)
+		{
+			std::wstring name = templates[i];
+			printf("# %ls\n", name.c_str());
+			const CParamNode* p = cmpTempMan->GetTemplate(name);
+			TS_ASSERT(p != NULL);
+		}
+	}
 };