22339: Fixes segfault bug (#314)

src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp

@@ -62,119 +62,6 @@ EvaluableNode *EvaluableNodeManager::AllocNode(EvaluableNode *original, Evaluabl
 	return n;
 }
 
-
-void InitializeListHeadOrNode(EvaluableNode *node, EvaluableNode *parent, EvaluableNodeType child_node_type,  size_t node_index, std::vector<EvaluableNode*> *ocn_buffer)
-{
-	if(node_index == 0)
-	{
-		// parent
-		node->InitializeType(ENT_LIST);
-		std::vector<EvaluableNode *> *ocn_ptr = &node->GetOrderedChildNodesReference();
-		std::swap(*ocn_buffer, *ocn_ptr);
-	}
-	else
-	{
-		// child node; initialize it and add it to the list items
-		auto &ocn = parent->GetOrderedChildNodesReference();
-		ocn[node_index-1] = node;
-		node->InitializeType(child_node_type);
-	}
-}
-
-EvaluableNode *EvaluableNodeManager::AllocListNodeWithOrderedChildNodes(EvaluableNodeType child_node_type, size_t num_child_nodes)
-{
-	if(num_child_nodes == 0)
-		return AllocNode(ENT_LIST);
-
-	EvaluableNode *parent = nullptr;
-	// Allocate from TLab
-
-	size_t num_to_alloc = 1 + num_child_nodes;
-	size_t num_allocated = 0;
-	size_t num_total_nodes_needed = 0;
-
-	//ordered child nodes destination; preallocate outside of the lock (for performance) and swap in
-	std::vector<EvaluableNode *> ocn_buffer;
-	ocn_buffer.resize(num_child_nodes);
-
-	while(num_allocated < num_to_alloc)
-	{
-		EvaluableNode *newNode = nullptr;
-
-		while((newNode = GetNextNodeFromTLab()) && num_allocated < num_to_alloc)
-		{
-			if(parent == nullptr)
-				parent = newNode;
-
-			InitializeListHeadOrNode(newNode, parent, child_node_type, num_allocated, &ocn_buffer);
-			num_allocated++;
-		}
-
-		if(num_allocated >= num_to_alloc)
-		{
-			//we got enough nodes out of the tlab
-			return parent;
-		}
-
-		{ // Not enough nodes in TLab; add some.
-			#ifdef MULTITHREAD_SUPPORT
-				Concurrency::ReadLock lock(managerAttributesMutex);
-			#endif
-
-			int num_added_to_tlab = 0;
-			for(; num_allocated + num_added_to_tlab < num_to_alloc; num_added_to_tlab++)
-			{
-				size_t allocated_index = firstUnusedNodeIndex++;
-				if(allocated_index < nodes.size())
-				{
-					if(nodes[allocated_index] == nullptr)
-						nodes[allocated_index] = new EvaluableNode(ENT_DEALLOCATED);
-
-
-					AddNodeToTLab(nodes[allocated_index]);
-				}
-				else
-				{
-					//the node wasn't valid; put it back and do a write lock to allocate more
-					--firstUnusedNodeIndex;
-					break;
-				}
-			}
-
-			// If we added enough nodes to the tlab, use them in the next loop iteration
-			if( num_added_to_tlab + num_allocated >= num_to_alloc)
-				continue;
-		}
-
-
-		// There weren't enough free nodes available to fill the tlab; allocate more
-		num_total_nodes_needed = firstUnusedNodeIndex + (num_to_alloc - num_allocated);
-		{
-			#ifdef MULTITHREAD_SUPPORT
-			//don't have enough nodes, so need to attempt a write lock to allocate more
-			Concurrency::WriteLock write_lock(managerAttributesMutex);
-
-			//try again after write lock to allocate a node in case another thread has performed the allocation
-			//already have the write lock, so don't need to worry about another thread stealing firstUnusedNodeIndex
-			#endif
-
-
-			//if don't currently have enough free nodes to meet the needs, then expand the allocation
-			if(nodes.size() <= num_total_nodes_needed)
-			{
-				size_t new_num_nodes = static_cast<size_t>(allocExpansionFactor * num_total_nodes_needed) + 1;
-
-				//fill new EvaluableNode slots with nullptr
-				nodes.resize(new_num_nodes, nullptr);
-			}
-		}
-	}
-
-	// unreachable 
-	assert(false);
-	return nullptr;
-}
-
 void EvaluableNodeManager::UpdateGarbageCollectionTrigger(size_t previous_num_nodes)
 {
 	//scale down the number of nodes previously allocated, because there is always a chance that
@@ -297,12 +184,12 @@ EvaluableNode *EvaluableNodeManager::AllocUninitializedNode()
 				AddNodeToTLab(nodes[i]);
 			}
 
+			lock.unlock();
 			return GetNextNodeFromTLab();
 		}
 
 		//couldn't allocate enough valid nodes; reset index and allocate more
 		firstUnusedNodeIndex -= tlabBlockAllocationSize;
