GNU bug report logs - #58780
python.el infinite loop in info-current-defun

Previous Next

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

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

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

From: JD Smith <jdtsmith <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: python.el infinite loop in info-current-defun
Date: Tue, 25 Oct 2022 15:06:36 -0400

[Message part 1 (text/plain, inline)]

I noticed python.el hangs hard on some of my files when `python-info-current-defun` is set up for use with which-function, and I open (but haven’t yet closed) a triple string: “”".  I tracked this down to a bug in `python-nav-end-of-statement` when an unclosed string is included in a file:

==========
def try():
    """Do the Foo

def a():
    """Do A's stuff"""
    a = True

===========

(Note: the final newline is important).  (python-info-current-defun) hangs on the unclosed docsig string of try().  

The reason why can be demonstrated by placing the cursor before a = True on the final line and (python-nav-end-of-statement).  Point moves to the end of the previous line!   Since `python-nav-end-of-defun` calls end-of-statement repeatedly looking for (eobp), this results in an infinite loop.  The problem is this call in end-of-statement:

  (re-search-forward (rx (syntax string-delimiter)) nil t)

Search starts at the single apostrophe in Do A’s stuff (the beginning of the apparent-but-incorrect string ppss has found), then searches forward to the triple quote at the end of the (prior) line.  

To reproduce this you need:

- an unclosed triple string above
- a triple string with another type of quote mark enclosed
- something after the final “”” (to prevent eobp). 

These are surprisingly common conditions to encounter given python docstring format.  A fix might be to insist that the `python-nav-end-of-statement` occurs at least at the end of the current line, or perhaps to improve the regex search for the end of string to match the opening string delimiter (although this could also be fooled I think).

This is Emacs 28, though aside from some additional commentary about such issues, end-of-statement hasn’t changed in the latest. 

As an aside, having stepped through this code, it seems python’s structural navigation and inspection are _very_ heavy, commonly traversing entire files one statement at a time to find the local function name, for example.  Due to their complexity, they are also susceptible to these types of infinite loops when syntax is in a temporarily broken state.  Good arguments for the inclusion of tree-sitter!

[Message part 2 (text/html, inline)]

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

From: Ryan B <pair <at> ryanb.org>
To: 58780 <at> debbugs.gnu.org
Subject: python.el infinite loop in info-current-defun
Date: Mon, 5 Dec 2022 06:44:53 -0800

[Message part 1 (text/plain, inline)]

I think I have a repro for this with a single quote ' , as opposed to
triple string. I think I started seeing this hang after upgrading to Emacs
28.2. I now hit it very often, it's pretty painful. I don't know for sure
that it's the same bug, but it seems likely. Reproduces with emacs -Q:

======
class Foo():

  def __init__(self):
    '

  def bar(self):
    """Fetches posts and converts them to ActivityStreams activities.
    See source.Source.get_activities_response for details. app_id is
    ignored. min_id is translated to Twitter's since_id.
    """
    pass
======

The single quote and multiple lines in the docstring are necessary for the
repro. No extra newline at the end needed though.

JD, do you have a workaround for this? I may look into overriding
python-nav-end-of-defun until it's fixed. Any other ideas?

My emacs-version: GNU Emacs 28.2 (build 1, aarch64-apple-darwin21.1.0, NS
appkit-2113.00 Version 12.0.1 (Build 21A559)) of 2022-09-12

-- 
https://snarfed.org/

[Message part 2 (text/html, inline)]

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

From: Ryan B <pair <at> ryanb.org>
To: 58780 <at> debbugs.gnu.org
Subject: Re: python.el infinite loop in info-current-defun
Date: Mon, 5 Dec 2022 06:57:31 -0800

[Message part 1 (text/plain, inline)]

Confirmed, probably the same root cause. When I brute force override
python-nav-end-of-statement like below, it doesn't happen any more.

(defun python-nav-end-of-statement (&optional noend)
  (interactive "^")
  (forward-line 1))


On Mon, Dec 5, 2022 at 6:44 AM Ryan B <pair <at> ryanb.org> wrote:

> I think I have a repro for this with a single quote ' , as opposed to
> triple string. I think I started seeing this hang after upgrading to Emacs
> 28.2. I now hit it very often, it's pretty painful. I don't know for sure
> that it's the same bug, but it seems likely. Reproduces with emacs -Q:
>
> ======
> class Foo():
>
>   def __init__(self):
>     '
>
>   def bar(self):
>     """Fetches posts and converts them to ActivityStreams activities.
>     See source.Source.get_activities_response for details. app_id is
>     ignored. min_id is translated to Twitter's since_id.
>     """
>     pass
> ======
>
> The single quote and multiple lines in the docstring are necessary for the
> repro. No extra newline at the end needed though.
>
> JD, do you have a workaround for this? I may look into overriding
> python-nav-end-of-defun until it's fixed. Any other ideas?
>
> My emacs-version: GNU Emacs 28.2 (build 1, aarch64-apple-darwin21.1.0, NS
> appkit-2113.00 Version 12.0.1 (Build 21A559)) of 2022-09-12
>
> --
> https://snarfed.org/
>


