Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Possible race condition between signal catching and signal.signal #87572

Closed
pitrou opened this issue Mar 4, 2021 · 10 comments
Closed

Possible race condition between signal catching and signal.signal #87572

pitrou opened this issue Mar 4, 2021 · 10 comments
Labels
3.8 only security fixes 3.9 only security fixes 3.10 only security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@pitrou
Copy link
Member

pitrou commented Mar 4, 2021

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
    Copy link
    Member Author

    pitrou commented Mar 4, 2021

    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 pitrou added 3.8 only security fixes 3.9 only security fixes 3.10 only security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error labels Mar 4, 2021
    @pitrou
    Copy link
    Member Author

    pitrou commented Mar 4, 2021

    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
    Copy link
    Member Author

    pitrou commented Mar 5, 2021

    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
    Copy link
    Contributor

    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
    Copy link
    Contributor

    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

    @pitrou pitrou closed this as completed Mar 6, 2021
    @pitrou pitrou closed this as completed Mar 6, 2021
    @vstinner
    Copy link
    Member

    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 vstinner reopened this Mar 10, 2021
    @vstinner vstinner reopened this Mar 10, 2021
    @vstinner
    Copy link
    Member

    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
    Copy link
    Member

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

    @miss-islington
    Copy link
    Contributor

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

    @vstinner
    Copy link
    Member

    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

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.8 only security fixes 3.9 only security fixes 3.10 only security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants