GeezerVison

Documentation build overview

📚 cpython-previews | 🛠️ Build #32572982 | 📁 Comparing 53fa94b against main (40dc61a)

  🔍 Preview build  

🤖 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.

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

Refleaks are worrying though.

Never mind; main currently leaks (#149179).

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

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.

This looks good to me. The only remaining issues I see are minor. GitHub is kind of broken so here are my comments:

• Some things found by Claude: https://gist.github.com/colesbury/e3ecafad30bb291d205c7be88c7fe7d4 (same as I messaged Peter in Discord)
• In make_pre_finalization_calls, the atomic compare exchange on finalization_guards should happen before HEAD_UNLOCK(interp->runtime). In other words, we should do both the checks and state transition under the same locks.
• We don't need both NO_TSTATE_SENTINEL and DETACHED_TSTATE_SENTINEL. You can just keep one or the other.

The comment about tstate->ensure.owned_guard = NULL (" This is usually done by PyThreadState_Clear()...") is confusing to me. The owned_guard = NULL in PyThreadState_Clear is both useless and not a concern of PyThreadState_Clear. These fields are managed by Ensure/Release, not PyThreadState_New/Clear/Delete and we are not clearing or touching any of the other ensure fields. I'd remove that line from PyThreadState_Clear and write the relevant part of PyThreadState_Release as:

    if (tstate->ensure.delete_on_release) {
        ++tstate->ensure.counter;
        PyThreadState_Clear(tstate);
        --tstate->ensure.counter;
    }
    else if (owned_guard != NULL) {
        tstate->ensure.owned_guard = NULL;
    }

Thanks for the reviews, both of you! Sorry again about the rushing.