gh-149101: Implement PEP 788 by ZeroIntensity · Pull Request #149116 · python/cpython

ZeroIntensity · 2026-04-28T21:41:53Z

Hugo has graciously given me permission to backport this if we don't make the May 5th deadline, but let's try to get this done in time!

I will write a full tutorial and migration guide once this is merged; I want to first make sure that this lands before the beta freeze.

Issue: Implement PEP 788 -- Protecting the C API from Interpreter Finalization #149101

read-the-docs-community · 2026-04-28T21:49:25Z

Documentation build overview

📚 cpython-previews | 🛠️ Build #32550521 | 📁 Comparing 17b0e2a against main (40dc61a)

🔍 Preview build

70 files changed · + 1 added · ± 69 modified

+ Added

c-api/slots.html

± Modified

encukou

Thanks for adding these!

I'll send notes for Doc/ now; code review coming up.

Co-authored-by: Petr Viktorin <encukou@gmail.com>

bedevere-bot · 2026-04-29T16:58:01Z

🤖 New build scheduled with the buildbot fleet by @ZeroIntensity for commit bc78c10 🤖

Results will be shown at:

https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F149116%2Fmerge

If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again.

encukou · 2026-04-30T07:31:27Z

for buildbots: The RHEL8 failures aren't relevant. Refleaks are worrying though.

encukou · 2026-04-30T09:30:04Z

Refleaks are worrying though.

