19108: Fixes resource leak with strings that could also lead to erron…

…eous mixing of strings when handling strings with few references (#64)
howsoai · Jan 26, 2024 · 63be16b · 63be16b
1 parent bf5fc88
commit 63be16b
Show file tree

Hide file tree

Showing 7 changed files with 62 additions and 61 deletions.
diff --git a/src/Amalgam/AmalgamMain.cpp b/src/Amalgam/AmalgamMain.cpp
@@ -318,9 +318,9 @@ PLATFORM_MAIN_CONSOLE
 			if(num_strings_used > 0)
 			{
 				std::cerr << "ERROR: Num strings still in use: " << num_strings_used << std::endl;
-				std::vector<std::string> in_use = string_intern_pool.GetNonStaticStringsInUse();
-				for(auto &s : in_use)
-					std::cerr << '"' << s << '"' << std::endl;
+				std::vector<std::pair<std::string, int64_t>> in_use = string_intern_pool.GetNonStaticStringsInUse();
+				for(auto &[s, count] : in_use)
+					std::cerr << '"' << s << "\":" << count << std::endl;
 			}
 
 			std::cout << "Memory reclaimation complete." << std::endl;

diff --git a/src/Amalgam/interpreter/Interpreter.cpp b/src/Amalgam/interpreter/Interpreter.cpp
@@ -435,7 +435,7 @@ EvaluableNodeReference Interpreter::ConvertArgsToCallStack(EvaluableNodeReferenc
 	}
 	else if(!args.unique)
 	{
-		args.SetReference(enm.AllocNode(args));
+		args.SetReference(enm.AllocNode(args, EvaluableNodeManager::ENMM_REMOVE_ALL));
 	}
 
 	EvaluableNode *call_stack = enm.AllocNode(ENT_LIST);
@@ -675,7 +675,6 @@ double Interpreter::InterpretNodeIntoNumberValue(EvaluableNode *n)
 
 	double value = result_value.GetValueAsNumber();
 	evaluableNodeManager->FreeNodeTreeIfPossible(result);
-	result.FreeImmediateResources();
 
 	return value;
 }
@@ -709,7 +708,6 @@ bool Interpreter::InterpretNodeIntoBoolValue(EvaluableNode *n, bool value_if_nul
 
 	bool value = result_value.GetValueAsBoolean();
 	evaluableNodeManager->FreeNodeTreeIfPossible(result);
-	result.FreeImmediateResources();
 
 	return value;
 }

diff --git a/src/Amalgam/interpreter/Interpreter.h b/src/Amalgam/interpreter/Interpreter.h
@@ -110,7 +110,7 @@ class Interpreter
 		if(EvaluableNode::IsAssociativeArray(new_context))
 		{
 			if(!new_context.unique)
-				new_context.SetReference(evaluableNodeManager->AllocNode(new_context));
+				new_context.SetReference(evaluableNodeManager->AllocNode(new_context, EvaluableNodeManager::ENMM_REMOVE_ALL));
 		}
 		else //not assoc, make a new one
 		{

diff --git a/src/Amalgam/interpreter/InterpreterOpcodesBase.cpp b/src/Amalgam/interpreter/InterpreterOpcodesBase.cpp
@@ -333,7 +333,7 @@ EvaluableNodeReference Interpreter::InterpretNode_ENT_SEQUENCE(EvaluableNode *en
 		evaluableNodeManager->FreeNodeTreeIfPossible(result);
 		//request immediate values when not last, since any allocs for returns would be wasted
 		//concludes won't be immediate
-		result = InterpretNode(ocn[i], immediate_result || i < ocn_size - 1);
+		result = InterpretNode(ocn[i], immediate_result || i + 1 < ocn_size);
 	}
 	return result;
 }