-		ClearThreadLocalAllocationBuffer();
 	}
 	//don't have enough nodes, so need to attempt a write lock to allocate more
 	Concurrency::WriteLock write_lock(managerAttributesMutex);

src/Amalgam/evaluablenode/EvaluableNodeManagement.h

@@ -442,10 +442,6 @@ class EvaluableNodeManager
 		return n;
 	}
 
-	//allocates and returns a node of type ENT_LIST
-	// and allocates num_child_nodes child nodes initialized to child_node_type (with an appropriate default value)
-	EvaluableNode *AllocListNodeWithOrderedChildNodes(EvaluableNodeType child_node_type, size_t num_child_nodes);
-
 	//ways that labels can be modified when a new node is allocated
 	enum EvaluableNodeMetadataModifier
 	{
@@ -992,6 +988,8 @@ class EvaluableNodeManager
 	inline static void ClearThreadLocalAllocationBuffer()
 	{
 		threadLocalAllocationBuffer.clear();
+		//set to null so nothing matches until more nodes are added
+		lastEvaluableNodeManager = nullptr;
 	}
 
 protected:
@@ -1112,16 +1110,17 @@ class EvaluableNodeManager
 	{
 		if(threadLocalAllocationBuffer.size() > 0 && this == lastEvaluableNodeManager)
 		{
-			EvaluableNode *end = threadLocalAllocationBuffer.back();
+			EvaluableNode *node = threadLocalAllocationBuffer.back();
 			threadLocalAllocationBuffer.pop_back();
-			return end;
+			return node;
 		}
 		else
 		{
 			if(lastEvaluableNodeManager != this)
 				ClearThreadLocalAllocationBuffer();
 
-			lastEvaluableNodeManager = this;
+			//set to null so nothing matches until more nodes are added
+			lastEvaluableNodeManager = nullptr;
 			return nullptr;
 		}
 	}
@@ -1155,5 +1154,4 @@ class EvaluableNodeManager
 	#endif
 		// Holds EvaluableNode*'s reserved for allocation by a specific thread
 		inline static std::vector<EvaluableNode *> threadLocalAllocationBuffer;
-
 };

src/Amalgam/evaluablenode/EvaluableNodeTreeFunctions.h

@@ -414,12 +414,13 @@ template<typename ValueContainer, typename GetNumberFunction>
 inline EvaluableNodeReference CreateListOfNumbersFromIteratorAndFunction(ValueContainer &value_container,
 	EvaluableNodeManager *enm, GetNumberFunction get_number)
 {
-	EvaluableNode *list = enm->AllocListNodeWithOrderedChildNodes(ENT_NUMBER, value_container.size());
+	EvaluableNode *list = enm->AllocNode(ENT_LIST);
 	auto &ocn = list->GetOrderedChildNodesReference();
+	ocn.resize(value_container.size());
 
 	size_t index = 0;
 	for(auto value_element : value_container)
-		ocn[index++]->SetTypeViaNumberValue(get_number(value_element));
+		ocn[index++] = enm->AllocNode(get_number(value_element));
 
 	return EvaluableNodeReference(list, true);
 }
@@ -430,12 +431,13 @@ template<typename StringContainer, typename GetStringFunction>
 inline EvaluableNodeReference CreateListOfStringsIdsFromIteratorAndFunction(StringContainer &string_container,
 	EvaluableNodeManager *enm, GetStringFunction get_string_id)
 {
-	EvaluableNode *list = enm->AllocListNodeWithOrderedChildNodes(ENT_STRING, string_container.size());
+	EvaluableNode *list = enm->AllocNode(ENT_LIST);
 	auto &ocn = list->GetOrderedChildNodesReference();
+	ocn.resize(string_container.size());
 
 	size_t index = 0;
 	for(auto string_element : string_container)
-		ocn[index++]->SetTypeViaStringIdValue(get_string_id(string_element));
+		ocn[index++] = enm->AllocNode(ENT_STRING, get_string_id(string_element));
 
 	return EvaluableNodeReference(list, true);
 }
