GH-100141: Fix Pdb loops endlessly on empty file #106467

SonOfLilit · Jul 5, 2023

The bug:

$ python3.10 -m pdb /dev/null  # good
> /dev/null(1)<module>()
(Pdb) q

$ python3.11 -m pdb /dev/null  # bad
The program finished and will be restarted
The program finished and will be restarted
The program finished and will be restarted
^CTraceback (most recent call last):
  File "/opt/homebrew/Cellar/python@3.11/3.11.2_1/Frameworks/Python.framework/Versions/3.11/lib/python3.11/pdb.py", line 1774, in main
    pdb._run(target)
  File "/opt/homebrew/Cellar/python@3.11/3.11.2_1/Frameworks/Python.framework/Versions/3.11/lib/python3.11/pdb.py", line 1652, in _run
    self.run(target.code)
             ^^^^^^^^^^^
  File "/opt/homebrew/Cellar/python@3.11/3.11.2_1/Frameworks/Python.framework/Versions/3.11/lib/python3.11/pdb.py", line 166, in code
    with io.open(self) as fp:
KeyboardInterrupt
Uncaught exception. Entering post mortem debugging
Running 'cont' or 'step' will restart the program
> /opt/homebrew/Cellar/python@3.11/3.11.2_1/Frameworks/Python.framework/Versions/3.11/lib/python3.11/pdb.py(166)code()
-> with io.open(self) as fp:
(Pdb) q
The program finished and will be restarted
The program finished and will be restarted
...

@SnoopJ had a snoop and said the change in behavior between 3.10 and 3.11 appears to be caused by #94552, which changes lineno for the first opcode in a module from -1 to 0, so we immediately suspect this code in pdb.py:

    def user_line(self, frame):
        """This function is called when we stop or break at this line."""
        if self._wait_for_mainpyfile:
            if (self.mainpyfile != self.canonic(frame.f_code.co_filename)
                or frame.f_lineno <= 0):
                return
            self._wait_for_mainpyfile = False
        if self.bp_commands(frame):
            self.interaction(frame, None)

Indeed, that frame.f_lineno <= 0 makes pdb not stop for user input when running with an empty module, and removing it doesn't affect behavior for a nonempty module. It was added in 2004 as part of the _wait_for_mainpyfile feature and wasn't touched since - at the time there must have been some other opcode with index -1 that we don't want to pause on. Honestly, I couldn't figure out why this doesn't manifest in 3.10.

Still, removing this clause seems safe as far as I can tell, and fixes the bug. @iritkatriel could I ask you to have a look? You opened the pull request that may have sparked this, and you're the one who inspired me yesterday at Pycon to take an evening and try to fix some pdb bugs.

Issue: Pdb loops endlessly on non-interactive scripts #100141

bedevere-bot · Jul 5, 2023

Most changes to Python require a NEWS entry.

Please add it using the blurb_it web app or the blurb command-line tool.

ghost · Jul 5, 2023

All commit authors signed the Contributor License Agreement.

SnoopJ · Jul 6, 2023

Confirming that this does seem to fix the problem, might be a good idea to introduce a regression test for this.

Still, removing this clause seems safe as far as I can tell, and fixes the bug.

I'll note that changing frame.f_lineno <= 0 to frame.f_lineno < 0 does also fix the issue reported in #100141. I'm not sure if f_lineno can (could previously?) ever be less than zero, it's not clear from 25b38c8, which introduced the check. I'm sure a core dev will have a better perspective on that than I do 😅

SonOfLilit · Jul 6, 2023

I felt brave and decided that instead of changing a logic clause I don't understand and probably nobody does to a conditional that should never happen, I should just remove it, that way future devs will not stare at it in confusion, and we will get faster feedback if we broke something terrible. But I worked under the assumption that a core dev will have better insight than me :-)

SonOfLilit · Jul 8, 2023

A couple of tests failed only on Windows x86 and then succeeded on second try. test_logging:test_udp_reconnection and something in test_concurrent_futures, I think a context manager that tries to remove a temp dir. Needless to say, neither seems in any way related to my one-line fix. Should I do anything? Rerun the tests?

gaogaotiantian · Jul 9, 2023

The current fix works fine for the current implementation, but it relies on the fact that a line event with f_lineno == 0 will trigger when executing an empty file. Is this a reliable behavior @markshannon @brandtbucher ?

