FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

gh-124703: Do not raise an exception when quitting pdb by gaogaotiantian · Pull Request #124704 · python/cpython · GitHub

/ cpython Public

gh-124703: Do not raise an exception when quitting pdb - #124704

Merged
gaogaotiantian merged 5 commits into
python:mainfrom
gaogaotiantian:pdb-exit-polish
Jan 27, 2025
ConversationCommits (5)ChecksFiles changed
Merged

gh-124703: Do not raise an exception when quitting pdb#124704
gaogaotiantian merged 5 commits into
python:mainfrom
gaogaotiantian:pdb-exit-polish

Conversation

gaogaotiantian commented Sep 27, 2024 •
edited by bedevere-app Bot
Loading

Copy link
Copy Markdown
Member

There are a few design details, which are open to discuss:

  1. lldb does not trigger confirm prompt on Ctrl+D, only on commands. gdb triggers on all. I prefer consistency so pdb will trigger confirmation in all cases.
  2. We don't want to make the quitting process too cumbersome for users, so there are more than one way to confirm:
    • y/Y as suggested in the prompt
    • <enter> so you can do q, <enter>, <enter>
    • Ctrl+D so you can do Ctrl+D, Ctrl+D
  3. The latter two are not listed in the prompt because the prompt would be a bit confusing. That's how gdb and lldb does it as well. (they do have slightly different policies on some input).
  4. n/N will return to debugger, all other inputs brings you back to the prompt
  5. os._exit(0) vs sys.exit(0). I gave some thoughts about how we should exit, do we want to do it gracefully. I chose the os._exit(0) at the end because I think when the users attach a debugger and want to quit, they don't care about whether the process will end gracefully (with all the atexit callbacks and potential exit routines), they just want to stop the process. If they want the process to run to the end, they should do c instead of q.
    In rare cases, where a separate non-daemon thread is there or the code in a raw try ... except ... block, force exit would do well for a debugger.

bedevere-app Bot mentioned this pull request Sep 27, 2024
Closed
JelleZijlstra reviewed Sep 29, 2024
View reviewed changes
Comment thread Lib/pdb.py Outdated
reply = 'y'
self.message('')
if reply == 'y' or reply == '':
os._exit(0)

JelleZijlstra Sep 29, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I think I'd prefer sys.exit here. os._exit may lead to unreleased resources. If the user wants to kill the process faster, they can hit Ctrl-C or Ctrl-\ after.

gaogaotiantian Sep 29, 2024

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

It's always possible to have unreleased resources - we can't prevent that with SystemExit. It may get better in some cases, but raising SystemExit in an arbitrary place of the code does not seem like a very safe way to end the program to me. The only way to make sure all resources are released (if the program is written correctly) is to continue the program.

One of the problem of SystemExit is:

while True:
    try:
        breakpoint()
    except:
        pass

This will trap in debugger forever. I know this example is a bit artificial, but it's not that rare for programs to handle SystemExit, and it's frustrating for users to be stuck in the debugger when they just want to quit.

We have a warning for the users already and they should be aware that they are "killing" a process - which means the resources could potentially be leaked. At least they'll know the process will definitely be killed after they say yes.

Of course that's my thought, and is open to more discussion.

gaogaotiantian commented Oct 8, 2024

Copy link
Copy Markdown
Member Author

I'm having second thought about this. Not because of the resource release, but it seems like some people will bring up pdb in REPL, for example by running some code with breakpoint() in it. Force quit will kill REPL as well. So maybe SystemExit would be a better choice and it kind of provides a similar backwards compatibility to BdbQuit.

gaogaotiantian commented Oct 15, 2024

Copy link
Copy Markdown
Member Author

Well, obviously sys.exit(0) will kill REPL as well, maybe that's ok. I'm switching to sys.exit(0) now.

gaogaotiantian commented Oct 18, 2024

Copy link
Copy Markdown
Member Author

@iritkatriel any suggestions on this feature? Thanks!

adamchainz commented Jan 3, 2025

Copy link
Copy Markdown
Contributor

Well, obviously sys.exit(0) will kill REPL as well, maybe that's ok. I'm switching to sys.exit(0) now.

IPython, at least, catches SystemExit and tells you that your program tried to quit. So sys.exit() is better than os._exit() there.

gaogaotiantian commented Jan 26, 2025

Copy link
Copy Markdown
Member Author

Hey @iritkatriel , any suggestions on this PR? I'm just circling back to my unmerged PRs :)

gaogaotiantian merged commit 7d27561 into python:main Jan 27, 2025
gaogaotiantian deleted the pdb-exit-polish branch January 27, 2025 03:29
ADThomas-astro mentioned this pull request Feb 7, 2025
Closed
mayfield mentioned this pull request Jan 14, 2026
Open
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

JelleZijlstra JelleZijlstra left review comments
iritkatriel iritkatriel approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

Back | FazBrowse Home | New Git URL