classification
Title: Possible race condition between signal catching and signal.signal
Type: behavior Stage: resolved
Components: Interpreter Core, Library (Lib) Versions: Python 3.10, Python 3.9, Python 3.8
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: miss-islington, neologix, pitrou, vstinner
Priority: normal Keywords: patch

Created on 2021-03-04 21:31 by pitrou, last changed 2021-03-10 15:11 by vstinner. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 24756 merged pitrou, 2021-03-04 21:57
PR 24761 merged miss-islington, 2021-03-05 09:33
PR 24762 merged pitrou, 2021-03-05 09:45
PR 24755 pitrou, 2021-03-07 22:41
PR 24815 merged vstinner, 2021-03-10 12:25
PR 24816 merged miss-islington, 2021-03-10 14:28
PR 24817 merged miss-islington, 2021-03-10 14:28
Messages (10)
msg388131 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2021-03-04 21:31
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.
msg388132 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2021-03-04 21:32
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
msg388150 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2021-03-05 09:33
New changeset 68245b7a1030287294c65c298975ab9026543fd2 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)
https://github.com/python/cpython/commit/68245b7a1030287294c65c298975ab9026543fd2
msg388202 - (view) Author: miss-islington (miss-islington) Date: 2021-03-06 15:07
New changeset 1385f8355a036fd65aaf9c7e7ab48467ca922bcf 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)
https://github.com/python/cpython/commit/1385f8355a036fd65aaf9c7e7ab48467ca922bcf
msg388203 - (view) Author: miss-islington (miss-islington) Date: 2021-03-06 15:08
New changeset 4715be8a4384159e47fb09e5f3bd077734842655 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)
https://github.com/python/cpython/commit/4715be8a4384159e47fb09e5f3bd077734842655
msg388429 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2021-03-10 12:01
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
(...)
msg388431 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2021-03-10 12:28
> 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.
msg388435 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2021-03-10 14:27
New changeset 1fa17e8cc62775a2e34b158135ce8589f9394f03 by Victor Stinner in branch 'master':
bpo-43406: Fix test_signal.test_stress_modifying_handlers() (GH-24815)
https://github.com/python/cpython/commit/1fa17e8cc62775a2e34b158135ce8589f9394f03
msg388437 - (view) Author: miss-islington (miss-islington) Date: 2021-03-10 14:49
New changeset ac5e23c540f5ec20fc390c72e097e0fdaf8b7ac9 by Miss Islington (bot) in branch '3.8':
bpo-43406: Fix test_signal.test_stress_modifying_handlers() (GH-24815)
https://github.com/python/cpython/commit/ac5e23c540f5ec20fc390c72e097e0fdaf8b7ac9
msg388438 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2021-03-10 15:11
New changeset 531f2ebd60a662111c78107935d249d3d52f9a4f by Miss Islington (bot) in branch '3.9':
bpo-43406: Fix test_signal.test_stress_modifying_handlers() (GH-24815) (GH-24817)
https://github.com/python/cpython/commit/531f2ebd60a662111c78107935d249d3d52f9a4f
History
Date User Action Args
2021-03-10 15:11:06vstinnersetmessages: + msg388438
2021-03-10 14:49:35miss-islingtonsetmessages: + msg388437
2021-03-10 14:30:05vstinnersetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2021-03-10 14:28:09miss-islingtonsetpull_requests: + pull_request23583
2021-03-10 14:28:03miss-islingtonsetpull_requests: + pull_request23582
2021-03-10 14:27:02vstinnersetmessages: + msg388435
2021-03-10 12:28:28vstinnersetmessages: + msg388431
2021-03-10 12:25:03vstinnersetstage: resolved -> patch review
pull_requests: + pull_request23581
2021-03-10 12:01:35vstinnersetstatus: closed -> open

nosy: + vstinner
messages: + msg388429

resolution: fixed -> (no value)
2021-03-07 22:41:26pitrousetpull_requests: + pull_request23546
2021-03-06 15:09:04pitrousetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2021-03-06 15:08:04miss-islingtonsetmessages: + msg388203
2021-03-06 15:07:53miss-islingtonsetmessages: + msg388202
2021-03-05 09:45:10pitrousetpull_requests: + pull_request23534
2021-03-05 09:33:27miss-islingtonsetnosy: + miss-islington
pull_requests: + pull_request23533
2021-03-05 09:33:19pitrousetmessages: + msg388150
2021-03-04 23:20:50pitroulinkissue39169 superseder
2021-03-04 22:00:42pitrousetnosy: + neologix
2021-03-04 21:57:31pitrousetkeywords: + patch
stage: needs patch -> patch review
pull_requests: + pull_request23527
2021-03-04 21:32:14pitrousetmessages: + msg388132
2021-03-04 21:31:08pitroucreate