PEP 669 says:

An instruction is about to be executed that has a different line number from the preceding instruction.

Obviously this extreme case violates the event definition. When the user executes an empty file, do we explicitly and purposely emitting a line event with line number 0? If that's a reliable feature, then this fix is good. Otherwise we need more as line event may never fire.

@SonOfLilit , in any case, we need a regression test for this. Loading an empty file is much better than using /dev/null, it's closer to what users can really experience, and it's cross-platform. Could you add a regression test for this bug?

SonOfLilit · Jul 9, 2023

Sure, I'll add one. And I guess since there is just one possible empty file, if we have a regression test we might feel comfortable with this even if it can be read as not promised by the spec? Alternatively, we can propose spec errata because obviously you'd want this event?

gaogaotiantian · Jul 9, 2023

The main concern is whether the monitoring code is working as expected. Having a f_lineno == 0 is not normal. I'm not sure if we can rely on the fact that an empty script will always trigger a line event(it also makes sense if it doesn't, as it does not have any "line"). We can deal with it with slightly more complicated code, so we should confirm.

There are actually more than one kind of empty file :). This would reproduce with any lines of comments in the "empty" file, as long as there's no compilable code.

gaogaotiantian · Oct 15, 2024

A PR (#125425) with the tests is merged for this issue. A sincere sorry to @SonOfLilit I added your name in the PR comment but forgot to add it in the commit message. Your observation and fix is greatly appreciated!

Fix: Pdb loops endlessly on empty file (pythonGH-100141)

e909402

bedevere-bot added the awaiting review label Jul 5, 2023

bedevere-bot mentioned this pull request Jul 5, 2023

Pdb loops endlessly on non-interactive scripts #100141

Closed

blurb-it bot and others added 3 commits July 6, 2023 08:28

📜🤖 Added by blurb_it.

c2d17cc

Merge branch 'main' into pythongh-100141

e5b1d00

Merge branch 'main' into pythongh-100141

6dffcb6

AlexWaygood requested a review from gaogaotiantian July 8, 2023 18:06

gaogaotiantian mentioned this pull request Oct 14, 2024

gh-100141: Allow pdb to deal with empty file #125425

Merged

gaogaotiantian closed this Oct 15, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

GH-100141: Fix Pdb loops endlessly on empty file #106467

GH-100141: Fix Pdb loops endlessly on empty file #106467

Uh oh!

SonOfLilit commented Jul 5, 2023 •

edited by bedevere-bot

Loading

Uh oh!

bedevere-bot commented Jul 5, 2023

Uh oh!

ghost commented Jul 5, 2023 •

edited by ghost

Loading

Uh oh!

SnoopJ commented Jul 6, 2023

Uh oh!

SonOfLilit commented Jul 6, 2023 •

edited

Loading

Uh oh!

SonOfLilit commented Jul 8, 2023

Uh oh!

gaogaotiantian commented Jul 9, 2023

Uh oh!

SonOfLilit commented Jul 9, 2023

Uh oh!

gaogaotiantian commented Jul 9, 2023

Uh oh!

gaogaotiantian commented Oct 15, 2024

Uh oh!

Uh oh!

Search code, repositories, users, issues, pull requests...

Uh oh!

GH-100141: Fix Pdb loops endlessly on empty file #106467

GH-100141: Fix Pdb loops endlessly on empty file #106467

Uh oh!

Conversation

SonOfLilit commented Jul 5, 2023 • edited by bedevere-bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bedevere-bot commented Jul 5, 2023

Uh oh!

ghost commented Jul 5, 2023 • edited by ghost Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

SnoopJ commented Jul 6, 2023

Uh oh!

SonOfLilit commented Jul 6, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

SonOfLilit commented Jul 8, 2023

Uh oh!

gaogaotiantian commented Jul 9, 2023

Uh oh!

SonOfLilit commented Jul 9, 2023

Uh oh!

gaogaotiantian commented Jul 9, 2023

Uh oh!

gaogaotiantian commented Oct 15, 2024

Uh oh!

Uh oh!

SonOfLilit commented Jul 5, 2023 •

edited by bedevere-bot

Loading

ghost commented Jul 5, 2023 •

edited by ghost

Loading

SonOfLilit commented Jul 6, 2023 •

edited

Loading