21413: Fixes segfaults (#245)

src/Amalgam/SeparableBoxFilterDataStore.h

@@ -697,8 +697,8 @@ class SeparableBoxFilterDataStore
 
 	//search a projection width in terms of bucket count or number of collected entities
 	//accumulates partial sums
-	//searches until num_entities_to_populate are popluated or other heuristics have been reached
-	//will only consider indices in enabled_indiced
+	//searches until num_entities_to_populate are populated or other heuristics have been reached
+	//will only consider indices in enabled_indices
 	// query_feature_index is the offset to access the feature relative to the particular query data parameters
 	//returns the smallest partial sum for any value not yet computed
 	double PopulatePartialSumsWithSimilarFeatureValue(RepeatedGeneralizedDistanceEvaluator &r_dist_eval,
@@ -903,7 +903,7 @@ class SeparableBoxFilterDataStore
 		auto [num_calculated_features, distance] = partial_sums.GetNumFilledAndSum(entity_index);
 
 		//complete known sums with worst and best possibilities
-		//calculate the number of features for which the minkowski distance term has not yet been calculated 
+		//calculate the number of features for which the Minkowski distance term has not yet been calculated 
 		size_t num_uncalculated_features = (num_features - num_calculated_features);
 		//if have already calculated everything, then already have the distance
 		if(num_uncalculated_features == 0)

src/Amalgam/evaluablenode/EvaluableNodeManagement.h

@@ -59,7 +59,8 @@ class EvaluableNodeReference
 
 	//when attached a child node, make sure that this node reflects the same properties
 	//if first_attachment_to_unique_node is true, then it will not call SetNeedCycleCheck(true)
-	//if the attached is not unique
+	//if the attached is not unique. Note that this parameter should not be set to true
+	//if the node can be accessed in any other way, such as the construction stack
 	void UpdatePropertiesBasedOnAttachedNode(EvaluableNodeReference &attached,
 		bool first_attachment_to_unique_node = false)
 	{

src/Amalgam/interpreter/Interpreter.cpp

@@ -326,13 +326,13 @@ Interpreter::Interpreter(EvaluableNodeManager *enm, RandomStream rand_stream,
 
 #ifdef MULTITHREAD_SUPPORT
 EvaluableNodeReference Interpreter::ExecuteNode(EvaluableNode *en,
-	EvaluableNode *call_stack, EvaluableNode *interpreter_node_stack,
+	EvaluableNode *call_stack, EvaluableNode *opcode_stack,
 	EvaluableNode *construction_stack,
 	std::vector<ConstructionStackIndexAndPreviousResultUniqueness> *construction_stack_indices,
 	Concurrency::ReadWriteMutex *call_stack_write_mutex, bool immediate_result)
 #else
 EvaluableNodeReference Interpreter::ExecuteNode(EvaluableNode *en,
-	EvaluableNode *call_stack, EvaluableNode *interpreter_node_stack,
+	EvaluableNode *call_stack, EvaluableNode *opcode_stack,
 	EvaluableNode *construction_stack,
 	std::vector<ConstructionStackIndexAndPreviousResultUniqueness> *construction_stack_indices,
 	bool immediate_result)
@@ -359,14 +359,22 @@ EvaluableNodeReference Interpreter::ExecuteNode(EvaluableNode *en,
 		call_stack->AppendOrderedChildNode(new_context_entry);
 	}
 
-	if(interpreter_node_stack == nullptr)
-		interpreter_node_stack = evaluableNodeManager->AllocNode(ENT_LIST);
+	bool free_opcode_stack = false;
+	if(opcode_stack == nullptr)
+	{
+		opcode_stack = evaluableNodeManager->AllocNode(ENT_LIST);
+		free_opcode_stack = true;
+	}
 
+	bool free_construction_stack = false;
 	if(construction_stack == nullptr)
+	{
 		construction_stack = evaluableNodeManager->AllocNode(ENT_LIST);
+		free_construction_stack = true;
+	}
 
 	callStackNodes = &call_stack->GetOrderedChildNodes();
-	opcodeStackNodes = &interpreter_node_stack->GetOrderedChildNodes();
+	opcodeStackNodes = &opcode_stack->GetOrderedChildNodes();
 	constructionStackNodes = &construction_stack->GetOrderedChildNodes();
 
 	if(construction_stack_indices != nullptr)
@@ -377,19 +385,21 @@ EvaluableNodeReference Interpreter::ExecuteNode(EvaluableNode *en,
 	call_stack->SetNeedCycleCheck(true);
 	for(auto &cn : call_stack->GetOrderedChildNodesReference())
 		cn->SetNeedCycleCheck(true);
-	interpreter_node_stack->SetNeedCycleCheck(true);
+	opcode_stack->SetNeedCycleCheck(true);
 	construction_stack->SetNeedCycleCheck(true);
 
 	//keep these references as long as the interpreter is around
-	evaluableNodeManager->KeepNodeReferences(call_stack, interpreter_node_stack, construction_stack);
+	evaluableNodeManager->KeepNodeReferences(call_stack, opcode_stack, construction_stack);
 
 	auto retval = InterpretNode(en, immediate_result);
 
-	evaluableNodeManager->FreeNodeReferences(call_stack, interpreter_node_stack, construction_stack);
+	evaluableNodeManager->FreeNodeReferences(call_stack, opcode_stack, construction_stack);
 
 	//remove these nodes
-	evaluableNodeManager->FreeNode(interpreter_node_stack);
-	evaluableNodeManager->FreeNode(construction_stack);
+	if(free_opcode_stack)
+		evaluableNodeManager->FreeNode(opcode_stack);
+	if(free_construction_stack)
+		evaluableNodeManager->FreeNode(construction_stack);
 
 	return retval;
 }

src/Amalgam/interpreter/Interpreter.h

@@ -163,21 +163,21 @@ class Interpreter
 
 	//Executes the current Entity that this Interpreter is contained by
 	// sets up all of the stack and contextual structures, then calls InterpretNode on en
-	//if call_stack, interpreter_node_stack, or construction_stack are nullptr, it will start with a new one
+	//if call_stack, opcode_stack, or construction_stack are nullptr, it will start with a new one
 	//note that construction_stack and construction_stack_indices should be specified together and should be the same length
 	//if immediate_result is true, then the returned value may be immediate
 #ifdef MULTITHREAD_SUPPORT
 	//if run multithreaded, then for performance reasons, it is optimal to have one of each stack per thread
 	// and call_stack_write_mutex is the mutex needed to lock for writing
 	EvaluableNodeReference ExecuteNode(EvaluableNode *en,
-		EvaluableNode *call_stack = nullptr, EvaluableNode *interpreter_node_stack = nullptr,
+		EvaluableNode *call_stack = nullptr, EvaluableNode *opcode_stack = nullptr,
 		EvaluableNode *construction_stack = nullptr,
 		std::vector<ConstructionStackIndexAndPreviousResultUniqueness> *construction_stack_indices = nullptr,
 		Concurrency::ReadWriteMutex *call_stack_write_mutex = nullptr,
 		bool immediate_result = false);
 #else
 	EvaluableNodeReference ExecuteNode(EvaluableNode *en,
-		EvaluableNode *call_stack = nullptr, EvaluableNode *interpreter_node_stack = nullptr,
+		EvaluableNode *call_stack = nullptr, EvaluableNode *opcode_stack = nullptr,
 		EvaluableNode *construction_stack = nullptr,
 		std::vector<ConstructionStackIndexAndPreviousResultUniqueness> *construction_stack_indices = nullptr,
 		bool immediate_result = false);

src/Amalgam/interpreter/InterpreterOpcodesDataTypes.cpp

@@ -91,7 +91,7 @@ EvaluableNodeReference Interpreter::InterpretNode_ENT_LIST(EvaluableNode *en, bo
 			auto value = InterpretNode(ocn[i]);
 			//add it to the list
 			new_list_ocn[i] = value;
-			new_list.UpdatePropertiesBasedOnAttachedNode(value, i == 0);
+			new_list.UpdatePropertiesBasedOnAttachedNode(value);
 		}
 
 		if(PopConstructionContextAndGetExecutionSideEffectFlag())
@@ -142,7 +142,6 @@ EvaluableNodeReference Interpreter::InterpretNode_ENT_ASSOC(EvaluableNode *en, b
 		//construction stack has a reference, so no KeepNodeReference isn't needed for anything referenced
 		PushNewConstructionContext(en, new_assoc, EvaluableNodeImmediateValueWithType(StringInternPool::NOT_A_STRING_ID), nullptr);
 
-		bool first_node = true;
 		for(auto &[cn_id, cn] : new_mcn)
 		{
 			SetTopCurrentIndexInConstructionStack(cn_id);
@@ -151,8 +150,7 @@ EvaluableNodeReference Interpreter::InterpretNode_ENT_ASSOC(EvaluableNode *en, b
 			EvaluableNodeReference element_result = InterpretNode(cn);
 
 			cn = element_result;
-			new_assoc.UpdatePropertiesBasedOnAttachedNode(element_result, first_node);
-			first_node = false;
+			new_assoc.UpdatePropertiesBasedOnAttachedNode(element_result);
 		}
 
 		if(PopConstructionContextAndGetExecutionSideEffectFlag())

src/Amalgam/interpreter/InterpreterOpcodesEntityControl.cpp

@@ -804,7 +804,7 @@ EvaluableNodeReference Interpreter::InterpretNode_ENT_STORE_ENTITY(EvaluableNode
 		evaluableNodeManager->FreeNodeTreeIfPossible(params);
 	}
 
-	//get the id of the source entity to store.  Don't need to keep the reference because it won't be used once the source entety pointer is looked up
+	//get the id of the source entity to store.  Don't need to keep the reference because it won't be used once the source entity pointer is looked up
 	//retrieve the entity after other parameters to minimize time in locks
 	// and prevent deadlock if one of the params accessed the entity
 	//StoreEntityToResourcePath will read lock all contained entities appropriately

src/Amalgam/interpreter/InterpreterOpcodesListManipulation.cpp

@@ -648,7 +648,7 @@ EvaluableNodeReference Interpreter::InterpretNode_ENT_RANGE(EvaluableNode *en, b
 
 		EvaluableNodeReference element_result = InterpretNode(function);
 		result_ocn[i] = element_result;
-		result.UpdatePropertiesBasedOnAttachedNode(element_result, i == 0);
+		result.UpdatePropertiesBasedOnAttachedNode(element_result);
 	}
 
 	if(PopConstructionContextAndGetExecutionSideEffectFlag())

src/Amalgam/interpreter/InterpreterOpcodesTransformations.cpp

@@ -115,7 +115,7 @@ EvaluableNodeReference Interpreter::InterpretNode_ENT_MAP(EvaluableNode *en, boo
 
 				EvaluableNodeReference element_result = InterpretNode(function);
 				result_ocn[i] = element_result;
-				result.UpdatePropertiesBasedOnAttachedNode(element_result, i == 0);
+				result.UpdatePropertiesBasedOnAttachedNode(element_result);
 			}
 
 			if(PopConstructionContextAndGetExecutionSideEffectFlag())
@@ -170,7 +170,6 @@ EvaluableNodeReference Interpreter::InterpretNode_ENT_MAP(EvaluableNode *en, boo
 
 			PushNewConstructionContext(list, result, EvaluableNodeImmediateValueWithType(StringInternPool::NOT_A_STRING_ID), nullptr);
 
-			bool first_element = true;
 			for(auto &[result_id, result_node] : result_mcn)
 			{
 				SetTopCurrentIndexInConstructionStack(result_id);
@@ -184,8 +183,7 @@ EvaluableNodeReference Interpreter::InterpretNode_ENT_MAP(EvaluableNode *en, boo
 				//in order to keep the node properties to be updated below
 				EvaluableNodeReference element_result = InterpretNode(function);
 				result_node = element_result;
-				result.UpdatePropertiesBasedOnAttachedNode(element_result, first_element);
-				first_element = false;
+				result.UpdatePropertiesBasedOnAttachedNode(element_result);
 			}
 
 			if(PopConstructionContextAndGetExecutionSideEffectFlag())
@@ -1567,7 +1565,7 @@ EvaluableNodeReference Interpreter::InterpretNode_ENT_ASSOCIATE(EvaluableNode *e
 
 			//handoff the reference from index_value to the assoc
 			new_assoc->SetMappedChildNodeWithReferenceHandoff(key_sid, value);
-			new_assoc.UpdatePropertiesBasedOnAttachedNode(value, i == 0);
+			new_assoc.UpdatePropertiesBasedOnAttachedNode(value);
 		}
 
 		if(PopConstructionContextAndGetExecutionSideEffectFlag())