@@ -613,7 +613,7 @@ EvaluableNodeReference Interpreter::InterpretNode_ENT_WHILE(EvaluableNode *en, b
 			//concludes won't be immediate
 			//but because previous_result may be used, that can't be immediate, so the last param
 			//cannot be evaulated as immediate
-			new_result = InterpretNode(ocn[i], i < ocn_size - 1);
+			new_result = InterpretNode(ocn[i], i + 1 < ocn_size);
 
 			if(!new_result.IsImmediateValue() && new_result != nullptr && new_result->GetType() == ENT_CONCLUDE)
 			{
@@ -626,7 +626,7 @@ EvaluableNodeReference Interpreter::InterpretNode_ENT_WHILE(EvaluableNode *en, b
 			}
 
 			//don't free the last new_result
-			if(i < ocn_size - 1)
+			if(i + 1 < ocn_size)
 				evaluableNodeManager->FreeNodeTreeIfPossible(new_result);
 		}
 
@@ -666,7 +666,7 @@ EvaluableNodeReference Interpreter::InterpretNode_ENT_LET(EvaluableNode *en, boo
 		evaluableNodeManager->FreeNodeTreeIfPossible(result);
 		//request immediate values when not last, since any allocs for returns would be wasted
 		//concludes won't be immediate
-		result = InterpretNode(ocn[i], immediate_result || i < ocn_size - 1);
+		result = InterpretNode(ocn[i], immediate_result || i + 1 < ocn_size);
 	}
 
 	//all finished with new context, but can't free it in case returning something
@@ -770,7 +770,7 @@ EvaluableNodeReference Interpreter::InterpretNode_ENT_DECLARE(EvaluableNode *en,
 		evaluableNodeManager->FreeNodeTreeIfPossible(result);
 		//request immediate values when not last, since any allocs for returns would be wasted
 		//concludes won't be immediate
-		result = InterpretNode(ocn[i], immediate_result || i < ocn_size - 1);
+		result = InterpretNode(ocn[i], immediate_result || i + 1 < ocn_size);
 	}
 
 	return result;
@@ -1036,8 +1036,9 @@ EvaluableNodeReference Interpreter::InterpretNode_ENT_RETRIEVE(EvaluableNode *en
 	if(to_lookup == nullptr || IsEvaluableNodeTypeImmediate(to_lookup->GetType()))
 	{
 		StringInternPool::StringID symbol_name_sid = EvaluableNode::ToStringIDIfExists(to_lookup);
+		EvaluableNode* symbol_value = GetCallStackSymbol(symbol_name_sid);
 		evaluableNodeManager->FreeNodeTreeIfPossible(to_lookup);
-		return EvaluableNodeReference(GetCallStackSymbol(symbol_name_sid), false);
+		return EvaluableNodeReference(symbol_value, false);
 	}
 	else if(to_lookup->IsAssociativeArray())
 	{
@@ -1070,11 +1071,12 @@ EvaluableNodeReference Interpreter::InterpretNode_ENT_RETRIEVE(EvaluableNode *en
 				continue;
 			}
 
+			EvaluableNode *symbol_value = GetCallStackSymbol(symbol_name_sid);
 			//if there are values passed in, free them to be clobbered
 			EvaluableNodeReference cnr(cn, to_lookup.unique);
 			evaluableNodeManager->FreeNodeTreeIfPossible(cnr);
 
-			cn = GetCallStackSymbol(symbol_name_sid);
+			cn = symbol_value;
 		}
 
 		return EvaluableNodeReference(to_lookup, false);

diff --git a/src/Amalgam/interpreter/InterpreterOpcodesEntityAccess.cpp b/src/Amalgam/interpreter/InterpreterOpcodesEntityAccess.cpp
@@ -335,12 +335,13 @@ EvaluableNodeReference Interpreter::InterpretNode_ENT_RETRIEVE_FROM_ENTITY_and_D
 	if(to_lookup == nullptr || IsEvaluableNodeTypeImmediate(to_lookup->GetType()))
 	{
 		StringInternPool::StringID label_sid = EvaluableNode::ToStringIDIfExists(to_lookup);
-		evaluableNodeManager->FreeNodeTreeIfPossible(to_lookup);
 
 		ExecutionCycleCount num_steps_executed = 0;
 		EvaluableNodeReference value = target_entity->GetValueAtLabel(label_sid, evaluableNodeManager, direct, target_entity == curEntity);
 		curExecutionStep += num_steps_executed;
 
+		evaluableNodeManager->FreeNodeTreeIfPossible(to_lookup);
+
 		return value;
 	}
 	else if(to_lookup->IsAssociativeArray())

diff --git a/src/Amalgam/out.txt b/src/Amalgam/out.txt
@@ -1045,7 +1045,7 @@ abcdef
 	4
 	"d"
 )
-(list 2 "d" 1 0 3)
+(list 2 1 "d" 0 3)
 (list
 	1
 	2
@@ -1240,7 +1240,7 @@ current_index: 2
 				8
 			)
 		accum_string "abcdef"
-		argv (list "C:\\Users\\Chris Hazard\\Desktop\\Howso_repos\\amalgam\\src\\Amalgam\\./amlg_code/full_test.amlg")
+		argv (list "C:\\Users\\ChristopherHazard\\Desktop\\Howso_repos\\amalgam\\src\\Amalgam\\./amlg_code/full_test.amlg")
 		bar (declare
 				(assoc x 6)
 				(+ x 2)
@@ -1253,10 +1253,10 @@ current_index: 2
 				A (assoc B 2)
 				B 2
 			)
-		interpreter "C:\\Users\\Chris Hazard\\Desktop\\Howso_repos\\amalgam\\x64\\MT_Release_EXE\\Amalgam.exe"
+		interpreter "C:\\Users\\ChristopherHazard\\Desktop\\Howso_repos\\amalgam\\x64\\MT_Release_EXE\\Amalgam.exe"
 		raaa 2
 		rwww 1
-		start_time 1706155111.185343
+		start_time 1706237472.540396
 		www 1
 		x 12
 		zz 10
@@ -1283,7 +1283,7 @@ current_index: 2
 			8
 		)
 	accum_string "abcdef"
