Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

22339: Fixes segfault bug #314

Merged
merged 2 commits into from
Nov 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
115 changes: 1 addition & 114 deletions src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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);
Expand Down
14 changes: 6 additions & 8 deletions src/Amalgam/evaluablenode/EvaluableNodeManagement.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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;
}
}
Expand Down Expand Up @@ -1155,5 +1154,4 @@ class EvaluableNodeManager
#endif
// Holds EvaluableNode*'s reserved for allocation by a specific thread
inline static std::vector<EvaluableNode *> threadLocalAllocationBuffer;

};
15 changes: 9 additions & 6 deletions src/Amalgam/evaluablenode/EvaluableNodeTreeFunctions.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand All @@ -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);
}
Expand All @@ -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);
}
Expand Down
30 changes: 16 additions & 14 deletions src/Amalgam/interpreter/InterpreterOpcodesBase.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down Expand Up @@ -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;
}
Expand Down
5 changes: 3 additions & 2 deletions src/Amalgam/interpreter/InterpreterOpcodesDataTypes.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
6 changes: 3 additions & 3 deletions src/Amalgam/interpreter/InterpreterOpcodesEntityAccess.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down
Loading