classification
Title: _thread.interrupt_main() errors if SIGINT handler in SIG_DFL, SIG_IGN
Type: behavior Stage: resolved
Components: Library (Lib) Versions: Python 3.8, Python 3.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: Andre Merzky, gozdal, mcepl, minrk, miss-islington, neologix, pitrou, sxsns243, takluyver, vstinner
Priority: normal Keywords: patch

Created on 2015-02-04 23:50 by takluyver, last changed 2019-09-26 10:28 by gozdal. This issue is now closed.

Files
File name Uploaded Description Edit
issue23395.patch Andre Merzky, 2016-06-26 19:08
issue23395.2.patch Andre Merzky, 2016-06-26 20:18
interrupt_main.patch vstinner, 2016-06-27 09:08 review
Pull Requests
URL Status Linked Edit
PR 7777 closed mcepl, 2018-06-18 15:13
PR 7778 merged mcepl, 2018-06-18 15:21
PR 13541 merged miss-islington, 2019-05-24 09:06
Messages (19)
msg235412 - (view) Author: Thomas Kluyver (takluyver) * Date: 2015-02-04 23:50
In tracking down an obscure error we were seeing, we boiled it down to this test case for thread.interrupt_main():

import signal, threading, _thread, time
signal.signal(signal.SIGINT, signal.SIG_DFL) # or SIG_IGN

def thread_run():
    _thread.interrupt_main()

t = threading.Thread(target=thread_run)
t.start()
time.sleep(10)

This fails with an error message "TypeError: 'int' object is not callable", and a traceback completely disconnected from the cause of the error, presumably because it's not coming from the usual Python stack.

The problem appears to be that interrupt_main sets (in the C code) Handlers[SIGINT].tripped, which is only expected to occur when the handler is a Python function. When PyErr_CheckSignals() runs, it tries to call Handlers[SIGINT].func as a Python function, but it's a Python integer, causing the error.

I think the fix for this is to check what Handlers[sig_num].func is, either in trip_signal() before setting Handlers[sig_num].tripped, or in PyErr_CheckSignals before calling it. I can work on a patch if desired, but I'm not brilliant at C.
msg269287 - (view) Author: Andre Merzky (Andre Merzky) * Date: 2016-06-26 14:31
Did anything ever come of this?  I also frequently stumble over that error -- but alas not in a debug setting, but on actual running code... :/
msg269289 - (view) Author: Thomas Kluyver (takluyver) * Date: 2016-06-26 14:49
As far as I know it has not been fixed.
msg269298 - (view) Author: Andre Merzky (Andre Merzky) * Date: 2016-06-26 17:21
Thanks for the PingBack, Thomas.

I am not very familiar with the Python community approach to bug reports, so can someone comment if that is worth waiting for to get fixed, or is it that a rather futile hope without providing a patch myself?  I don't think I currently have the resources to dig into the details...

Thanks, Andre.
msg269300 - (view) Author: Thomas Kluyver (takluyver) * Date: 2016-06-26 17:40
It's waiting for someone to make a patch - I'm no longer running into it, so it's not high on my priority list. Given that it's been over a year since I created this issue, it's probably not about to get fixed unless you've got some time to work on it.
msg269301 - (view) Author: Andre Merzky (Andre Merzky) * Date: 2016-06-26 17:44
Thanks Thomas.
msg269303 - (view) Author: Andre Merzky (Andre Merzky) * Date: 2016-06-26 19:08
Attached is a patch which seems to resolve the issue for me -- this now triggers the expected `KeyboardInterrupt` in the main thread.  For verification one can run the unaltered code as provided by Thomas.

I would very much appreciate feedback, to make sure that the semantics is actually what one would expect.  The patch is against the 2.7 branch from https://github.com/python/cpython.git, and I did not test it against any other branch.

