Possible race condition between signal catching and signal.signal

[pitrou]

BPO	43406
Nosy	@pitrou, @vstinner, @miss-islington
PRs	bpo-43406: Fix possible race condition where PyErr_CheckSignals tries to execute a non-Python signal handler #24756 [3.9] bpo-43406: Fix possible race condition where PyErr_CheckSignals tries to execute a non-Python signal handler (GH-24756) #24761 [3.8] bpo-43406: Fix possible race condition where PyErr_CheckSignals tries to execute a non-Python signal handler (GH-24756) #24762 bpo-43356: Allow passing a signal number to interrupt_main() #24755 bpo-43406: Fix test_signal.test_stress_modifying_handlers() #24815 [3.8] bpo-43406: Fix test_signal.test_stress_modifying_handlers() (GH-24815) #24816 [3.9] bpo-43406: Fix test_signal.test_stress_modifying_handlers() (GH-24815) #24817

^{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 2021-03-10.14:30:05.963>
created_at = <Date 2021-03-04.21:31:08.922>
labels = ['interpreter-core', 'type-bug', '3.8', '3.9', '3.10', 'library']
title = 'Possible race condition between signal catching and signal.signal'
updated_at = <Date 2021-03-10.15:11:06.044>
user = 'https://github.com/pitrou'

bugs.python.org fields:

activity = <Date 2021-03-10.15:11:06.044>
actor = 'vstinner'
assignee = 'none'
closed = True
closed_date = <Date 2021-03-10.14:30:05.963>
closer = 'vstinner'
components = ['Interpreter Core', 'Library (Lib)']
creation = <Date 2021-03-04.21:31:08.922>
creator = 'pitrou'
dependencies = []
files = []
hgrepos = []
issue_num = 43406
keywords = ['patch']
message_count = 10.0
messages = ['388131', '388132', '388150', '388202', '388203', '388429', '388431', '388435', '388437', '388438']
nosy_count = 4.0
nosy_names = ['pitrou', 'vstinner', 'neologix', 'miss-islington']
pr_nums = ['24756', '24761', '24762', '24755', '24815', '24816', '24817']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue43406'
versions = ['Python 3.8', 'Python 3.9', 'Python 3.10']

[pitrou]

We can receive signals (at the C level, in trip_signal() in signalmodule.c) while signal.signal is being called to modify the corresponding handler. Later when PyErr_CheckSignals() is called to handle the given signal, the handler may be a non-callable object and will raise a cryptic asynchronous exception.

[pitrou]

Here is a reproducer:
https://gist.github.com/pitrou/e5a566e644730516b51de71145c5ea06

If you execute it, it will fail after a few iterations:

sig 2
sig 2
sig 2
Traceback (most recent call last):
  File "/home/antoine/cpython/default/setinterrupt.py", line 30, in <module>
    main()
  File "/home/antoine/cpython/default/setinterrupt.py", line 27, in main
    cycle_handlers(signum)
  File "/home/antoine/cpython/default/setinterrupt.py", line 19, in cycle_handlers
    signal.signal(signum, handler)
  File "/home/antoine/cpython/default/Lib/signal.py", line 48, in signal
    return _int_to_enum(handler, Handlers)
  File "/home/antoine/cpython/default/Lib/signal.py", line 30, in _int_to_enum
    return enum_klass(value)
  File "/home/antoine/cpython/default/Lib/enum.py", line 606, in __call__
    return cls.__new__(cls, value)
  File "/home/antoine/cpython/default/Lib/enum.py", line 927, in __new__
    ve_exc = ValueError("%r is not a valid %s" % (value, cls.__qualname__))
TypeError: 'int' object is not callable

[pitrou]

New changeset 68245b7 by Antoine Pitrou in branch 'master':
bpo-43406: Fix possible race condition where PyErr_CheckSignals tries to execute a non-Python signal handler (GH-24756)
68245b7

[miss-islington]

New changeset 1385f83 by Miss Islington (bot) in branch '3.9':
[3.9] bpo-43406: Fix possible race condition where PyErr_CheckSignals tries to execute a non-Python signal handler (GH-24756) (GH-24761)
1385f83

[miss-islington]

New changeset 4715be8 by Antoine Pitrou in branch '3.8':
[3.8] bpo-43406: Fix possible race condition where PyErr_CheckSignals tries to execute a non-Python signal handler (GH-24756) (GH-24762)
4715be8

[vstinner]

The change added a new functional test. As I expected, it failed on "AMD64 Debian root". I expected it because the previously added functional test failed failed on "AMD64 Debian root", I reported the race condition in 2017 and it's still not fixed: bpo-30849.

The new functional test has two race conditions.

I reopen the issue.

== Race condition 1 ==

AMD64 Debian root 3.x:
https://buildbot.python.org/all/#/builders/345/builds/903

0:15:05 load avg: 2.58 [137/427/1] test_signal failed (env changed) (1 min 13 sec)
Warning -- Unraisable exception
Traceback (most recent call last):
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/threading.py", line 1067, in _wait_for_tstate_lock
    elif lock.acquire(block, timeout):
OSError: Signal 10 ignored due to race condition

== Race condition 2 ==

By the way, the test also fails when I stress my laptop:

$ ./python -m test --fail-env-changed test_signal -F -j40 -m test_stress_modifying_handlers -v
== CPython 3.10.0a6+ (heads/master:a9c03d7fb7, Mar 10 2021, 12:41:26) [GCC 10.2.1 20201125 (Red Hat 10.2.1-9)]
== Linux-5.10.15-200.fc33.x86_64-x86_64-with-glibc2.32 little-endian
== cwd: /home/vstinner/python/master/build/test_python_223315æ
== CPU count: 8
== encodings: locale=UTF-8, FS=utf-8
0:00:00 load avg: 4.17 Run tests in parallel using 40 child processes
(...)
0:00:05 load avg: 7.52 [  5/1] test_signal failed
test_stress_modifying_handlers (test.test_signal.StressTest) ... FAIL

======================================================================
FAIL: test_stress_modifying_handlers (test.test_signal.StressTest)
----------------------------------------------------------------------

Traceback (most recent call last):
  File "/home/vstinner/python/master/Lib/test/test_signal.py", line 1297, in test_stress_modifying_handlers
    self.assertGreater(num_received_signals, 0)
AssertionError: 0 not greater than 0

---

Ran 1 test in 0.039s

FAILED (failures=1)
test test_signal failed
(...)

[vstinner]

The new functional test has two race conditions.

I don't think that it's a bug in Python, but more race conditions in the test.

[vstinner]

New changeset 1fa17e8 by Victor Stinner in branch 'master':
bpo-43406: Fix test_signal.test_stress_modifying_handlers() (GH-24815)
1fa17e8

[miss-islington]

New changeset ac5e23c by Miss Islington (bot) in branch '3.8':
bpo-43406: Fix test_signal.test_stress_modifying_handlers() (GH-24815)
ac5e23c

[vstinner]

New changeset 531f2eb by Miss Islington (bot) in branch '3.9':
bpo-43406: Fix test_signal.test_stress_modifying_handlers() (GH-24815) (GH-24817)
531f2eb