From 7c44b0e387591546f73eb5d4e479afd087d42944 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Fri, 29 Jan 2016 17:31:43 -0800 Subject: [PATCH] Fix leaked lookahead trees in normal parsing --- spec/runtime/document_spec.cc | 47 ++++++++++++++++++++++++---------- spec/runtime/node_spec.cc | 12 ++++++++- spec/runtime/stack_spec.cc | 1 - spec/spec_helper.h | 2 ++ src/runtime/parser.c | 48 +++++++++++++++++++++++------------ src/runtime/stack.c | 7 +++-- src/runtime/tree.c | 4 +++ 7 files changed, 85 insertions(+), 36 deletions(-) diff --git a/spec/runtime/document_spec.cc b/spec/runtime/document_spec.cc index 194beeac..85552fcf 100644 --- a/spec/runtime/document_spec.cc +++ b/spec/runtime/document_spec.cc @@ -1,5 +1,8 @@ #include "spec_helper.h" +#include "runtime/alloc.h" #include "runtime/debugger.h" +#include "helpers/record_alloc.h" +#include "helpers/stream_methods.h" #include "helpers/tree_helpers.h" #include "helpers/spy_debugger.h" #include "helpers/spy_input.h" @@ -12,13 +15,22 @@ describe("Document", [&]() { TSNode root; before_each([&]() { + record_alloc::start(); doc = ts_document_make(); }); after_each([&]() { ts_document_free(doc); + record_alloc::stop(); + AssertThat(record_alloc::outstanding_allocation_indices(), IsEmpty()); }); + auto assert_node_string_equals = [&](TSNode node, const string &expected) { + char *actual = ts_node_string(node, doc); + AssertThat(actual, Equals(expected)); + ts_free(actual); + }; + describe("set_input(input)", [&]() { SpyInput *spy_input; @@ -30,8 +42,9 @@ describe("Document", [&]() { ts_document_parse(doc); root = ts_document_root_node(doc); - AssertThat(ts_node_string(root, doc), Equals( - "(object (pair (string) (array (number) (number))))")); + assert_node_string_equals( + root, + "(object (pair (string) (array (number) (number))))"); }); after_each([&]() { @@ -48,8 +61,9 @@ describe("Document", [&]() { ts_document_parse(doc); root = ts_document_root_node(doc); - AssertThat(ts_node_string(root, doc), Equals( - "(array (true) (false))")); + assert_node_string_equals( + root, + "(array (true) (false))"); }); it("allows the input to be retrieved later", [&]() { @@ -74,8 +88,9 @@ describe("Document", [&]() { ts_document_parse(doc); TSNode new_root = ts_document_root_node(doc); - AssertThat(ts_node_string(new_root, doc), Equals( - "(object (pair (string) (array (null) (number))))")); + assert_node_string_equals( + new_root, + "(object (pair (string) (array (null) (number))))"); AssertThat(spy_input->strings_read, Equals(vector({" [null, 2", ""}))); }); @@ -83,14 +98,16 @@ describe("Document", [&]() { ts_document_set_input_string(doc, ""); ts_document_parse(doc); TSNode new_root = ts_document_root_node(doc); - AssertThat(ts_node_string(new_root, doc), Equals( - "(ERROR (UNEXPECTED ))")); + assert_node_string_equals( + new_root, + "(ERROR (UNEXPECTED ))"); ts_document_set_input_string(doc, "1"); ts_document_parse(doc); new_root = ts_document_root_node(doc); - AssertThat(ts_node_string(new_root, doc), Equals( - "(number)")); + assert_node_string_equals( + new_root, + "(number)"); }); }); @@ -104,8 +121,9 @@ describe("Document", [&]() { ts_document_parse(doc); root = ts_document_root_node(doc); - AssertThat(ts_node_string(root, doc), Equals( - "(object (pair (string) (array (number) (number))))")); + assert_node_string_equals( + root, + "(object (pair (string) (array (number) (number))))"); }); it("clears out any previous tree", [&]() { @@ -117,9 +135,10 @@ describe("Document", [&]() { ts_document_parse(doc); root = ts_document_root_node(doc); - AssertThat(ts_node_string(root, doc), Equals( + assert_node_string_equals( + root, "(program (expression_statement " - "(object (pair (string) (array (number) (number))))))")); + "(object (pair (string) (array (number) (number))))))"); }); }); diff --git a/spec/runtime/node_spec.cc b/spec/runtime/node_spec.cc index 9b371f6e..cfd1bc1a 100644 --- a/spec/runtime/node_spec.cc +++ b/spec/runtime/node_spec.cc @@ -1,7 +1,10 @@ #include "spec_helper.h" +#include "runtime/alloc.h" #include "helpers/tree_helpers.h" #include "helpers/point_helpers.h" #include "helpers/test_languages.h" +#include "helpers/record_alloc.h" +#include "helpers/stream_methods.h" START_TEST @@ -33,21 +36,28 @@ describe("Node", []() { size_t null_end_index = null_index + string("null").size(); before_each([&]() { + record_alloc::start(); + document = ts_document_make(); ts_document_set_language(document, get_test_language("json")); ts_document_set_input_string(document, input_string.c_str()); ts_document_parse(document); array_node = ts_document_root_node(document); - AssertThat(ts_node_string(array_node, document), Equals( + char *node_string = ts_node_string(array_node, document); + AssertThat(node_string, Equals( "(array " "(number) " "(false) " "(object (pair (string) (null))))")); + ts_free(node_string); }); after_each([&]() { ts_document_free(document); + + record_alloc::stop(); + AssertThat(record_alloc::outstanding_allocation_indices(), IsEmpty()); }); describe("named_child_count(), named_child(i)", [&]() { diff --git a/spec/runtime/stack_spec.cc b/spec/runtime/stack_spec.cc index f9694445..a183ebf4 100644 --- a/spec/runtime/stack_spec.cc +++ b/spec/runtime/stack_spec.cc @@ -1,4 +1,3 @@ -#define TREE_SITTER_WRAP_MALLOC #include "spec_helper.h" #include "helpers/tree_helpers.h" #include "helpers/record_alloc.h" diff --git a/spec/spec_helper.h b/spec/spec_helper.h index 03cc699e..7a3d5bce 100644 --- a/spec/spec_helper.h +++ b/spec/spec_helper.h @@ -14,4 +14,6 @@ using namespace tree_sitter; #define START_TEST go_bandit([]() { #define END_TEST }); +#define TREE_SITTER_WRAP_MALLOC + #endif // SPEC_HELPER_ diff --git a/src/runtime/parser.c b/src/runtime/parser.c index d12f5d17..b04288aa 100644 --- a/src/runtime/parser.c +++ b/src/runtime/parser.c @@ -221,6 +221,7 @@ static TSTree *ts_parser__get_next_lookahead(TSParser *self, int head) { LOG("reuse sym:%s size:%lu extra:%d", SYM_NAME(result->symbol), size.chars, result->extra); ts_parser__pop_reusable_subtree(state); + ts_tree_retain(result); return result; } @@ -305,7 +306,7 @@ static ParseActionResult ts_parser__reduce(TSParser *self, int head, const TSSymbolMetadata *all_metadata = self->language->symbol_metadata; TSSymbolMetadata metadata = all_metadata[symbol]; Vector pop_results = ts_stack_pop(self->stack, head, child_count, count_extra); - if (!pop_results.element_size) + if (!vector_valid(&pop_results)) return FailedToUpdateStackHead; int last_head_index = -1; @@ -326,6 +327,7 @@ static ParseActionResult ts_parser__reduce(TSParser *self, int head, if (pop_result->trees == prior_result->trees) { TSTree **existing_parent = vector_get(&self->reduce_parents, j); parent = *existing_parent; + ts_tree_retain(parent); trailing_extra_count = pop_result->tree_count - parent->child_count; break; } @@ -346,10 +348,10 @@ static ParseActionResult ts_parser__reduce(TSParser *self, int head, parent = ts_tree_make_node(symbol, child_count, pop_result->trees, metadata); if (!parent) - return FailedToUpdateStackHead; + goto error; } if (!vector_push(&self->reduce_parents, &parent)) - return FailedToUpdateStackHead; + goto error; /* * If another path led to the same stack head, add this new parent tree @@ -370,6 +372,7 @@ static ParseActionResult ts_parser__reduce(TSParser *self, int head, */ if (i > 0) { if (symbol == ts_builtin_sym_error) { + removed_heads++; ts_stack_remove_head(self->stack, new_head); continue; } @@ -378,7 +381,7 @@ static ParseActionResult ts_parser__reduce(TSParser *self, int head, LookaheadState lookahead_state = *(LookaheadState *)vector_get(&self->lookahead_states, head); if (!vector_push(&self->lookahead_states, &lookahead_state)) - return FailedToUpdateStackHead; + goto error; } /* @@ -412,7 +415,7 @@ static ParseActionResult ts_parser__reduce(TSParser *self, int head, */ switch (ts_stack_push(self->stack, new_head, state, parent)) { case StackPushResultFailed: - return FailedToUpdateStackHead; + goto error; case StackPushResultMerged: LOG("merge_during_reduce head:%d", new_head); vector_erase(&self->lookahead_states, new_head); @@ -449,17 +452,20 @@ static ParseActionResult ts_parser__reduce(TSParser *self, int head, } } - if (fragile) { - for (size_t i = 0; i < self->reduce_parents.size; i++) { - TSTree **parent = vector_get(&self->reduce_parents, i); + for (size_t i = 0; i < self->reduce_parents.size; i++) { + TSTree **parent = vector_get(&self->reduce_parents, i); + if (fragile) (*parent)->fragile_left = (*parent)->fragile_right = true; - } + ts_tree_release(*parent); } if (removed_heads < revealed_heads) return UpdatedStackHead; else return RemovedStackHead; + +error: + return FailedToUpdateStackHead; } static ParseActionResult ts_parser__reduce_error(TSParser *self, int head, @@ -472,12 +478,12 @@ static ParseActionResult ts_parser__reduce_error(TSParser *self, int head, case RemovedStackHead: return RemovedStackHead; case UpdatedStackHead: { - TSTree **parent = vector_back(&self->reduce_parents); StackEntry *stack_entry = ts_stack_head(self->stack, head); + TSTree *parent = stack_entry->tree; stack_entry->position = ts_length_add(stack_entry->position, lookahead->padding); - (*parent)->size = ts_length_add((*parent)->size, lookahead->padding); - (*parent)->fragile_left = (*parent)->fragile_right = true; + parent->size = ts_length_add(parent->size, lookahead->padding); + parent->fragile_left = parent->fragile_right = true; lookahead->padding = ts_length_zero(); return UpdatedStackHead; } @@ -538,6 +544,7 @@ static ParseActionResult ts_parser__handle_error(TSParser *self, int head, if (lookahead->symbol == ts_builtin_sym_end) { LOG("fail_to_recover"); ts_parser__reduce_error(self, head, -1, lookahead); + ts_tree_release(lookahead); return RemovedStackHead; } } @@ -596,10 +603,10 @@ static ParseActionResult ts_parser__accept(TSParser *self, int head) { size_t new_count = root->child_count + leading_extra_count + trailing_extra_count; ts_tree_set_children(root, new_count, new_children); - ts_tree_retain(root); ts_parser__remove_head(self, pop_result->head_index); self->finished_tree = ts_parser__select_tree(self, self->finished_tree, root); + ts_free(pop_result->trees); break; } } @@ -788,8 +795,11 @@ TSTree *ts_parser_parse(TSParser *self, TSInput input, TSTree *previous_tree) { ts_stack_head_count(self->stack), ts_stack_top_state(self->stack, head), position.chars); - if (position.chars != last_position.chars || - !ts_parser__can_reuse(self, head, lookahead)) { + if (position.chars == last_position.chars && + ts_parser__can_reuse(self, head, lookahead)) { + ts_tree_retain(lookahead); + } else { + ts_tree_release(lookahead); lookahead = ts_parser__get_next_lookahead(self, head); if (!lookahead) return NULL; @@ -800,7 +810,7 @@ TSTree *ts_parser_parse(TSParser *self, TSInput input, TSTree *previous_tree) { switch (ts_parser__consume_lookahead(self, head, lookahead)) { case FailedToUpdateStackHead: - return NULL; + goto error; case RemovedStackHead: removed = true; break; @@ -810,9 +820,15 @@ TSTree *ts_parser_parse(TSParser *self, TSInput input, TSTree *previous_tree) { } } + ts_tree_release(lookahead); + if (ts_stack_head_count(self->stack) == 0) { + ts_stack_clear(self->stack); ts_tree_assign_parents(self->finished_tree); return self->finished_tree; } } + +error: + return NULL; } diff --git a/src/runtime/stack.c b/src/runtime/stack.c index 8e4c63f7..685fc781 100644 --- a/src/runtime/stack.c +++ b/src/runtime/stack.c @@ -163,12 +163,11 @@ static StackNode *stack_node_new(StackNode *next, TSStateId state, TSTree *tree) return self; } -static void ts_stack__add_alternative_tree(Stack *self, StackNode *node, TSTree *tree) { +static void ts_stack__add_alternative_tree(Stack *self, StackNode *node, + TSTree *tree) { if (tree != node->entry.tree) { TSTree *new_tree = self->tree_selection_function( - self->tree_selection_payload, - node->entry.tree, - tree); + self->tree_selection_payload, node->entry.tree, tree); if (new_tree != node->entry.tree) { ts_tree_retain(new_tree); diff --git a/src/runtime/tree.c b/src/runtime/tree.c index 5bcb8127..6d15fb36 100644 --- a/src/runtime/tree.c +++ b/src/runtime/tree.c @@ -76,6 +76,8 @@ void ts_tree_assign_parents(TSTree *self) { } void ts_tree_set_children(TSTree *self, size_t child_count, TSTree **children) { + if (self->child_count > 0) + ts_free(self->children); self->children = children; self->child_count = child_count; self->named_child_count = 0; @@ -131,6 +133,8 @@ void ts_tree_retain(TSTree *self) { } void ts_tree_release(TSTree *self) { + if (!self) + return; assert(self->ref_count > 0); self->ref_count--; if (self->ref_count == 0) {