GNU bug report logs - #62823
29.0.60; Fixing a small leak in tree-sitter search functions

Previous Next

Package: emacs;

Reported by: Yuan Fu <casouri <at> gmail.com>

Date: Thu, 13 Apr 2023 22:50:02 UTC

Severity: normal

Found in version 29.0.60

Done: Stefan Kangas <stefankangas <at> gmail.com>

Bug is archived. No further changes may be made.

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 62823 in the body.
You can then email your comments to 62823 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to bug-gnu-emacs <at> gnu.org:
bug#62823; Package emacs. (Thu, 13 Apr 2023 22:50:02 GMT) Full text and rfc822 format available.

Acknowledgement sent to Yuan Fu <casouri <at> gmail.com>:
New bug report received and forwarded. Copy sent to bug-gnu-emacs <at> gnu.org. (Thu, 13 Apr 2023 22:50:02 GMT) Full text and rfc822 format available.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Yuan Fu <casouri <at> gmail.com>
To: Bug Report Emacs <bug-gnu-emacs <at> gnu.org>
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 <at> 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)))






Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#62823; Package emacs. (Fri, 14 Apr 2023 06:47:02 GMT) Full text and rfc822 format available.

Message #8 received at 62823 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Yuan Fu <casouri <at> gmail.com>
Cc: 62823 <at> debbugs.gnu.org
Subject: Re: bug#62823: 29.0.60;
 Fixing a small leak in tree-sitter search functions
Date: Fri, 14 Apr 2023 09:45:47 +0300
> From: Yuan Fu <casouri <at> gmail.com>
> 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.

Thanks, but this kind of bugs should be fixed on the release branch to
begin with (I've backported it now).  If you are unsure on which
branch to install such changes, please ask before installing.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#62823; Package emacs. (Fri, 14 Apr 2023 18:28:02 GMT) Full text and rfc822 format available.

Message #11 received at 62823 <at> debbugs.gnu.org (full text, mbox):

From: Yuan Fu <casouri <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 62823 <at> debbugs.gnu.org
Subject: Re: bug#62823: 29.0.60; Fixing a small leak in tree-sitter search
 functions
Date: Fri, 14 Apr 2023 11:27:22 -0700

> On Apr 13, 2023, at 11:45 PM, Eli Zaretskii <eliz <at> gnu.org> wrote:
> 
>> From: Yuan Fu <casouri <at> gmail.com>
>> 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.
> 
> Thanks, but this kind of bugs should be fixed on the release branch to
> begin with (I've backported it now).  If you are unsure on which
> branch to install such changes, please ask before installing.

Oops. I’ll do that the next time.

Yuan



Reply sent to Stefan Kangas <stefankangas <at> gmail.com>:
You have taken responsibility. (Tue, 05 Sep 2023 23:56:01 GMT) Full text and rfc822 format available.

Notification sent to Yuan Fu <casouri <at> gmail.com>:
bug acknowledged by developer. (Tue, 05 Sep 2023 23:56:01 GMT) Full text and rfc822 format available.

Message #16 received at 62823-done <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefankangas <at> gmail.com>
To: Yuan Fu <casouri <at> gmail.com>
Cc: 62823-done <at> debbugs.gnu.org
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 <at> 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.




bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Wed, 04 Oct 2023 11:24:14 GMT) Full text and rfc822 format available.

This bug report was last modified 176 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.