GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-116738: Add locks for FT build in signalmodule by johng · Pull Request #161221 · python/cpython

Conversation

@johng

@johng johng commented Nov 2, 2026 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

This addresses one of the found race conditions visible with TSAN and pydebug for #116738 in Modules/signalmodule.c.

def main():
    signum = signal.SIGUSR1
    done = threading.Event()

    def reader():
        while not done.is_set():
            # Load the current handler and touch the returned object.
            callable(_signal.getsignal(signum))

    threads = [threading.Thread(target=reader) for _ in range(4)]
    for t in threads:
        t.start()

    # Main thread: replace the handler over and over.  signal() returns the
    # old handler, which is dropped immediately and freed.
    for _ in range(300_000):
        _signal.signal(signum, lambda signum, frame: None)

    done.set()
    for t in threads:
        t.join()
    print("done, no crash")

Was not able to see this crashing on regular build, unsure what the precedence here is in terms off addressing this likely low issue race for FT but it came up as a potential gap in the linked issue in this module.

  • Issue: Audit all built-in modules for thread safety #116738

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@johng
Add locks for FT build in signalmodule
bb67636
@bedevere-app bedevere-app Bot added awaiting review type-feature A feature request or enhancement labels Nov 2, 2026
@bedevere-app bedevere-app Bot mentioned this pull request Nov 2, 2026
Audit all built-in modules for thread safety #116738
Open
@johng

johng commented Nov 2, 2026

Copy link
Copy Markdown
Contributor Author

CC @ngoldbaum if you have bandwdith, thanks!

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@johng
pythongh-116738: Add NEWS entry for the signal.getsignal() fix
3d66eaa
@ngoldbaum ngoldbaum added the DO-NOT-MERGE label Nov 3, 2026
@ngoldbaum

ngoldbaum commented Nov 3, 2026 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

I think this needs a different approach.

First, this doesn’t protect all accesses to the handler pointers, so it’s incomplete. It also doesn’t protect all the state the GIL previously serialized access to, so it’s incomplete from that perspective. Finally, there are no tests and fixes really should come with tests that run quickly and are deterministically broken before the fixes are applied.

Second, a mutex cannot protect every read of signal state. PyErr_SetInterruptEx() may run inside a C signal handler, where acquiring a mutex is unsafe. Correctly implementing signal handlers and handling signal safety is incredibly difficult, see https://iafisher.com/2026/08/sigprint and other posts in that series for examples.

That means we’d need to come up with some other signal-safe approach anyway.

A lock-free design could cover the path that must be signal-safe as well as reads that need the handler object. Here is an outline of a design:

signal.signal() could update an atomic flag indicating whether a Python handler is installed. PyErr_SetInterruptEx() could read that flag without touching a Python object. For signal.getsignal() and Python signal dispatch, each reader could mark a read in progress before loading an atomic handler pointer and taking a strong reference. After replacing the pointer, signal.signal() could keep the old handler alive until earlier reads finish. You’d need to add bookkeeping to track live reads and add or expand tests for the interactions between threads and signals to stress-test the bookkeeping.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@johng johng closed this Nov 3, 2026
@johng johng mentioned this pull request Nov 3, 2026
signalmodule.c in free-threaded Python #161235
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

awaiting review DO-NOT-MERGE type-feature A feature request or enhancement

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

2 participants

@johng @ngoldbaum
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.