allow to break into pdb with Ctrl-C for all the commands that resume execution

[xdegaye]

BPO	22135
Nosy	@birkenfeld, @blueyed, @xdegaye, @devjoe
PRs	bpo-22135: Allow to break into pdb with Ctrl-C for all the commands t… #13269
Files	fix-cc-and-add-tests.patch fix-cc-and-add-tests-v2.patch

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = None
created_at = <Date 2014-08-04.20:44:21.500>
labels = ['3.7', 'type-bug', 'library']
title = 'allow to break into pdb with Ctrl-C for all the commands that resume execution'
updated_at = <Date 2019-05-12.21:23:27.973>
user = 'https://github.com/xdegaye'

bugs.python.org fields:

activity = <Date 2019-05-12.21:23:27.973>
actor = 'Chun-Yu Tseng'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Library (Lib)']
creation = <Date 2014-08-04.20:44:21.500>
creator = 'xdegaye'
dependencies = []
files = ['45913', '45928']
hgrepos = []
issue_num = 22135
keywords = ['patch']
message_count = 8.0
messages = ['224772', '283293', '283330', '283428', '284820', '284829', '340538', '340734']
nosy_count = 4.0
nosy_names = ['georg.brandl', 'blueyed', 'xdegaye', 'Chun-Yu Tseng']
pr_nums = ['13269']
priority = 'normal'
resolution = None
stage = 'patch review'
status = 'open'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue22135'
versions = ['Python 3.7']

[xdegaye]

Pdb sets a handler for the SIGINT signal (which is sent when the user presses Ctrl-C on the console) when you give a continue command.
'continue' is not the only pdb command that may be interrupted, all the commands that resume the execution of the program (i.e. the 'do_xxx' Pdb methods that return 1), except for the ones that terminate pdb, should also set the SIGINT signal handler. These are the 'step', 'next', 'until' and 'return' commands.
For example, a 'next' command issued at the invocation of a function when the function is doing a long processing and the user wishes to break into pdb again with Ctrl-C within this function.

It is probably better to fix this issue after bpo-20766 is fixed.

[devjoe]

Here comes the patch:

1. Let Pdb can also resume from return, until and next commands when receiving Control-C.

2. Explicitly raise bdb.BdbQuit when an unexpected exception occurs in sigint_handler. See bpo-24283.

3. Add two tests test_break_during_interactive_input, test_resume_from_sigint and some helper functions to test_pdb.py to make sure that Pdb behaves as we expected.
   See below, Pdb resumes back in a wrong position when receiving Control-C. The environment is Python 3.5.2 (Mac). But Pdb works right in latest 3.5/3.6/default now. So we should have tests here.
   (Pdb) list
   1 import time
   2 def f():
   3 import pdb; pdb.Pdb().set_trace();
   4 -> delay()
   5 print("***** f() done *****")
   6
   7 def delay():
   8 time.sleep(3)
   (Pdb) c
   ^C
   Program interrupted. (Use 'cont' to resume).
   --Call--

/usr/local/var/pyenv/versions/3.5.2/lib/python3.5/signal.py(45)signal()
-> @_wraps(_signal.signal)
(Pdb)

What this patch does NOT do are:

1. Let Pdb can resume from step command.
   I tried by the same way like what I did for continue/return/until/next commands, but Pdb resumed back at the beginning of sigint_handler. The user should type in continue to go to the right place. I can't find an elegant way to work around it:
   -> time.sleep(3)
   (Pdb) s
   ^C--Call--

/Users/chun-yutseng/Projects/cpython/Lib/pdb.py(189)sigint_handler()
-> def sigint_handler(self, signum, frame):
(Pdb) l
184 self.commands_defining = False # True while in the process of defining
185 # a command list
186 self.commands_bnum = None # The breakpoint number for which we are
187 # defining a list
188
189 -> def sigint_handler(self, signum, frame):
190 if self.allow_kbdint:
191 raise KeyboardInterrupt
192 try:
193 self.message("\nProgram interrupted.")
194
(Pdb)

2. Let the two added tests can be run on Windows.
   I tried, but when I found that I may need to use Windows-specific signals (CTRL_C_EVENT/CTRL_BREAK_EVENT) in pdb.py to let automated tests pass, I decided not to introduce such complexity.
   So I use @unittest.skipIf to skip these two tests and tested the patch on Windows manually.

Call for review/advice/guides, please.

[xdegaye]

Thanks for the patch. See my comments in Rietveld.
I think we can skip the 'step' command.
For the tests, can you use the existing run_pdb() method and trigger the signals from within the code being executed with 'os.kill(os.getpid(), signal.SIGINT | signal.CTRL_C_EVENT)', this would avoid having to sleep on delays.

[devjoe]

Appreciate for your quick response. I have already left the reply in Rietveld.

I have uploaded a new patch with revised tests. In fact, the tests I wrote in the first patch are based on the style of test_pdb2.py in bpo-7245 . But I am sure that now the new tests are better than the old because they run faster and source code looks simpler. (Note that these tests are still skipped to run on Windows)

Please tell me what can I do next :)

[devjoe]

Ping :)

[xdegaye]

The code review of your first patch still applies to your last patch.

[blueyed]

Would be nice to have this indeed.
Please consider creating a PR with an updated patch then for easier review.

[devjoe]

My bad, I totally forget this patch. Need to spend some time to retest my code and catch up with current development flow. Thanks for reminding.