-- 
https://snarfed.org/

[Message part 2 (text/html, inline)]

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

From: Ryan B <pair <at> ryanb.org>
To: 58780 <at> debbugs.gnu.org
Subject: Re: python.el infinite loop in info-current-defun
Date: Mon, 5 Dec 2022 07:01:19 -0800

[Message part 1 (text/plain, inline)]

I take it back, that override doesn't fully prevent it after all. 😢

On Mon, Dec 5, 2022 at 6:57 AM Ryan B <pair <at> ryanb.org> wrote:

> Confirmed, probably the same root cause. When I brute force override
> python-nav-end-of-statement like below, it doesn't happen any more.
>
> (defun python-nav-end-of-statement (&optional noend)
>   (interactive "^")
>   (forward-line 1))
>
>
> On Mon, Dec 5, 2022 at 6:44 AM Ryan B <pair <at> ryanb.org> wrote:
>
>> I think I have a repro for this with a single quote ' , as opposed to
>> triple string. I think I started seeing this hang after upgrading to Emacs
>> 28.2. I now hit it very often, it's pretty painful. I don't know for sure
>> that it's the same bug, but it seems likely. Reproduces with emacs -Q:
>>
>> ======
>> class Foo():
>>
>>   def __init__(self):
>>     '
>>
>>   def bar(self):
>>     """Fetches posts and converts them to ActivityStreams activities.
>>     See source.Source.get_activities_response for details. app_id is
>>     ignored. min_id is translated to Twitter's since_id.
>>     """
>>     pass
>> ======
>>
>> The single quote and multiple lines in the docstring are necessary for
>> the repro. No extra newline at the end needed though.
>>
>> JD, do you have a workaround for this? I may look into overriding
>> python-nav-end-of-defun until it's fixed. Any other ideas?
>>
>> My emacs-version: GNU Emacs 28.2 (build 1, aarch64-apple-darwin21.1.0, NS
>> appkit-2113.00 Version 12.0.1 (Build 21A559)) of 2022-09-12
>>
>> --
>> https://snarfed.org/
>>
>
>
> --
> https://snarfed.org/
>


-- 
https://snarfed.org/

[Message part 2 (text/html, inline)]

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

From: Ryan B <pair <at> ryanb.org>
To: 58780 <at> debbugs.gnu.org
Subject: Re: python.el infinite loop in info-current-defun
Date: Mon, 5 Dec 2022 12:32:23 -0800

[Message part 1 (text/plain, inline)]

Advice seems to work better at preventing the hang:

(defun python-nav-end-of-statement--bug-override (orig-fn &optional noend)
  (forward-line 1))
