--- Begin Message ---
|
Subject: |
29.0.60; Fixing a small leak in tree-sitter search functions |
|
Date: |
Thu, 13 Apr 2023 15:48:43 -0700 |
When we switch to using cursors instead of nodes [1] in treesit.c for
search functions, we introduced a small leak: the search functions
allows the user to pass a predicate function, which can signal. And
because we are now using cursors, which needs to be freed, everytime the
predicate function signals, the cursor is leaked.
I pushed a fix to master (a5eb9f6ad4e), the change is relatively
straightforward so I hope we can pick it into the next pretest.
And sorry for introducing the leak in the first place :-(
Yuan
[1] Commit e492c21e81040b9539139b78f6baf98df17bbaab (bug#60267)
Below is the fix:
commit a5eb9f6ad4e6f5a2819b540a477f1e889f6ef355
Author: Yuan Fu <casouri@gmail.com>
Date: Thu Apr 13 14:36:46 2023 -0700
Catch signals produced by PRED in tree-sitter search functions
Earlier we switched to using cursors rather than nodes to traverse the
parse tree. Because cursors need cleanup, we have to catch signals
thrown by the predicate functions and free the cursor. Failing to do
this will result in leaking the cursor whenever the predicate function
signals in a search function.
This change fixes the leak.
* src/treesit.c (treesit_traverse_cleanup_cursor): New function.
(Ftreesit_search_subtree)
(Ftreesit_search_forward)
(Ftreesit_induce_sparse_tree): Catch signals.
diff --git a/src/treesit.c b/src/treesit.c
index fd5fda78133..76d1dc8ccf4 100644
--- a/src/treesit.c
+++ b/src/treesit.c
@@ -3247,6 +3247,12 @@ treesit_search_forward (TSTreeCursor *cursor,
}
}
+/** Cleanup function for cursor. */
+static void treesit_traverse_cleanup_cursor(void *cursor)
+{
+ ts_tree_cursor_delete ((TSTreeCursor *) cursor);
+}
+
DEFUN ("treesit-search-subtree",
Ftreesit_search_subtree,
Streesit_search_subtree, 2, 5, 0,
@@ -3288,12 +3294,18 @@ DEFUN ("treesit-search-subtree",
if (!treesit_cursor_helper (&cursor, XTS_NODE (node)->node, parser))
return return_value;
+ specpdl_ref count = SPECPDL_INDEX ();
+ record_unwind_protect_ptr (treesit_traverse_cleanup_cursor, &cursor);
+
if (treesit_search_dfs (&cursor, predicate, parser, NILP (backward),
NILP (all), the_limit, false))
{
TSNode node = ts_tree_cursor_current_node (&cursor);
return_value = make_treesit_node (parser, node);
}
+
+ unbind_to (count, Qnil);
+
ts_tree_cursor_delete (&cursor);
return return_value;
}
@@ -3345,12 +3357,18 @@ DEFUN ("treesit-search-forward",
if (!treesit_cursor_helper (&cursor, XTS_NODE (start)->node, parser))
return return_value;
+ specpdl_ref count = SPECPDL_INDEX ();
+ record_unwind_protect_ptr (treesit_traverse_cleanup_cursor, &cursor);
+
if (treesit_search_forward (&cursor, predicate, parser,
NILP (backward), NILP (all)))
{
TSNode node = ts_tree_cursor_current_node (&cursor);
return_value = make_treesit_node (parser, node);
}
+
+ unbind_to (count, Qnil);
+
ts_tree_cursor_delete (&cursor);
return return_value;
}
@@ -3467,8 +3485,14 @@ DEFUN ("treesit-induce-sparse-tree",
to use treesit_cursor_helper. */
TSTreeCursor cursor = ts_tree_cursor_new (XTS_NODE (root)->node);
+ specpdl_ref count = SPECPDL_INDEX ();
+ record_unwind_protect_ptr (treesit_traverse_cleanup_cursor, &cursor);
+
treesit_build_sparse_tree (&cursor, parent, predicate, process_fn,
the_limit, parser);
+
+ unbind_to (count, Qnil);
+
ts_tree_cursor_delete (&cursor);
Fsetcdr (parent, Fnreverse (Fcdr (parent)));
if (NILP (Fcdr (parent)))
--- End Message ---
--- Begin Message ---
|
Subject: |
Re: bug#62823: 29.0.60; Fixing a small leak in tree-sitter search functions |
|
Date: |
Tue, 5 Sep 2023 16:55:36 -0700 |
Yuan Fu <casouri@gmail.com> writes:
> When we switch to using cursors instead of nodes [1] in treesit.c for
> search functions, we introduced a small leak: the search functions
> allows the user to pass a predicate function, which can signal. And
> because we are now using cursors, which needs to be freed, everytime the
> predicate function signals, the cursor is leaked.
>
> I pushed a fix to master (a5eb9f6ad4e), the change is relatively
> straightforward so I hope we can pick it into the next pretest.
>
> And sorry for introducing the leak in the first place :-(
It seems like this was already installed, so I'm closing this.
--- End Message ---