@@ -446,12 +448,13 @@ template<typename StringContainer, typename GetStringFunction>
 inline EvaluableNodeReference CreateListOfStringsFromIteratorAndFunction(StringContainer &string_container,
 	EvaluableNodeManager *enm, GetStringFunction get_string)
 {
-	EvaluableNode *list = enm->AllocListNodeWithOrderedChildNodes(ENT_STRING, string_container.size());
+	EvaluableNode *list = enm->AllocNode(ENT_LIST);
 	auto &ocn = list->GetOrderedChildNodesReference();
+	ocn.resize(string_container.size());
 
 	size_t index = 0;
 	for(auto string_element : string_container)
-		ocn[index++]->SetStringValue(get_string(string_element));
+		ocn[index++] = enm->AllocNode(ENT_STRING, get_string(string_element));
 
 	return EvaluableNodeReference(list, true);
 }

src/Amalgam/interpreter/InterpreterOpcodesBase.cpp

@@ -184,31 +184,33 @@ EvaluableNodeReference Interpreter::InterpretNode_ENT_SYSTEM(EvaluableNode *en,
 	else if(command == "sign_key_pair")
 	{
 		auto [public_key, secret_key] = GenerateSignatureKeyPair();
-		EvaluableNode *list = evaluableNodeManager->AllocListNodeWithOrderedChildNodes(ENT_STRING, 2);
+		EvaluableNode *list = evaluableNodeManager->AllocNode(ENT_LIST);
 		auto &list_ocn = list->GetOrderedChildNodesReference();
-		list_ocn[0]->SetStringValue(public_key);
-		list_ocn[1]->SetStringValue(secret_key);
+		list_ocn.resize(2);
+		list_ocn[0] = evaluableNodeManager->AllocNode(public_key);
+		list_ocn[1] = evaluableNodeManager->AllocNode(secret_key);
 
 		return EvaluableNodeReference(list, true);
 
 	}
 	else if(command == "encrypt_key_pair")
 	{
 		auto [public_key, secret_key] = GenerateEncryptionKeyPair();
-		EvaluableNode *list = evaluableNodeManager->AllocListNodeWithOrderedChildNodes(ENT_STRING, 2);
+		EvaluableNode *list = evaluableNodeManager->AllocNode(ENT_LIST);
 		auto &list_ocn = list->GetOrderedChildNodesReference();
-		list_ocn[0]->SetStringValue(public_key);
-		list_ocn[1]->SetStringValue(secret_key);
+		list_ocn.resize(2);
+		list_ocn[0] = evaluableNodeManager->AllocNode(public_key);
+		list_ocn[1] = evaluableNodeManager->AllocNode(secret_key);
 
 		return EvaluableNodeReference(list, true);
 	}
 	else if(command == "debugging_info")
 	{
-		EvaluableNode *debugger_info = evaluableNodeManager->AllocListNodeWithOrderedChildNodes(ENT_FALSE, 2);
-		if(Interpreter::GetDebuggingState())
-			debugger_info->GetOrderedChildNodesReference()[0]->SetType(ENT_TRUE, evaluableNodeManager, false);
-		if(asset_manager.debugSources)
-			debugger_info->GetOrderedChildNodesReference()[1]->SetType(ENT_TRUE, evaluableNodeManager, false);
+		EvaluableNode *debugger_info = evaluableNodeManager->AllocNode(ENT_LIST);
+		auto &list_ocn = debugger_info->GetOrderedChildNodesReference();
+		list_ocn.resize(2);
+		list_ocn[0] = evaluableNodeManager->AllocNode(Interpreter::GetDebuggingState());
+		list_ocn[1] = evaluableNodeManager->AllocNode(asset_manager.debugSources);
 
 		return EvaluableNodeReference(debugger_info, true);
 	}
@@ -312,13 +314,13 @@ EvaluableNodeReference Interpreter::InterpretNode_ENT_PARSE(EvaluableNode *en, b
 	retval->ReserveOrderedChildNodes(2);
 	retval->AppendOrderedChildNode(node);
 
-	EvaluableNodeReference warning_list(
-		evaluableNodeManager->AllocListNodeWithOrderedChildNodes(ENT_STRING, warnings.size()), true);
+	EvaluableNodeReference warning_list(evaluableNodeManager->AllocNode(ENT_LIST), true);
 	retval->AppendOrderedChildNode(warning_list);
 
 	auto &list_ocn = warning_list->GetOrderedChildNodesReference();
+	list_ocn.resize(warnings.size());
 	for(size_t i = 0; i < warnings.size(); i++)
-		list_ocn[i]->SetStringValue(warnings[i]);
+		list_ocn[i] = evaluableNodeManager->AllocNode(ENT_STRING, warnings[i]);
 
 	return retval;
 }

src/Amalgam/interpreter/InterpreterOpcodesDataTypes.cpp

@@ -890,13 +890,14 @@ EvaluableNodeReference Interpreter::InterpretNode_ENT_GET_LABELS(EvaluableNode *
 	size_t num_labels = n->GetNumLabels();
 
 	//make list of labels
-	EvaluableNodeReference result(evaluableNodeManager->AllocListNodeWithOrderedChildNodes(ENT_STRING, num_labels), true);
+	EvaluableNodeReference result(evaluableNodeManager->AllocNode(ENT_LIST), true);
 	auto &result_ocn = result->GetOrderedChildNodesReference();
+	result_ocn.resize(num_labels);
 
 	//because labels can be stored in different ways, it is just easiest to iterate
 	// rather than to get a reference to each string id
 	for(size_t i = 0; i < num_labels; i++)
-		result_ocn[i]->SetStringID(n->GetLabelStringId(i));
+		result_ocn[i] = evaluableNodeManager->AllocNode(ENT_STRING, n->GetLabelStringId(i));
 
 	evaluableNodeManager->FreeNodeTreeIfPossible(n);
 	return result;

src/Amalgam/interpreter/InterpreterOpcodesEntityAccess.cpp

@@ -96,13 +96,13 @@ EvaluableNodeReference Interpreter::InterpretNode_ENT_CONTAINED_ENTITIES_and_COM
 			return EvaluableNodeReference(static_cast<double>(contained_entities.size()));
 
 		//new list containing the contained entity ids to return
-		EvaluableNodeReference result(
-			evaluableNodeManager->AllocListNodeWithOrderedChildNodes(ENT_STRING, contained_entities.size()), true);
+		EvaluableNodeReference result(evaluableNodeManager->AllocNode(ENT_LIST), true);
 		auto &result_ocn = result->GetOrderedChildNodesReference();
+		result_ocn.resize(contained_entities.size());
 
 		//create the string references all at once and hand off
 		for(size_t i = 0; i < contained_entities.size(); i++)
-			result_ocn[i]->SetTypeViaStringIdValue(contained_entities[i]->GetIdStringId());
+			result_ocn[i] = evaluableNodeManager->AllocNode(ENT_STRING, contained_entities[i]->GetIdStringId());
 
 		//if not using SBFDS, make sure always return in the same order for consistency, regardless of cashing, hashing, etc.
 		//if using SBFDS, then the order is assumed to not matter for other queries, so don't pay the cost of sorting here

src/Amalgam/interpreter/InterpreterOpcodesListManipulation.cpp

@@ -597,11 +597,12 @@ EvaluableNodeReference Interpreter::InterpretNode_ENT_RANGE(EvaluableNode *en, b
 	//if no function, just return a list of numbers
 	if(index_of_start == 0)
 	{
-		EvaluableNodeReference range_list(evaluableNodeManager->AllocListNodeWithOrderedChildNodes(ENT_NUMBER, num_nodes), true);
+		EvaluableNodeReference range_list(evaluableNodeManager->AllocNode(ENT_LIST), true);
 
 		auto &range_list_ocn = range_list->GetOrderedChildNodesReference();
+		range_list_ocn.resize(num_nodes);
 		for(size_t i = 0; i < num_nodes; i++)
-			range_list_ocn[i]->SetTypeViaNumberValue(i * range_step_size + range_start);
+			range_list_ocn[i] = evaluableNodeManager->AllocNode(i * range_step_size + range_start);
 
 		return range_list;
 	}

src/Amalgam/interpreter/InterpreterOpcodesTransformations.cpp

@@ -1034,11 +1034,12 @@ EvaluableNodeReference Interpreter::InterpretNode_ENT_INDICES(EvaluableNode *en,
 	else if(container->IsOrderedArray())
 	{
 		size_t num_ordered_nodes = container->GetOrderedChildNodesReference().size();
-		index_list.SetReference(evaluableNodeManager->AllocListNodeWithOrderedChildNodes(ENT_NUMBER, num_ordered_nodes));
+		index_list.SetReference(evaluableNodeManager->AllocNode(ENT_LIST));
 
 		auto &index_list_ocn = index_list->GetOrderedChildNodesReference();
+		index_list_ocn.resize(num_ordered_nodes);
 		for(size_t i = 0; i < num_ordered_nodes; i++)
-			index_list_ocn[i]->SetTypeViaNumberValue(static_cast<double>(i));
+			index_list_ocn[i] = evaluableNodeManager->AllocNode(static_cast<double>(i));
 	}
 	else //no child nodes, just alloc an empty list
 		index_list.SetReference(evaluableNodeManager->AllocNode(ENT_LIST));