Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gh-113190: Reenable non-debug interned string cleanup #113601

Merged
merged 19 commits into from
Aug 15, 2024
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 12 additions & 6 deletions Doc/c-api/init.rst
Original file line number Diff line number Diff line change
Expand Up @@ -394,8 +394,7 @@ Initializing and finalizing the interpreter
Undo all initializations made by :c:func:`Py_Initialize` and subsequent use of
Python/C API functions, and destroy all sub-interpreters (see
:c:func:`Py_NewInterpreter` below) that were created and not yet destroyed since
the last call to :c:func:`Py_Initialize`. Ideally, this frees all memory
allocated by the Python interpreter. This is a no-op when called for a second
the last call to :c:func:`Py_Initialize`. This is a no-op when called for a second
time (without calling :c:func:`Py_Initialize` again first).

Since this is the reverse of :c:func:`Py_Initialize`, it should be called
Expand All @@ -407,6 +406,12 @@ Initializing and finalizing the interpreter
If there were errors during finalization (flushing buffered data),
``-1`` is returned.

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.

This function is provided for a number of reasons. An embedding application
might want to restart Python without having to restart the application itself.
An application that has loaded the Python interpreter from a dynamically
Expand All @@ -421,10 +426,11 @@ Initializing and finalizing the interpreter
loaded extension modules loaded by Python are not unloaded. Small amounts of
memory allocated by the Python interpreter may not be freed (if you find a leak,
please report it). Memory tied up in circular references between objects is not
freed. Some memory allocated by extension modules may not be freed. Some
extensions may not work properly if their initialization routine is called more
than once; this can happen if an application calls :c:func:`Py_Initialize` and
:c:func:`Py_FinalizeEx` more than once.
freed. Interned strings will all be deallocated regarldess of their reference count.
Some memory allocated by extension modules may not be freed. Some extensions may not
work properly if their initialization routine is called more than once; this can
happen if an application calls :c:func:`Py_Initialize` and :c:func:`Py_FinalizeEx`
more than once.

.. audit-event:: cpython._PySys_ClearAuditHooks "" c.Py_FinalizeEx

Expand Down
11 changes: 11 additions & 0 deletions Doc/whatsnew/3.14.rst
Original file line number Diff line number Diff line change
Expand Up @@ -368,6 +368,17 @@ New Features

(Contributed by Victor Stinner in :gh:`119182`.)

* Modified the ``_PyUnicode_ClearInterned`` function to always delete all
interned strings during a call to :c:func:`Py_Finalize`. This
eduardo-elizondo marked this conversation as resolved.
Show resolved Hide resolved
is backwards incompatible to any C-Extension that holds onto an interned
string after a call to :c:func:`Py_Finalize` and is then reused after a
call to :c:func:`Py_Initialize`. Any issues arising from this behavior will
normally result in crashes during the exectuion of the subsequent call to
:c:func:`Py_Initialize` from accessing uninitialized memory. To fix, use
an address sanitizer (i.e. ASAN) to identify any use-after-free coming from
encukou marked this conversation as resolved.
Show resolved Hide resolved
an interned string and deallocate it during module shutdown.
(Contribued by Eddie Elizondo in :gh:`113601`.)

Porting to Python 3.14
----------------------

Expand Down
40 changes: 18 additions & 22 deletions Lib/test/_test_embed_structseq.py
Original file line number Diff line number Diff line change
@@ -1,31 +1,27 @@
import sys
import types
import unittest

# Note: This test file can't import `unittest` since the runtime can't
# currently guarantee that it will not leak memory. Doing so will mark
# the test as passing but with reference leaks. This can safely import
# the `unittest` library once there's a strict guarantee of no leaks
# during runtime shutdown.

# bpo-46417: Test that structseq types used by the sys module are still
# valid when Py_Finalize()/Py_Initialize() are called multiple times.
class TestStructSeq:
class TestStructSeq(unittest.TestCase):
# test PyTypeObject members
def _check_structseq(self, obj_type):
def check_structseq(self, obj_type):
# ob_refcnt
assert sys.getrefcount(obj_type) > 1
self.assertGreaterEqual(sys.getrefcount(obj_type), 1)
# tp_base
assert issubclass(obj_type, tuple)
self.assertTrue(issubclass(obj_type, tuple))
# tp_bases
assert obj_type.__bases__ == (tuple,)
self.assertEqual(obj_type.__bases__, (tuple,))
# tp_dict
assert isinstance(obj_type.__dict__, types.MappingProxyType)
self.assertIsInstance(obj_type.__dict__, types.MappingProxyType)
# tp_mro
assert obj_type.__mro__ == (obj_type, tuple, object)
self.assertEqual(obj_type.__mro__, (obj_type, tuple, object))
# tp_name
assert isinstance(type.__name__, str)
self.assertIsInstance(type.__name__, str)
# tp_subclasses
assert obj_type.__subclasses__() == []
self.assertEqual(obj_type.__subclasses__(), [])

def test_sys_attrs(self):
for attr_name in (
Expand All @@ -36,23 +32,23 @@ def test_sys_attrs(self):
'thread_info', # ThreadInfoType
'version_info', # VersionInfoType
):
attr = getattr(sys, attr_name)
self._check_structseq(type(attr))
with self.subTest(attr=attr_name):
attr = getattr(sys, attr_name)
self.check_structseq(type(attr))

def test_sys_funcs(self):
func_names = ['get_asyncgen_hooks'] # AsyncGenHooksType
if hasattr(sys, 'getwindowsversion'):
func_names.append('getwindowsversion') # WindowsVersionType
for func_name in func_names:
func = getattr(sys, func_name)
obj = func()
self._check_structseq(type(obj))
with self.subTest(func=func_name):
func = getattr(sys, func_name)
obj = func()
self.check_structseq(type(obj))


try:
tests = TestStructSeq()
tests.test_sys_attrs()
tests.test_sys_funcs()
unittest.main()
except SystemExit as exc:
if exc.args[0] != 0:
raise
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
This updates the interned string deallocation function
``_PyUnicode_ClearInterned`` to delete all interned strings during runtime
finalization when calling ``Py_Finalize``, regardless of their reference
count.

Worth noting that if an extension accidentally holds onto a interned string,
even after calling Py_Finalize, it will result in use-after-free error,
leaving the user to a potential vulnerabilities. That said, the history of
how these interned strings are handled have been changing during throughout
different versions.

For example, in Python 3.9 and older, interned strings were never deleted.
Only special builds 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. In Python 3.13, interned strings will
now be all deleted.

Given how the guarantees changed throughout different versions, it is not
expected that users actively rely on this behavior for their extensions.
eduardo-elizondo marked this conversation as resolved.
Show resolved Hide resolved
2 changes: 0 additions & 2 deletions Objects/unicodeobject.c
Original file line number Diff line number Diff line change
Expand Up @@ -15644,7 +15644,6 @@ _PyUnicode_ClearInterned(PyInterpreterState *interp)
* strict guarantee that no memory will be left after
* `Py_Finalize`.
*/
#ifdef Py_DEBUG
// Skip the Immortal Instance check and restore
// the two references (key and value) ignored
// by PyUnicode_InternInPlace().
Expand All @@ -15657,7 +15656,6 @@ _PyUnicode_ClearInterned(PyInterpreterState *interp)
#ifdef INTERNED_STATS
total_length += PyUnicode_GET_LENGTH(s);
#endif
#endif // Py_DEBUG
break;
case SSTATE_INTERNED_IMMORTAL_STATIC:
/* It is shared between interpreters, so we should unmark it
Expand Down
Loading