22367: Fixes deadlock issue with entity writes

src/Amalgam/entity/Entity.h

@@ -668,16 +668,16 @@ class Entity
 		EvaluableNodeManager::EvaluableNodeMetadataModifier metadata_modifier = EvaluableNodeManager::ENMM_NO_CHANGE,
 		std::vector<EntityWriteListener *> *write_listeners = nullptr);
 
-	//collects garbage on evaluableNodeManager
+	//collects garbage on evaluableNodeManager, assuming it has a write reference
 #ifdef MULTITHREAD_SUPPORT
-	//if multithreaded, then memory_modification_lock is the lock used for memoryModificationMutex
-	__forceinline void CollectGarbage(Concurrency::ReadLock *memory_modification_lock)
+	__forceinline void CollectGarbageWithEntityWriteReference()
 	{
-		if(evaluableNodeManager.RecommendGarbageCollection())
-			evaluableNodeManager.CollectGarbage(memory_modification_lock);
+		if(evaluableNodeManager.RecommendGarbageCollection()
+				&& !evaluableNodeManager.IsAnyNodeReferencedOtherThanRoot())
+			evaluableNodeManager.CollectGarbage();
 	}
 #else
-	__forceinline void CollectGarbage()
+	__forceinline void CollectGarbageWithEntityWriteReference()
 	{
 		if(evaluableNodeManager.RecommendGarbageCollection())
 			evaluableNodeManager.CollectGarbage();

src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp

@@ -79,11 +79,7 @@ void EvaluableNodeManager::UpdateGarbageCollectionTrigger(size_t previous_num_no
 	numNodesToRunGarbageCollection = std::max(max_from_allocation, std::max<size_t>(max_from_previous, max_from_current));
 }
 
-#ifdef MULTITHREAD_SUPPORT
-void EvaluableNodeManager::CollectGarbage(Concurrency::ReadLock *memory_modification_lock)
-#else
 void EvaluableNodeManager::CollectGarbage()
-#endif
 {
 	if(PerformanceProfiler::IsProfilingEnabled())
 	{
@@ -93,7 +89,24 @@ void EvaluableNodeManager::CollectGarbage()
 
 	ClearThreadLocalAllocationBuffer();
 
+	MarkAllReferencedNodesInUse(firstUnusedNodeIndex);
+
+	FreeAllNodesExceptReferencedNodes(firstUnusedNodeIndex);
+
+	if(PerformanceProfiler::IsProfilingEnabled())
+		PerformanceProfiler::EndOperation(GetNumberOfUsedNodes());
+}
+
 #ifdef MULTITHREAD_SUPPORT
+void EvaluableNodeManager::CollectGarbageWithConcurrentAccess(Concurrency::ReadLock *memory_modification_lock)
+{
+	if(PerformanceProfiler::IsProfilingEnabled())
+	{
+		static const std::string collect_garbage_string = ".collect_garbage";
+		PerformanceProfiler::StartOperation(collect_garbage_string, GetNumberOfUsedNodes());
+	}
+
+	ClearThreadLocalAllocationBuffer();
 
 	//free lock so can attempt to enter write lock to collect garbage
 	if(memory_modification_lock != nullptr)
@@ -112,7 +125,6 @@ void EvaluableNodeManager::CollectGarbage()
 	{
 		if(RecommendGarbageCollection())
 		{
-#endif
 			size_t cur_first_unused_node_index = firstUnusedNodeIndex;
 			//clear firstUnusedNodeIndex to signal to other threads that they won't need to do garbage collection
 			firstUnusedNodeIndex = 0;
@@ -122,12 +134,9 @@ void EvaluableNodeManager::CollectGarbage()
 					&& nodes[cur_first_unused_node_index - 1]->IsNodeDeallocated())
 				cur_first_unused_node_index--;
 
-			//set to contain everything that is referenced
 			MarkAllReferencedNodesInUse(cur_first_unused_node_index);
 
 			FreeAllNodesExceptReferencedNodes(cur_first_unused_node_index);
-
-#ifdef MULTITHREAD_SUPPORT
 		}
 
 		//free the unique lock and reacquire the shared lock
@@ -136,11 +145,11 @@ void EvaluableNodeManager::CollectGarbage()
 
 	if(memory_modification_lock != nullptr)
 		memory_modification_lock->lock();
-#endif
 
 	if(PerformanceProfiler::IsProfilingEnabled())
 		PerformanceProfiler::EndOperation(GetNumberOfUsedNodes());
 }
+#endif
 
 void EvaluableNodeManager::FreeAllNodes()
 {

src/Amalgam/evaluablenode/EvaluableNodeManagement.h

@@ -719,12 +719,12 @@ class EvaluableNodeManager
 	//updates the memory threshold when garbage collection will be next called
 	void UpdateGarbageCollectionTrigger(size_t previous_num_nodes = 0);
 
-	//runs heuristics and collects garbage
+	//collects garbage
+	void CollectGarbage();
+
 #ifdef MULTITHREAD_SUPPORT
 	//if multithreaded, then memory_modification_lock is the lock used for memoryModificationMutex if not nullptr
-	void CollectGarbage(Concurrency::ReadLock *memory_modification_lock);
-#else
-	void CollectGarbage();
+	void CollectGarbageWithConcurrentAccess(Concurrency::ReadLock *memory_modification_lock);
 #endif
 
 	//frees an EvaluableNode (must be owned by this EvaluableNodeManager)

src/Amalgam/interpreter/Interpreter.h

@@ -208,7 +208,7 @@ class Interpreter
 		if(evaluableNodeManager->RecommendGarbageCollection())
 		{
 		#ifdef MULTITHREAD_SUPPORT
-			evaluableNodeManager->CollectGarbage(&memoryModificationLock);
+			evaluableNodeManager->CollectGarbageWithConcurrentAccess(&memoryModificationLock);
 		#else
 			evaluableNodeManager->CollectGarbage();
 		#endif

src/Amalgam/interpreter/InterpreterOpcodesEntityAccess.cpp

@@ -293,13 +293,8 @@ EvaluableNodeReference Interpreter::InterpretNode_ENT_ASSIGN_TO_ENTITIES_and_DIR
 				VerifyEvaluableNodeIntegrity();
 			#endif
 
-				//collect garbage, but not on current entity, save that for between instructions
-			#ifdef MULTITHREAD_SUPPORT
-				target_entity->CollectGarbage(&memoryModificationLock);
-			#else
-				target_entity->CollectGarbage();
-			#endif
-
+			target_entity->CollectGarbageWithEntityWriteReference();
+
 			#ifdef AMALGAM_MEMORY_INTEGRITY
 				VerifyEvaluableNodeIntegrity();
 			#endif

src/Amalgam/interpreter/InterpreterOpcodesEntityControl.cpp

@@ -197,11 +197,7 @@ EvaluableNodeReference Interpreter::InterpretNode_ENT_ASSIGN_ENTITY_ROOTS_and_AC
 			}
 		}
 
-	#ifdef MULTITHREAD_SUPPORT
-		target_entity->CollectGarbage(&memoryModificationLock);
-	#else
-		target_entity->CollectGarbage();
-	#endif
+		target_entity->CollectGarbageWithEntityWriteReference();
 	}
 
 	return AllocReturn(all_assignments_successful, immediate_result);