I also opened a pull request (https://github.com/python/cpython/pull/39).
msg269304 - (view) Author: Thomas Kluyver (takluyver) * Date: 2016-06-26 19:20
Does Py_DECREF(arglist); need to be called in all cases? I'm genuinely unsure, as I don't usually work on C code, but my guess would be that it does.

I'm not sure whether review is now done on Github.
msg269307 - (view) Author: Andre Merzky (Andre Merzky) * Date: 2016-06-26 19:58
thanks for looking into this!  And also, thanks for the details in the original bug report -- I found this by chance, after unsuccessfully banging my head against this very problem for quite a while! 

I am not sure if the DecRef needs to be called really if the arglist is not stored or passed on.  But thanks for pointing that out, I'll check if I can find where the corresponding IncRef is called (assuming that happens somewhere).
msg269309 - (view) Author: Andre Merzky (Andre Merzky) * Date: 2016-06-26 20:18
It seems you were right, that needed a DecRef indeed: the IncRef is already called on construction.  The DecRef for the result also needed fixing - an updated patch is attached.
msg269362 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2016-06-27 09:08
Attached interrupt_main.patch fixes for _thread.interrupt_main(): raise an exception if the SIGINT signal is ignored (SIG_IGN) or not handled by Python (SIG_DFL).

My patch updates the documentation and addds unit tests.

issue23395.2.patch looks wrong: it's not the right place to handle the error.
msg269414 - (view) Author: Andre Merzky (Andre Merzky) * Date: 2016-06-28 01:15
I can confirm that the patch provided by Victor addresses the problem.  It seems to be against the current HEAD -- is there a chance that this will be backported to 2.7 (which is what I need to use)?

Thanks!  Andre.
msg269418 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2016-06-28 07:03
> It seems to be against the current HEAD -- is there a chance that this will be backported to 2.7 (which is what I need to use)?

Yes, I will fix Python 2.7 and 3.5, I will just make PyErr_SetInterruptWithErr() private (rename to _PyErr_SetInterruptWithErr()).
msg282756 - (view) Author: Wataru Matsumoto (sxsns243) * Date: 2016-12-09 03:05
Recently I've encountered the similar problem.
I found the case that SIGINT signal is ignored(set as SIG_IGN) due to the external reason.
If the python program starts background in bash script, bash set SIGINT as SIG_IGN.

test_interrupt.py
import _thread
_thread.interrupt_main()

run.sh
#!/bin/bash
python test_interrupt.py &
sleep 1s

./run.sh 
Traceback (most recent call last):
  File "test_interrupt.py", line 2, in <module>
    _thread.interrupt_main()
RuntimeError: the SIGINT signal is ignored
(it was TypeError: 'int' object is not callable before the patch)

Python mapped default_int_handler to SIG_INT on SIG_DFL in PyInit__signal.
As the python developer can't control how the program is started, it may be better to setup default_int_handler regardless the handler type.

And initially SIG_INT is mapped to default_int_handler but once signal.signal is called, the mapping is lost even though the SIG_DFL is specified.
It may need to handle SIG_INT specially in signal_signal_impl as well to keep the consistent behavior.
msg343323 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2019-05-23 20:30
New changeset 608876b6b1eb59538e6c29671a733033fb8b5be7 by Antoine Pitrou (Matěj Cepl) in branch 'master':
bpo-23395: Fix PyErr_SetInterrupt if the SIGINT signal is ignored or not handled (GH-7778)
https://github.com/python/cpython/commit/608876b6b1eb59538e6c29671a733033fb8b5be7
msg343354 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-05-24 08:21
The interrupt_main() documentation needs a "versionchanged:: 3.8" to document that the behavior changed in Python 3.8, no? (do nothing if the signal is not handled by Python).

I'm not comfortable to backport this change. I suggest to only change Python 3.8 (master branch).

While reviewing this change, I found code which should be refactored: see bpo-37031.
msg343357 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2019-05-24 09:06
It's just a bugfix to avoid a rogue exception.  It sounds gratuitous to add markers for bug fixes in the documentation.
msg343359 - (view) Author: miss-islington (miss-islington) Date: 2019-05-24 09:22
New changeset 310f414bbd4d6ed1d8813f724c91ce9b4129c0ba by Miss Islington (bot) in branch '3.7':
bpo-23395: Fix PyErr_SetInterrupt if the SIGINT signal is ignored or not handled (GH-7778)
https://github.com/python/cpython/commit/310f414bbd4d6ed1d8813f724c91ce9b4129c0ba
msg353291 - (view) Author: Marcin Gozdalik (gozdal) Date: 2019-09-26 10:28
A program which failed under Python 3.6 with

TypeError: 'int' object is not callable

still fails under Python 3.7.4 with same exception:

import signal
import os
import sys
import time

import multiprocessing

def fail():
    def handler(*args):
        pass

    while True:
        signal.signal(signal.SIGUSR1, signal.SIG_IGN)
        signal.signal(signal.SIGUSR1, handler)

proc = multiprocessing.Process(target=fail)
proc.start()
time.sleep(1)
pid = proc.pid

i = 0
try:
    while proc.is_alive():
        print("\r", end='')
        print(i, end='')
        i += 1
        os.kill(pid, signal.SIGUSR1)
        time.sleep(0.001)
finally:
    proc.join()
History
Date User Action Args
2019-09-26 10:28:13gozdalsetnosy: + gozdal
messages: + msg353291
2019-05-24 09:28:54pitrousetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2019-05-24 09:22:41miss-islingtonsetnosy: + miss-islington
messages: + msg343359
2019-05-24 09:06:45miss-islingtonsetpull_requests: + pull_request13455
2019-05-24 09:06:36pitrousetmessages: + msg343357
versions: + Python 3.7, Python 3.8, - Python 2.7, Python 3.5
2019-05-24 08:21:22vstinnersetmessages: + msg343354
2019-05-23 20:30:05pitrousetnosy: + pitrou
messages: + msg343323
2018-06-18 15:21:02mceplsetpull_requests: + pull_request7385
2018-06-18 15:13:32mceplsetstage: patch review
pull_requests: + pull_request7384
2018-06-18 14:17:31mceplsetnosy: + mcepl
2016-12-09 03:05:34sxsns243setmessages: + msg282756
2016-12-09 01:34:18sxsns243setnosy: + sxsns243
2016-06-28 07:03:21vstinnersetmessages: + msg269418
2016-06-28 01:15:40Andre Merzkysetmessages: + msg269414
2016-06-27 09:08:40vstinnersetfiles: + interrupt_main.patch

messages: + msg269362
2016-06-26 20:18:53Andre Merzkysetfiles: + issue23395.2.patch

messages: + msg269309
2016-06-26 19:58:58Andre Merzkysetmessages: + msg269307
2016-06-26 19:20:17takluyversetmessages: + msg269304
2016-06-26 19:08:33Andre Merzkysetfiles: + issue23395.patch
keywords: + patch
messages: + msg269303
2016-06-26 17:44:04Andre Merzkysetmessages: + msg269301
2016-06-26 17:40:16takluyversetmessages: + msg269300
2016-06-26 17:24:07Andre Merzkysetversions: + Python 2.7
2016-06-26 17:21:57Andre Merzkysetmessages: + msg269298
2016-06-26 14:49:33takluyversetmessages: + msg269289
2016-06-26 14:31:02Andre Merzkysetnosy: + Andre Merzky
messages: + msg269287
2015-02-04 23:57:05pitrousetnosy: + vstinner, neologix
2015-02-04 23:50:50takluyvercreate