-	argv (list "C:\\Users\\Chris Hazard\\Desktop\\Howso_repos\\amalgam\\src\\Amalgam\\./amlg_code/full_test.amlg")
+	argv (list "C:\\Users\\ChristopherHazard\\Desktop\\Howso_repos\\amalgam\\src\\Amalgam\\./amlg_code/full_test.amlg")
 	bar (declare
 			(assoc x 6)
 			(+ x 2)
@@ -1296,10 +1296,10 @@ current_index: 2
 			A (assoc B 2)
 			B 2
 		)
-	interpreter "C:\\Users\\Chris Hazard\\Desktop\\Howso_repos\\amalgam\\x64\\MT_Release_EXE\\Amalgam.exe"
+	interpreter "C:\\Users\\ChristopherHazard\\Desktop\\Howso_repos\\amalgam\\x64\\MT_Release_EXE\\Amalgam.exe"
 	raaa 2
 	rwww 1
-	start_time 1706155111.185343
+	start_time 1706237472.540396
 	www 1
 	x 12
 	zz 10
@@ -1325,7 +1325,7 @@ current_index: 2
 			8
 		)
 	accum_string "abcdef"
-	argv (list "C:\\Users\\Chris Hazard\\Desktop\\Howso_repos\\amalgam\\src\\Amalgam\\./amlg_code/full_test.amlg")
+	argv (list "C:\\Users\\ChristopherHazard\\Desktop\\Howso_repos\\amalgam\\src\\Amalgam\\./amlg_code/full_test.amlg")
 	bar (declare
 			(assoc x 6)
 			(+ x 2)
@@ -1338,10 +1338,10 @@ current_index: 2
 			A (assoc B 2)
 			B 2
 		)
-	interpreter "C:\\Users\\Chris Hazard\\Desktop\\Howso_repos\\amalgam\\x64\\MT_Release_EXE\\Amalgam.exe"
+	interpreter "C:\\Users\\ChristopherHazard\\Desktop\\Howso_repos\\amalgam\\x64\\MT_Release_EXE\\Amalgam.exe"
 	raaa 2
 	rwww 1
-	start_time 1706155111.185343
+	start_time 1706237472.540396
 	www 1
 	x 12
 	zz 10
