Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Jan 27, 2025
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 14 additions & 3 deletions Lib/pdb.py
Original file line number Diff line number Diff line change
Expand Up @@ -1723,6 +1723,19 @@ def do_quit(self, arg):

Quit from the debugger. The program being executed is aborted.
"""
if self.mode == 'inline':
while True:
try:
reply = input('Quitting pdb will kill the process. Quit anyway? [y/n] ')
reply = reply.lower().strip()
except EOFError:
reply = 'y'
self.message('')
if reply == 'y' or reply == '':
os._exit(0)
Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Member Author

Choose a reason for hiding this comment

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

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.

elif reply.lower() == 'n':
return

self._user_requested_quit = True
self.set_quit()
return 1
Expand All @@ -1736,9 +1749,7 @@ def do_EOF(self, arg):
Handles the receipt of EOF as a command.
"""
self.message('')
self._user_requested_quit = True
self.set_quit()
return 1
return self.do_quit(arg)

def do_args(self, arg):
"""a(rgs)
Expand Down
56 changes: 56 additions & 0 deletions Lib/test/test_pdb.py
Original file line number Diff line number Diff line change
Expand Up @@ -3967,6 +3967,62 @@ def test_checkline_is_not_executable(self):
self.assertFalse(db.checkline(os_helper.TESTFN, lineno))


@support.requires_subprocess()
class PdbTestInline(unittest.TestCase):
@unittest.skipIf(sys.flags.safe_path,
'PYTHONSAFEPATH changes default sys.path')
def _run_script(self, script, commands,
expected_returncode=0,
extra_env=None):
self.addCleanup(os_helper.rmtree, '__pycache__')
filename = 'main.py'
with open(filename, 'w') as f:
f.write(textwrap.dedent(script))
self.addCleanup(os_helper.unlink, filename)

commands = textwrap.dedent(commands)

cmd = [sys.executable, 'main.py']
if extra_env is not None:
env = os.environ | extra_env
else:
env = os.environ
with subprocess.Popen(
cmd,
stdout=subprocess.PIPE,
stdin=subprocess.PIPE,
stderr=subprocess.PIPE,
env = {**env, 'PYTHONIOENCODING': 'utf-8'}
) as proc:
stdout, stderr = proc.communicate(str.encode(commands))
stdout = bytes.decode(stdout) if isinstance(stdout, bytes) else stdout
stderr = bytes.decode(stderr) if isinstance(stderr, bytes) else stderr
self.assertEqual(
proc.returncode,
expected_returncode,
f"Unexpected return code\nstdout: {stdout}\nstderr: {stderr}"
)
return stdout, stderr

def test_quit(self):
script = """
x = 1
breakpoint()
"""

commands = """
quit
n
p x + 1
quit
y
"""

stdout, stderr = self._run_script(script, commands)
self.assertIn("2", stdout)
self.assertIn("Quit anyway", stdout)


@support.requires_subprocess()
class PdbTestReadline(unittest.TestCase):
def setUpClass():
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Quitting :mod:`pdb` in ``inline`` mode will emit a confirmation prompt and exit gracefully now, instead of printing an exception traceback.
Loading