22324: Fixes crash involving EvaluableNodeManager allocations with ad…

…dress collisions wrt TLAB (#311)

Co-authored-by: Andrew Bassett <[email protected]>

src/Amalgam/AssetManager.cpp

@@ -333,7 +333,11 @@ Entity *AssetManager::LoadEntityFromResource(AssetParameters &asset_params, bool
 		}
 
 		if(persistent)
-			SetEntityPersistenceForFlattenedEntity(new_entity, &asset_params);
+		{
+			delete new_entity;
+			return nullptr;
+		}
+			//SetEntityPersistenceForFlattenedEntity(new_entity, &asset_params);
 
 		return new_entity;
 	}
@@ -358,7 +362,11 @@ Entity *AssetManager::LoadEntityFromResource(AssetParameters &asset_params, bool
 		new_entity->evaluableNodeManager.FreeNode(call_stack);
 
 		if(persistent)
-			SetEntityPersistenceForFlattenedEntity(new_entity, &asset_params);
+		{
+			delete new_entity;
+			return nullptr;
+		}
+		//	SetEntityPersistenceForFlattenedEntity(new_entity, &asset_params);
 
 		return new_entity;
 	}

src/Amalgam/evaluablenode/EvaluableNode.h

@@ -19,7 +19,6 @@
 #define AMALGAM_FAST_MEMORY_INTEGRITY
 #endif
 
-
 //forward declarations:
 class EvaluableNodeManager;
 

src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp

@@ -15,6 +15,9 @@ const double EvaluableNodeManager::allocExpansionFactor = 1.5;
 
 EvaluableNodeManager::~EvaluableNodeManager()
 {
+	if(lastEvaluableNodeManager == this)
+		ClearThreadLocalAllocationBuffer();
+
 #ifdef MULTITHREAD_SUPPORT
 	Concurrency::WriteLock lock(managerAttributesMutex);
 #endif
@@ -277,13 +280,12 @@ EvaluableNode *EvaluableNodeManager::AllocUninitializedNode()
 
 #ifdef MULTITHREAD_SUPPORT
 	{
-
 		//slow path allocation; attempt to allocate using an atomic without write locking
 		Concurrency::ReadLock lock(managerAttributesMutex);
 
 		//attempt to allocate enough nodes to refill thread local buffer
-		size_t first_index_to_allocate = firstUnusedNodeIndex.fetch_add(tlabSize);
-		size_t last_index_to_allocate = first_index_to_allocate + tlabSize;
+		size_t first_index_to_allocate = firstUnusedNodeIndex.fetch_add(tlabBlockAllocationSize);
+		size_t last_index_to_allocate = first_index_to_allocate + tlabBlockAllocationSize;
 
 		if(last_index_to_allocate < nodes.size())
 		{
@@ -299,7 +301,7 @@ EvaluableNode *EvaluableNodeManager::AllocUninitializedNode()
 		}
 
 		//couldn't allocate enough valid nodes; reset index and allocate more
-		firstUnusedNodeIndex -= tlabSize;
+		firstUnusedNodeIndex -= tlabBlockAllocationSize;
 		ClearThreadLocalAllocationBuffer();
 	}
 	//don't have enough nodes, so need to attempt a write lock to allocate more

src/Amalgam/evaluablenode/EvaluableNodeManagement.h

@@ -1100,11 +1100,10 @@ class EvaluableNodeManager
 #if defined(MULTITHREAD_SUPPORT) || defined(MULTITHREAD_INTERFACE)
 	thread_local
 #endif
-
-	// We need to keep track of the the last EvaluableNodeManager that accessed 
-	// the thread local allocation buffer for a given thread. We do this so 
-	// a given thread local allocation buffer has only nodes associated with one manager.
-	// If a different manager accesses the buffer, we clear the buffer to maintain this invariant.
+	// Keeps track of the the last EvaluableNodeManager that accessed 
+	// the thread local allocation buffer for a each thread.
+	// A given thread local allocation buffer should only have nodes associated with one manager.
+	// If a different manager accesses the buffer, it is cleared to maintain this invariant.
 	static inline EvaluableNodeManager *lastEvaluableNodeManager;
 
 	// Get a pointer to the next available node from the thread local allocation buffer.
@@ -1125,7 +1124,6 @@ class EvaluableNodeManager
 			lastEvaluableNodeManager = this;
 			return nullptr;
 		}
-
 	}
 
 	// Adds a node to the thread local allocation buffer.
@@ -1134,7 +1132,9 @@ class EvaluableNodeManager
 	// before adding the node.
 	inline void AddNodeToTLab(EvaluableNode *en)
 	{
+	#ifdef AMALGAM_FAST_MEMORY_INTEGRITY
 		assert(en->IsNodeDeallocated());
+	#endif
 
 		if(this != lastEvaluableNodeManager)
 		{
@@ -1147,14 +1147,13 @@ class EvaluableNodeManager
 
 private:
 
-	static const int tlabSize = 20;
-
-	typedef std::vector<EvaluableNode *> TLab;
+	//number of nodes to allocate at once for the thread local allocation buffer
+	static const int tlabBlockAllocationSize = 20;
 
 	#if defined(MULTITHREAD_SUPPORT) || defined(MULTITHREAD_INTERFACE)
 		thread_local
 	#endif
-		// This buffer holds EvaluableNode*'s reserved for allocation by a specific thread
-		inline static TLab threadLocalAllocationBuffer;
+		// Holds EvaluableNode*'s reserved for allocation by a specific thread
+		inline static std::vector<EvaluableNode *> threadLocalAllocationBuffer;
 
 };

src/Amalgam/interpreter/Interpreter.h

@@ -709,6 +709,7 @@ class Interpreter
 				current_value, &result, results_saver_location]
 				{
 					EvaluableNodeManager *enm = parentInterpreter->evaluableNodeManager;
+					EvaluableNodeManager::ClearThreadLocalAllocationBuffer();
 
 					Interpreter interpreter(parentInterpreter->evaluableNodeManager, rand_seed,
 						parentInterpreter->writeListeners, parentInterpreter->printListener,
@@ -776,6 +777,7 @@ class Interpreter
 				[this, rand_seed, node_to_execute, result, immediate_results, results_saver_location]
 				{
 					EvaluableNodeManager *enm = parentInterpreter->evaluableNodeManager;
+					EvaluableNodeManager::ClearThreadLocalAllocationBuffer();
 
 					Interpreter interpreter(parentInterpreter->evaluableNodeManager, rand_seed,
 						parentInterpreter->writeListeners, parentInterpreter->printListener,