GeezerVison

I added DO-NOT-MERGE label for you :)

I added DO-NOT-MERGE label for you :)

Thanks!! Will ping you once all the tests are passing to move this into safe-to-merge mode 🙂

@Eclips4 tests are passing! Could you help me by adding the "skip news" label (since it's probably not required) and removing the "DO-NOT-MERGE" label?

Thanks!!

cc @nascheme to take a look when you have a chance!

It would be nice to do a better cleanup but I'm concerned about how safe this actually is. Since we don't have an accurate refcnt for these strings, we are assuming that no one is actually still using them. It seems possible that some extensions or programs embedding Python could hang on to interned strings after Py_Finalize(). That would risk introducing "use-after-free" bugs. In the worst case, that could turn into an arbitrary code execution security bug.

Maybe I'm being too paranoid? I don't know how we could actually fix this though. Keeping hold of and using memory allocated by the Python runtime after Py_Finalize() surely puts you an very shaky ground. Previously, the obmalloc state was global to the process and you could keep using memory allocated from there after finalizing. With sub-interpeters potentially having their own obmalloc state, that's likely not okay.

My gut feeling is we should proceed cautiously, even though we would like to fix the leak. Maybe we could enable this for alphas and then turn it off again for release? Maybe we need to wait until we get python -X showrefcount showing zero for all or nearly all built-in and well known extensions? Seems like a tall order though.

@nascheme I agree with what you mentioned above and I was paranoid of that scenario too, hence the disabling of the code.

Purely going off from the docs, I don't think there's anywhere where Python provides an explicit expectation of what the behavior is of memory that survives a call to Py_Finalize. Furthermore, in the C-API it is specified that Py_Finalize will: "Undo all initializations made by Py_Initialize() and subsequent use of Python/C API functions". This gives an an implicit assumption that holding onto these interned strings after Py_Finalize is an undefined behavior.

Perhaps we should be more explicit about this in the docs (maybe in the "Bugs and Caveats" section of Py_Finalize)? i.e python can't guarantee correctness for any memory allocated during a python initialization and used in any subsequent initializations. Thoughts?

Regardless, I completely agree that we should still be careful about putting this out there. Stating with alphas sounds like a good idea to get some early signal. Let me know what's the best way to only enable this only for alphas.

Maybe we need to wait until we get python -X showrefcount showing zero for all or nearly all built-in and well known extensions?

I think we are already there? Here's a quick test program that I ran:

./python -X showrefcount -c "
import sys
import importlib
for mod in sys.modules.keys():
  importlib.import_module(mod)
"
[0 refs, 0 blocks]

Maybe we need to wait until we get python -X showrefcount showing zero for all or nearly all built-in and well known extensions?

It's mostly done for one year: https://mail.python.org/archives/list/[email protected]/thread/E4C6TDNVDPDNNP73HTGHN5W42LGAE22F/ "Mostly" means: some remaining stdlib C extensions still implement their own static types which are not cleared by _PyTypes_FiniTypes(), and/or don't use the multi-phase initialization API yet.

test_embed checks that python -c pass does not leak memory at Python exit: see MiscTests.test_no_memleak().

Purely going off from the docs, I don't think there's anywhere where Python provides an explicit expectation of what the behavior is of memory that survives a call to Py_Finalize.

This change is a backward incompatible. I suggest to just document well the change in What's New in Python 3.13:

• Explain how to qualify if a crash is related to this issue,
• Document how to update affected code.

In short, a program must not keep references to any Python object between Py_Finalize() and Py_Initialize(). IMO the general rule is calling Py_Finalize() converts all Python object pointers to dangling pointers. There are some very specific exceptions which should be listed with conditions, such as "only on CPython and it's an implementation detail which can change between two Python versions".

In general, I suggest to reset all pointers to Python objects at each Py_Initialize() call. Or said differently: don't store any pointer to Python objects after Py_Finalize() :-)

Examples of exceptions:

• Immortal objects
• Static types (built-in immortal objects are marked as immortal)
• Singletons (built-in singleton objects are marked as immortal) such as: empty string, empty tuple, small integers (-5..256).

This change is a backward incompatible. I suggest to just document well the change in What's New in Python 3.13

For sure, I can update the PR with the details with what you mentioned here!

It's mostly done for one year

Given this guarantee, I think it's safe to assume that if we have any Python object leak it's a user induced leak, right? i.e. the runtime itself will not cause incorrect behavior. I will include some wording around this so that users can isolate the surface are when looking for issues (i.e look at the third-party extensions not at the runtime).

In short, a program must not keep references to any Python object between Py_Finalize() and Py_Initialize()

That's my take too - though it's not specified in the docs! Any thoughts of also updating the Py_Finalize/Py_Initialize documentation as part of this PR to include stricter language like this?

This change is a backward incompatible. I suggest to just document well the change in What's New in Python 3.13

I looked again at the code. It has a complicated history:

• In Python 3.9 and older, interned strings were never deleted. Only special build for Valgrind and Purity cleared them at exit.
• In Python 3.10 and 3.11, interned strings are always deleted at exit.
• In Python 3.12, interned strings are deleted only if Python is built in debug mode: they are not deleted in release mode.

If there are applications relying on interned strings to survive between multiple Py_Initialize()/Py_Finalize() cycles, they should already be affected by Python 3.10 and 3.11.

By the way, Python 3.11 added _PyTypes_FiniTypes() to "clear" static types in Py_Finalize().

🤖 New build scheduled with the buildbot fleet by @encukou for commit 6aee7f5 🤖

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

Thanks @eduardo-elizondo for the PR, and @encukou for merging it 🌮🎉.. I'm working now to backport this PR to: 3.12, 3.13.
🐍🍒⛏🤖

Sorry, @eduardo-elizondo and @encukou, I could not cleanly backport this to 3.13 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker 3203a7412977b8da3aba2770308136a37f48c927 3.13

Sorry, @eduardo-elizondo and @encukou, I could not cleanly backport this to 3.12 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker 3203a7412977b8da3aba2770308136a37f48c927 3.12

Thanks @encukou !!

Is there any chance this will be backported to 3.13.1?

GH-130581 is a backport of this pull request to the 3.13 branch.

GH-130582 is a backport of this pull request to the 3.12 branch.

The conflict preventing Miss Islington from backporting #113601 was the addition to Doc/whatsnew/3.14.rst.

I've made the backport PRs. Do we need any other docs in them?

Is there any chance this will be backported to 3.13.1?

Sorry, I missed the notification!

See this paragraph in the added docs:

Note that Python will do a best effort at freeing all memory allocated by the Python
interpreter. Therefore, any C-Extension should make sure to correctly clean up all
of the preveiously allocated PyObjects before using them in subsequent calls to
:c:func:Py_Initialize. Otherwise it could introduce vulnerabilities and incorrect
behavior.

In other words, it's possible that some extensions will start crashing after Py_Finalize & Py_Initialize.
It would only be extensions that used the API incorreclty, but, that's little consolation for affected users.

I don't think we should backport to bugfix branches, unfortunately.

I don't think we should backport to bugfix branches, unfortunately.

Okay, I'll close the backports.