_thread.interrupt_main() errors if SIGINT handler in SIG_DFL, SIG_IGN

[takluyver]

BPO	23395
Nosy	@pitrou, @vstinner, @mcepl, @takluyver, @minrk, @miss-islington, @gozdal
PRs	WIP: bpo-23395: Raise an exception if the SIGINT signal is ignored or not handled #7777 bpo-23395: Raise an exception if the SIGINT signal is ignored or not handled #7778 [3.7] bpo-23395: Fix PyErr_SetInterrupt if the SIGINT signal is ignored or not handled (GH-7778) #13541
Files	issue23395.patch issue23395.2.patch interrupt_main.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 = <Date 2019-05-24.09:28:54.994>
created_at = <Date 2015-02-04.23:50:50.367>
labels = ['3.7', '3.8', 'type-bug', 'library']
title = '_thread.interrupt_main() errors if SIGINT handler in SIG_DFL, SIG_IGN'
updated_at = <Date 2021-03-04.23:22:55.101>
user = 'https://github.com/takluyver'

bugs.python.org fields:

activity = <Date 2021-03-04.23:22:55.101>
actor = 'pitrou'
assignee = 'none'
closed = True
closed_date = <Date 2019-05-24.09:28:54.994>
closer = 'pitrou'
components = ['Library (Lib)']
creation = <Date 2015-02-04.23:50:50.367>
creator = 'takluyver'
dependencies = []
files = ['43544', '43546', '43560']
hgrepos = []
issue_num = 23395
keywords = ['patch']
message_count = 20.0
messages = ['235412', '269287', '269289', '269298', '269300', '269301', '269303', '269304', '269307', '269309', '269362', '269414', '269418', '282756', '343323', '343354', '343357', '343359', '353291', '388136']
nosy_count = 10.0
nosy_names = ['pitrou', 'vstinner', 'mcepl', 'neologix', 'takluyver', 'minrk', 'Andre Merzky', 'sxsns243', 'miss-islington', 'gozdal']
pr_nums = ['7777', '7778', '13541']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue23395'
versions = ['Python 3.7', 'Python 3.8']

[takluyver]

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.

[AndreMerzky]

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... :/

[takluyver]

As far as I know it has not been fixed.

[AndreMerzky]

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.

[takluyver]

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.

[AndreMerzky]

Thanks Thomas.

[AndreMerzky]

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 (#39).

[takluyver]

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.

[AndreMerzky]

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).

[AndreMerzky]

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.

[vstinner]

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.

bpo-23395.2.patch looks wrong: it's not the right place to handle the error.

[AndreMerzky]

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.

[vstinner]

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()).

[sxsns243]

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.

[pitrou]

New changeset 608876b 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)
608876b

[vstinner]

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.

[pitrou]

It's just a bugfix to avoid a rogue exception. It sounds gratuitous to add markers for bug fixes in the documentation.

[miss-islington]

New changeset 310f414 by Miss Islington (bot) in branch '3.7':
bpo-23395: Fix PyErr_SetInterrupt if the SIGINT signal is ignored or not handled (GH-7778)
310f414

[gozdal]

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()

[pitrou]

Turns out this didn't fix all possible situations. See bpo-43406 for an updated patch.