(advice-add 'python-nav-end-of-statement :around
#'python-nav-end-of-statement--bug-override)

On Mon, Dec 5, 2022 at 7:01 AM Ryan B <pair <at> ryanb.org> wrote:

> I take it back, that override doesn't fully prevent it after all. 😢
>
> On Mon, Dec 5, 2022 at 6:57 AM Ryan B <pair <at> ryanb.org> wrote:
>
>> Confirmed, probably the same root cause. When I brute force override
>> python-nav-end-of-statement like below, it doesn't happen any more.
>>
>> (defun python-nav-end-of-statement (&optional noend)
>>   (interactive "^")
>>   (forward-line 1))
>>
>>
>> On Mon, Dec 5, 2022 at 6:44 AM Ryan B <pair <at> ryanb.org> wrote:
>>
>>> I think I have a repro for this with a single quote ' , as opposed to
>>> triple string. I think I started seeing this hang after upgrading to Emacs
>>> 28.2. I now hit it very often, it's pretty painful. I don't know for sure
>>> that it's the same bug, but it seems likely. Reproduces with emacs -Q:
>>>
>>> ======
>>> class Foo():
>>>
>>>   def __init__(self):
>>>     '
>>>
>>>   def bar(self):
>>>     """Fetches posts and converts them to ActivityStreams activities.
>>>     See source.Source.get_activities_response for details. app_id is
>>>     ignored. min_id is translated to Twitter's since_id.
>>>     """
>>>     pass
>>> ======
>>>
>>> The single quote and multiple lines in the docstring are necessary for
>>> the repro. No extra newline at the end needed though.
>>>
>>> JD, do you have a workaround for this? I may look into overriding
>>> python-nav-end-of-defun until it's fixed. Any other ideas?
>>>
>>> My emacs-version: GNU Emacs 28.2 (build 1, aarch64-apple-darwin21.1.0,
>>> NS appkit-2113.00 Version 12.0.1 (Build 21A559)) of 2022-09-12
>>>
>>> --
>>> https://snarfed.org/
>>>
>>
>>
>> --
>> https://snarfed.org/
>>
>
>
> --
> https://snarfed.org/
>


-- 
https://snarfed.org/

[Message part 2 (text/html, inline)]

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

From: kobarity <kobarity <at> gmail.com>
To: JD Smith <jdtsmith <at> gmail.com>, Ryan B <pair <at> ryanb.org>, Tom
 Gillespie <tgbugs <at> gmail.com>, 58780 <at> debbugs.gnu.org
Subject: Re: python.el infinite loop in info-current-defun
Date: Sun, 05 Mar 2023 17:19:43 +0900

[Message part 1 (text/plain, inline)]

As JD Smith pointed out, this problem is caused by
`python-nav-end-of-statement' moving point backward.  A simple example
illustrating the problem of `python-nav-end-of-statement' is as
follows:

#+begin_src python
' """
a = 1
#+end_src

When point is placed on the second line and
`python-nav-end-of-statement' is executed, point will move to the end
of the first line.

Here is why this happens.

`python-nav-end-of-statement' moves point to the end of the second
line, and checks if it is in the middle of the string using:

(python-syntax-context 'string)

It tells that the point is within the string started at the
single-quote in the first line, and the variable `string-start' is set
to the start of the string (single-quote in the first line).  Then,
point is moved to the start of the string, and the end of the string
is searched for using:

(re-search-forward (rx (syntax string-delimiter)) nil t)

It finds the triple double-quotes and moves point immediately after
them.  The variable `last-string-end' is set to this position.

In the second iteration of the while loop, the variable `string-start'
is set to the single-quote in the first line again.  This results in
exiting the loop because the following condition is not met.

(>= string-start last-string-end)

There are several issues here:

1. Although `python-syntax-context' detects both string-quote
(single quote) and string-delimiter (triple quotes) as the start of
string, only string-delimiter is searched for as the end of string.
2. The triple double-quotes is marked as string-delimiter even though
they are within the string begins with the single-quote.
3. As the end of the first line is not escaped using a backslash, the
string begins with the single-quote should not be continued on the
second line.

I would like to discuss the second and third issues separately.  The
first issue is the main cause of Bug#58780.  This issue also causes
another type of problem, illustrated by the following example.

#+begin_src python
abc = 'a\
b\
c'
d = '''d'''
#+end_src

When `python-nav-end-of-statement' is executed with point located at
the second line, point is moved to the end of the fourth line instead
of the third line.  This is a wrong point move for a correct Python
program.

The attached
0001-Fix-searching-for-end-of-string-in-python-nav-end-of.patch is my
proposal to fix this issue with some ERTs.  `forward-sexp' (in fact
`python-nav--lisp-forward-sexp') is added to search for the end of the
string if the string begins with single single/double-quote (' or ").

I think this 0001 patch also fixes Bug#56271.  At least, it passes the
ERT python-nav-end-of-block-2 which is introduced in Bug#56271, even
the workaround introduced in Bug#56271 is reverted.  The attached
0002-Revert-workaround-introduced-in-Bug-56271.patch is a patch to
revert the workaround.  If the 0001 patch is accepted, the 0002 patch
can also be applied.  But if we want to be safer, we can leave the
workaround as it is by not applying the 0002 patch.

I look forward to your comments.

[0001-Fix-searching-for-end-of-string-in-python-nav-end-of.patch (application/octet-stream, attachment)]

[0002-Revert-workaround-introduced-in-Bug-56271.patch (application/octet-stream, attachment)]

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: kobarity <kobarity <at> gmail.com>
Cc: tgbugs <at> gmail.com, pair <at> ryanb.org, jdtsmith <at> gmail.com,
 58780-done <at> debbugs.gnu.org
Subject: Re: bug#58780: python.el infinite loop in info-current-defun
Date: Thu, 09 Mar 2023 12:16:37 +0200

> Date: Sun, 05 Mar 2023 17:19:43 +0900
> From: kobarity <kobarity <at> gmail.com>
> 
> The attached
> 0001-Fix-searching-for-end-of-string-in-python-nav-end-of.patch is my
> proposal to fix this issue with some ERTs.  `forward-sexp' (in fact
> `python-nav--lisp-forward-sexp') is added to search for the end of the
> string if the string begins with single single/double-quote (' or ").
> 
> I think this 0001 patch also fixes Bug#56271.  At least, it passes the
> ERT python-nav-end-of-block-2 which is introduced in Bug#56271, even
> the workaround introduced in Bug#56271 is reverted.  The attached
> 0002-Revert-workaround-introduced-in-Bug-56271.patch is a patch to
> revert the workaround.  If the 0001 patch is accepted, the 0002 patch
> can also be applied.  But if we want to be safer, we can leave the
> workaround as it is by not applying the 0002 patch.
> 
> I look forward to your comments.

There were no more comments, so I've now installed both patches on the
emacs-29 branch, and I'm boldly closing this bug.

Thanks.

Previous Next

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