@@ -1489,7 +1489,7 @@ infinity test c or d: (list "c" @(get (target 0) 0) "d" @(get (target 0) 0))
 
 (assoc a 29 b 44 c 27)
 
-(list "5" "2" "1")
+(list "7" "2" "1")
 
 --get_rand_seed--
 ��ȼ�\�KOaV�T z�
@@ -1609,7 +1609,7 @@ e:
   - .inf
 
 25: (assoc a 1)
-current date-time in epoch: 2024-01-24-22.58.31.5508810
+current date-time in epoch: 2024-01-25-21.51.12.5883020
 2020-06-07 00:22:59
 1391230800
 1391230800
@@ -2685,10 +2685,10 @@ MergeEntityChild1
 (associate "x" 3 "y" 4)
 MergeEntityChild2
 (associate "p" 3 "q" 4)
-_2169689611
-(associate "e" 3 "f" 4)
 _2280722175
 (associate "E" 3 "F" 4)
+_2169689611
+(associate "e" 3 "f" 4)
 --union_entities--
 (associate "b" 4 "a" 3 "c" 3)
 MergeEntityChild1
@@ -2706,17 +2706,6 @@ MergeEntityChild2
 	"w"
 	7
 )
-_2169689611
-(associate
-	"e"
-	3
-	"f"
-	4
-	"g"
-	5
-	"h"
-	6
-)
 _2280722175
 (associate
 	"E"
@@ -2728,11 +2717,7 @@ _2280722175
 	"H"
 	6
 )
-(parallel
-	##p
-	(list "_3990396532" "_3330773578" "_3990396532" "_3330773578")
-)
-_3330773578
+_2169689611
 (associate
 	"e"
 	3
@@ -2743,6 +2728,10 @@ _3330773578
 	"h"
 	6
 )
+(parallel
+	##p
+	(list "_3990396532" "_3330773578" "_3990396532" "_3330773578")
+)
 _3990396532
 (associate
 	"E"
@@ -2754,6 +2743,17 @@ _3990396532
 	"H"
 	6
 )
+_3330773578
+(associate
+	"e"
+	3
+	"f"
+	4
+	"g"
+	5
+	"h"
+	6
+)
 --difference_entities--
 (declare
 	(assoc _ (null))
@@ -3375,17 +3375,17 @@ MergeEntityChild1
 (associate "x" 3 "y" 4 "z" 5)
 MergeEntityChild2
 (associate "p" 3 "q" 4 "u" 5)
-_2169689611
-(associate "e" 3 "f" 4)
 _2280722175
+(associate "E" 3 "F" 4)
+_2169689611
 (associate
-	"E"
+	"e"
 	3
-	"F"
+	"f"
 	4
-	"G"
+	"g"
 	5
-	"H"
+	"h"
 	6
 )
 --get_entity_comments--
@@ -3442,7 +3442,7 @@ deep sets
 
 --set_entity_root_permission--
 RootTest
-1706155111.773997
+1706237472.781054
 (true)
 
 RootTest
@@ -3666,7 +3666,7 @@ hello
 				)
 			)
 	)
-	(set_entity_rand_seed new_entity "U�EO��0є-I���")
+	(set_entity_rand_seed new_entity "�^\n�brj�0є-I���")
 	(set_entity_rand_seed
 		(first
 			(create_entities
@@ -3676,7 +3676,7 @@ hello
 				)
 			)
 		)
-		"C(Űv���6�RbH��"
+		"5ȳ\0�Nb�h��e��"
 	)
 	(set_entity_rand_seed
 		(first
@@ -3709,7 +3709,7 @@ hello
 				)
 			)
 	)
-	(set_entity_rand_seed new_entity "U�EO��0є-I���")
+	(set_entity_rand_seed new_entity "�^\n�brj�0є-I���")
 	(set_entity_rand_seed
 		(first
 			(create_entities
@@ -4658,4 +4658,4 @@ Expecting 1000: 1000
 concurrent entity writes successful: (true)
 
 --total execution time--
-1.1163010597229004
+1.728956937789917