[3.13] gh-126986: Drop _PyInterpreterState_FailIfNotRunning() (gh-126988)

[miss-islington]

We replace it with _PyErr_SetInterpreterAlreadyRunning().
(cherry picked from commit d6b3e78)

Co-authored-by: Eric Snow [email protected]

• Issue: Crash in _PyXI_ApplyErrorCode() #126986

[zware]

Something doesn't look right here...

[zware]

Sorry for blocking the merge this way, this just seems like an enormous change that probably shouldn't be. Looking at the history of the file, though, that seems to be the rule for it. On the other hand, the fact that it's changing at all also seems suspect (though I don't pretend to actually understand all of the C API/ABI stuff :)), and l wonder if this is something that @encukou really does need to look at.

[ericsnowcurrently]

Thanks for double-checking on this. The tool (and CI check) are there to make sure we don't change the user-facing ABI in a bug fix release. Unfortunately, we never taught the tool to distinguish between user-facing ABI and internal ABI. The PyInterpreterState layout is internal ABI, and the new field in this PR caused the tool to raise a false alarm. The solution: manually update the python3.13.abi file to match the new internal ABI, which is what I've done.

[ericsnowcurrently]

Whoops, in this case we're adding a new function in the internal C-API, not adding a new PyInterpreterState field, but the point stays the same.

[zware]

If it's a manual update, can the line endings be reverted back to the style they are currently checked in with? That accounts for most of the diff in this PR. There's some other oddities with path names that are visible in git diff -b origin/3.13 output.

[encukou]

It's an autogenerated file that should only be changed with release manager approval, to a version downloaded from failing CI.

It can be diffed using abidiff:

$ git show 3.13:Doc/data/python3.13.abi > /tmp/old.abi
$ git show miss-islington/backport-d6b3e78-3.13:Doc/data/python3.13.abi > /tmp/new.abi

$ abidiff /tmp/old.abi /tmp/new.abi
Functions changes summary: 1 Removed, 0 Changed, 1 Added functions
Variables changes summary: 0 Removed, 0 Changed, 0 Added variable

1 Removed function:

  [D] 'function int _PyInterpreterState_FailIfRunningMain(PyInterpreterState*)'    {_PyInterpreterState_FailIfRunningMain}

1 Added function:

  [A] 'function void _PyErr_SetInterpreterAlreadyRunning(void)'    {_PyErr_SetInterpreterAlreadyRunning}

IOW, this matches the changes as seen in the header file.
RMs are usually fine with changing private functions when needed, but it would be more compatible to:

• keep the unused function in 3.13, and/or
• use PyErr_SetString directly in the 2 places, so _PyErr_SetInterpreterAlreadyRunning it doesn't need to be added

[bedevere-app]

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

[ericsnowcurrently]

I'm applying @encukou's suggestions to avoid affecting the ABI: gh-127112.