classification
Title: Default nosigint option to pdb.Pdb() prevents use in non-main thread
Type: behavior Stage: resolved
Components: Library (Lib) Versions: Python 3.4, Python 3.2, Python 3.3
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: asvetlov Nosy List: asvetlov, bpb, georg.brandl, icemac, isandler, meador.inge, python-dev, skrah
Priority: normal Keywords: patch

Created on 2011-10-06 21:17 by bpb, last changed 2012-12-05 13:43 by asvetlov. This issue is now closed.

Files
File name Uploaded Description Edit
thread_sigint.patch isandler, 2011-11-27 03:10 review
test_pdb.py.patch isandler, 2011-11-28 04:08 review
issue13120.txt asvetlov, 2012-12-03 00:34 review
Messages (14)
msg145040 - (view) Author: Ben Bass (bpb) Date: 2011-10-06 21:17
The new SIGINT behaviour of pdb.Pdb prevents use of pdb within a non-main thread without explicitly setting nosigint=True. Specifically the 'continue' command causes a traceback as follows:

{{{
...
  File "/Library/Frameworks/Python.framework/Versions/3.2/lib/python3.2/pdb.py", line 959, in do_continue
    signal.signal(signal.SIGINT, self.sigint_handler)
ValueError: signal only works in main thread
}}}

Since the new behaviour seems to be to gain an enhancement rather than anything fundamentally necessary to pdb, wouldn't it be better if the default was reversed, so the same code would work identically on Python 3.1 (and potentially earlier, i.e. Python2) and Python 3.2?

At the moment in my codebase (rpcpdb) I'm using inspect.getargspec sniffing for nosigint on pdb.Pdb.__init__ to determine whether to include a nosigint=True parameter, which clearly isn't ideal!
msg148439 - (view) Author: Ilya Sandler (isandler) Date: 2011-11-27 03:10
I confirm the bug. But I don't think disabling Ctrl-C (SIGINT) handling by default is a good idea. Proper Ctrl-C support seems like a fundamental feature for a command line debugger. 

However, I think the bug is easily fixable w/o changing SIGINT handling. Basically, just put try/except around signal.signal, catch the ValueError and proceed. Would this approach solve your problem?

Patch attached.

PS. and here is a small program demonstrating the bug (just run it and execute "c" command at pdb prompt)

 import threading
 import pdb

 def start_pdb():
    pdb.Pdb().set_trace()
    x = 1
    y = 1

 t = threading.Thread( target=start_pdb)
 t.start()
msg148457 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2011-11-27 20:09
Patch seems fine to me.
msg148463 - (view) Author: Meador Inge (meador.inge) * (Python committer) Date: 2011-11-27 23:26
Normally a test case is needed.  I tried the following:

diff --git a/Lib/test/test_pdb.py b/Lib/test/test_pdb.py
--- a/Lib/test/test_pdb.py
+++ b/Lib/test/test_pdb.py
@@ -595,6 +595,25 @@ def test_pdb_run_with_code_object():
     (Pdb) continue
     """
 
+def test_pdb_continue_in_thread():
+    """Testing the ability to continue from a non-main thread.
+
+    >>> import threading
+    >>>
+    >>> def start_pdb():
+    ...     import pdb
+    ...     pdb.Pdb().set_trace()
+    ...     x = 1
+    ...     y = 1
+    ...
+
+    >>> with PdbTestInput(['continue']):
+    ...    t = threading.Thread(target=start_pdb)
+    ...    t.start()
+    > <doctest test.test_pdb.test_pdb_continue_in_thread[1]>(4)start_pdb()
+    -> x = 1
+    (Pdb) continue
+"""

but 'doctests' doesn't seem to play nicely with threading.  The pdb testsuite fails with:


[meadori@motherbrain cpython]$ ./python -m test test_pdb
[1/1] test_pdb
**********************************************************************
File "/home/meadori/code/python/cpython/Lib/test/test_pdb.py", line 610, in test.test_pdb.test_pdb_continue_in_thread
Failed example:
    with PdbTestInput(['continue']):
       t = threading.Thread(target=start_pdb)
       t.start()
Expected:
    > <doctest test.test_pdb.test_pdb_continue_in_thread[1]>(4)start_pdb()
    -> x = 1
    (Pdb) continue
Got nothing
**********************************************************************
File "/home/meadori/code/python/cpython/Lib/test/test_pdb.py", line 34, in test.test_pdb.test_pdb_displayhook
Failed example:
    def test_function(foo, bar):
        import pdb; pdb.Pdb(nosigint=True).set_trace()
        pass
Expected nothing
Got:
    > <doctest test.test_pdb.test_pdb_continue_in_thread[1]>(4)start_pdb()
Exception in thread Thread-1:

It looks like the output is going to the wrong test.

Anyone else have test ideas?  If not, then I guess it is OK to commit as is.
msg148466 - (view) Author: Ilya Sandler (isandler) Date: 2011-11-28 04:08
I think stuff like this can only be tested out-of-process.

So I added an out-of-process test to test_pdb.py. The test passes with the fixed pdb.py. (and fails with the original one).

Patch for the test attached.
msg148705 - (view) Author: Meador Inge (meador.inge) * (Python committer) Date: 2011-12-01 05:26
Ilya, I agree.  Thanks for the test patch.  These two patches look OK to me.  Georg OK with you?
msg176806 - (view) Author: (icemac) Date: 2012-12-02 19:47
This bug bites me when developing with Pyramid using Python 3.2.
I'd like to get it solved generally as the patch works for me (Python 3.2.3)).

What needs to be done to get this patch into the version control system?
msg176822 - (view) Author: Andrew Svetlov (asvetlov) * (Python committer) Date: 2012-12-03 00:34
Uploaded updated cumulative patch.

Can we apply the patch to 3.2 or at least to 3.3?

I see nothing wrong with it, but I'm ok with pushing to 3.4 only if we want to be extra careful.
msg176931 - (view) Author: Roundup Robot (python-dev) Date: 2012-12-04 19:11
New changeset 708586792eec by Andrew Svetlov in branch '3.2':
Issue #13120: Allow to call pdb.set_trace() from thread.
http://hg.python.org/cpython/rev/708586792eec

New changeset 678dba60c12d by Andrew Svetlov in branch '3.3':
Merge issue #13120: Allow to call pdb.set_trace() from thread.
http://hg.python.org/cpython/rev/678dba60c12d

New changeset 4006c4ca0c1f by Andrew Svetlov in branch 'default':
Merge issue #13120: Allow to call pdb.set_trace() from thread.
http://hg.python.org/cpython/rev/4006c4ca0c1f
msg176932 - (view) Author: Andrew Svetlov (asvetlov) * (Python committer) Date: 2012-12-04 19:14
Fixed. Thanks.
msg176977 - (view) Author: Stefan Krah (skrah) * (Python committer) Date: 2012-12-05 11:56
I think this commit broke the --without-threads buildbot:

http://buildbot.python.org/all/builders/AMD64%20Fedora%20without%20threads%203.x/builds/3581/steps/test/logs/stdio
msg176979 - (view) Author: Andrew Svetlov (asvetlov) * (Python committer) Date: 2012-12-05 12:41
Will take a look.
msg176980 - (view) Author: Roundup Robot (python-dev) Date: 2012-12-05 13:07
New changeset 26056f8a0afe by Andrew Svetlov in branch '3.2':
Skip pdb test for #13120 if threading is not available.
http://hg.python.org/cpython/rev/26056f8a0afe

New changeset 328a8824c1a7 by Andrew Svetlov in branch '3.3':
Merge: skip pdb test for #13120 if threading is not available.
http://hg.python.org/cpython/rev/328a8824c1a7

New changeset 4cb84c0fbee2 by Andrew Svetlov in branch 'default':
Merge: skip pdb test for #13120 if threading is not available.
http://hg.python.org/cpython/rev/4cb84c0fbee2
msg176983 - (view) Author: Andrew Svetlov (asvetlov) * (Python committer) Date: 2012-12-05 13:43
Failed test is skipped if there are no threads.
Thanks for report, Stefan!
History
Date User Action Args
2012-12-05 13:43:13asvetlovsetmessages: + msg176983
2012-12-05 13:07:20python-devsetmessages: + msg176980
2012-12-05 12:41:09asvetlovsetmessages: + msg176979
2012-12-05 11:56:45skrahsetnosy: + skrah
messages: + msg176977
2012-12-04 19:14:02asvetlovsetstatus: open -> closed
resolution: fixed
messages: + msg176932

stage: patch review -> resolved
2012-12-04 19:11:07python-devsetnosy: + python-dev
messages: + msg176931
2012-12-03 00:34:26asvetlovsetfiles: + issue13120.txt
assignee: asvetlov
messages: + msg176822

versions: + Python 3.4
2012-12-02 22:21:08asvetlovsetnosy: + asvetlov
2012-12-02 19:47:20icemacsetnosy: + icemac
messages: + msg176806
2011-12-01 05:26:00meador.ingesetmessages: + msg148705
2011-11-28 04:08:36isandlersetfiles: + test_pdb.py.patch

messages: + msg148466
2011-11-27 23:26:51meador.ingesetmessages: + msg148463
2011-11-27 20:09:10georg.brandlsetmessages: + msg148457
2011-11-27 20:00:46pitrousetnosy: + georg.brandl
stage: patch review

versions: + Python 3.3
2011-11-27 17:19:28meador.ingesetnosy: + meador.inge
2011-11-27 03:10:58isandlersetfiles: + thread_sigint.patch

nosy: + isandler
messages: + msg148439

keywords: + patch
2011-10-06 21:19:19bpbsettitle: Default nosigint optionto pdb.Pdb() prevents use in non-main thread -> Default nosigint option to pdb.Pdb() prevents use in non-main thread
2011-10-06 21:17:27bpbcreate