Never mind; main currently leaks (#149179).

colesbury · 2026-05-04T22:58:20Z

+try_acquire_interp_guard(PyInterpreterState *interp, PyInterpreterGuard *guard)
+{
+    assert(interp != NULL);
+    _PyRWMutex_RLock(&interp->finalization_guards.lock);


I know I've made a bunch of other comments about this lock, but also try_acquire_interp_guard is called within HEAD_LOCK() and HEAD_LOCK should always be the inner-most lock. This risks lock ordering deadlocks.

Moot now that the lock is gone

colesbury · 2026-05-04T23:01:03Z

+
+// CPython detail: this struct is not defined anywhere; tokens are special
+// sentinels or PyThreadState's.
+typedef struct PyThreadStateToken PyThreadStateToken;


I thought we talked about this being typdef void PyThreadStateToken. Casting PyThreadState* to a distinct struct PyThreadStateToken* looks like UB to me.

Petr wrote this part and I just didn't notice. No idea if it's UB, but I'll change it.

I'm pretty sure it's not UB, but, void works too.

I stuck the pattern in ChatGPT and it agreed with @encukou that it's not UB, at least if struct PyThreadStateToken is kept opaque.

The typedef struct PyThreadStateToken PyThreadStateToken definition is fine with me, and will probably provide stricter type checking.

This will probably reveal all sorts of fun bugs.

I'm seeing some odd failures in CI. I doubt this is the cause, but this looks like it could be its own bug.

Claude pointed this out as an issue. I think it makes some sense.

colesbury · 2026-05-05T15:44:30Z

@@ -2330,17 +2370,28 @@ make_pre_finalization_calls(PyThreadState *tstate, int subinterpreters)
        int has_subinterpreters = subinterpreters


I think all these checks here should be done atomically without acquiring/releasing locks in between. So basically:

PyMutex_Lock(&interp->ceval.pending.mutex); _PyEval_StopTheWorldAll(interp->runtime); HEAD_LOCK(&_PyRuntime); // ... do the checks and compare-exchange on finalization_guards HEAD_UNLOCK(_PyRuntime); _PyEval_StartTheWorldAll(interp->runtime); PyMutex_Unlock(&interp->ceval.pending.mutex);

In other words, lift the HEAD_LOCK() out from runtime_has_subinterpreters and interp_has_threads

colesbury · 2026-05-05T15:53:46Z

+
+// CPython detail: this struct is not defined anywhere; tokens are special
+// sentinels or PyThreadState's.
+typedef struct PyThreadStateToken PyThreadStateToken;


I stuck the pattern in ChatGPT and it agreed with @encukou that it's not UB, at least if struct PyThreadStateToken is kept opaque.

The typedef struct PyThreadStateToken PyThreadStateToken definition is fine with me, and will probably provide stricter type checking.

colesbury · 2026-05-05T15:56:21Z

+        _PyThreadState_Detach(tstate);
        _PyEval_StartTheWorldAll(interp->runtime);
        PyMutex_Unlock(&interp->ceval.pending.mutex);
+        _PyThreadState_Attach(tstate);


Is the current ordering here deliberate? I'm not sure if there's any issues, but the split of detach/attach across _PyEval_StartTheWorldAll and PyMutex_Unlock confuses me.

I would write it as:

_PyEval_StartTheWorldAll(interp->runtime); PyMutex_Unlock(&interp->ceval.pending.mutex); // Temporarily let other threads execute _PyThreadState_Detach(tstate); _PyThreadState_Attach(tstate);

But it's also not clear to me whey we need _PyThreadState_Detach/Attach here. If other threads are running, won't we wait on them in either wait_for_thread_shutdown or when parking on the finalization_guards?

Yeah, I think that's an artifact of an older variation.

colesbury · 2026-05-05T19:48:38Z

+        ++attached_tstate->ensure.counter;
+        return NO_TSTATE_SENTINEL;


There is a bug here (found by Claude) where PyThreadState_Ensure/PyThreadState_Release with an already attached detaches on release when it should keep the active thread state.

In other words, we aren't properly distinguishing the two cases "already has a valid active thread state" and "doesn't have any attached thread state".

https://gist.github.com/colesbury/b6538312a898dd97fdd770b22fb3c338

colesbury · 2026-05-05T19:55:52Z

+   Failure indicates that the process is out of memory or that the main
+   interpreter has finalized (or never existed).
+
+   Using this function in extension libraries is strongly discouraged, because


We should use an affirmative tone. For example:

Use this function when an interpreter pointer or view cannot be supplied by the caller, for example, when a native threading library does not provide a void *arg parameter that could carry a :c:type:PyInterpreterGuard or :c:type:PyInterpreterView. In code that supports subinterpreters, prefer :c:func:PyInterpreterView_FromCurrent so the guard tracks the calling interpreter rather than the main one.

colesbury · 2026-05-05T20:01:17Z

 // Yield the processor to other threads (e.g., sched_yield).
-extern void _Py_yield(void);
+// Exported for _testembed.
+PyAPI_FUNC(void) _Py_yield(void);


I'm not sure I understand the purpose of this in the tests. If the intent is to try to capture timing bugs, use something like pysleep. You can copy-paste those few lines of code if necessary.

I'd prefer we don't export this symbol since that complicates potential future bug fixes if we need to change or remove it.

I'll just remove it. My hope was that this would make some of the apparent thread safety issues reproduce more reliably, but it doesn't seem that it did.

colesbury · 2026-05-05T20:22:46Z

+    if (detached_gilstate != NULL && detached_gilstate->interp == interp) {
+        /* There's a detached thread state that works. */
+        assert(attached_tstate == NULL);
+        ++detached_gilstate->ensure.counter;


I think this also has a bug. Let's say you have:

t1 = PyThreadState_Ensure(main_interp_guard); // counter 0 -> 1 Py_BEGIN_ALLOW_THREADS t2 = PyThreadState_Ensure(main_interp_guard); // counter 1 -> 2 PyThreadState_Release(t2); // BUG! 2->1 Py_END_ALLOW_THREADS PyThreadState_Release(t1);

The inner PyThreadState_Release should restore the thread state to "not attached", but it leaves it as attached due to the counter.

Co-authored-by: Sam Gross <colesbury@gmail.com>

ZeroIntensity added 6 commits April 28, 2026 15:49

Copy the reference implementation.

43b0798

Merge branch 'main' of https://github.com/python/cpython into pep-788

722dbdf

Document the new APIs.

73cccbb

Add a whatsnew entry.

25e1cf0

Fix stable ABI things.

af1022a

Fix test_embed.

2fb8419

ZeroIntensity requested review from a team, AA-Turner, FFY00, encukou and ericsnowcurrently as code owners April 28, 2026 21:41

bedevere-app Bot added the awaiting core review label Apr 28, 2026

bedevere-app Bot mentioned this pull request Apr 28, 2026

Implement PEP 788 -- Protecting the C API from Interpreter Finalization #149101

Open

ZeroIntensity added 2 commits April 28, 2026 17:43

SIlly news entry.

1096a32

Documentation fixes.

407062f

ZeroIntensity and others added 3 commits April 28, 2026 18:09

Make the sentinel const instead of changing the C analyzer.

a33bdfc

Fix the html IDs job.

c504a9f

Merge branch 'main' into pep-788

f95bfc7

encukou reviewed Apr 29, 2026

View reviewed changes

ZeroIntensity and others added 5 commits April 29, 2026 08:24

Apply suggestions from code review

a00bfbb

Co-authored-by: Petr Viktorin <encukou@gmail.com>

Update Doc/c-api/threads.rst

6ca2499

Co-authored-by: Petr Viktorin <encukou@gmail.com>

Fix lint and remove dead comment.

74259f8

Improve new PyThreadState API docs.

eff9b40

Co-authored-by: Petr Viktorin <encukou@gmail.com>

Fix lint.

bc78c10

ZeroIntensity added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Apr 29, 2026

bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Apr 29, 2026

Fix leak in get_main_interp_guard.

df720a5

colesbury reviewed May 4, 2026

View reviewed changes

Comment thread Programs/_testembed.c

ZeroIntensity added 17 commits May 4, 2026 19:39

Improve readability and fix warning.

767c894

Use void instead of a struct for PyThreadStateToken.

38a2153

Run test_embed with TSan.

321c783

This will probably reveal all sorts of fun bugs.

Switch to a single atomic field instead of using locks and events.

811ca19

Fix existing thread-safety issue with interp_has_threads().

ac58a16

Use the parking lot to avoid eating CPU cycles.

4ced3a1

Stupid merge conflicts.

9345afd

Fix C analyzer.

11cbe5c

Stupid warnings.

99098d7

Fix thread leaks in test_embed.

1c184f1

Fix deadlock in test_atexit.

af57d9d

Fix missing newline.

7289037

Remove stray newline change from pycore_lock.h

a538acc

Use sequential memory ordering in _testembed.

4bec1e7

I'm seeing some odd failures in CI. I doubt this is the cause, but this looks like it could be its own bug.

Potentially fix some things?

5d28e1a

Add a test stressing a lot of threads against finalization.

62b137e

Export _Py_yield.

2378c88

colesbury reviewed May 5, 2026

View reviewed changes

Comment thread Python/pylifecycle.c Outdated

ZeroIntensity added 4 commits May 5, 2026 10:21

Fix race in interp_has_threads.

e7466f1

Fix formatting.

5d797cf

Merge branch 'main' of https://github.com/python/cpython into pep-788

5f1d2e0

Close owned guards before deleting them, not after.

4b446e7

Claude pointed this out as an issue. I think it makes some sense.

colesbury reviewed May 5, 2026

View reviewed changes

ZeroIntensity and others added 3 commits May 5, 2026 16:40

Update Python/pystate.c

5e25ba9

Co-authored-by: Sam Gross <colesbury@gmail.com>

Update Python/pylifecycle.c

08e845c

Co-authored-by: Sam Gross <colesbury@gmail.com>

Update Doc/c-api/interp-lifecycle.rst

17b0e2a

Co-authored-by: Sam Gross <colesbury@gmail.com>

		@@ -2330,17 +2370,28 @@ make_pre_finalization_calls(PyThreadState *tstate, int subinterpreters)
		int has_subinterpreters = subinterpreters

		++attached_tstate->ensure.counter;
		return NO_TSTATE_SENTINEL;

Uh oh!

Conversation

ZeroIntensity commented Apr 28, 2026 • edited by bedevere-app Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

read-the-docs-community Bot commented Apr 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Documentation build overview

Uh oh!

encukou left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bedevere-bot commented Apr 29, 2026

Uh oh!

encukou commented Apr 30, 2026

Uh oh!

encukou commented Apr 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ZeroIntensity commented Apr 28, 2026 •

edited by bedevere-app Bot

Loading

read-the-docs-community Bot commented Apr 28, 2026 •

edited

Loading

encukou commented Apr 30, 2026 •

edited

Loading