Skip to content

Commit

Permalink
[3.12] pythongh-116510: Fix a crash due to shared immortal interned s…
Browse files Browse the repository at this point in the history
…trings. (pythongh-124541)

Fix a crash caused by immortal interned strings being shared between
sub-interpreters that use basic single-phase init.  In that case, the string
can be used by an interpreter that outlives the interpreter that created and
interned it.  For interpreters that share obmalloc state, also share the
interned dict with the main interpreter.
  • Loading branch information
nascheme authored Sep 27, 2024
1 parent 69fd1f2 commit 5dd07eb
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 6 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Fix a crash caused by immortal interned strings being shared between
sub-interpreters that use basic single-phase init. In that case, the string
can be used by an interpreter that outlives the interpeter that created and
interned it. For interpreters that share obmalloc state, also share the
interned dict with the main interpreter.
48 changes: 42 additions & 6 deletions Objects/unicodeobject.c
Original file line number Diff line number Diff line change
Expand Up @@ -266,6 +266,23 @@ hashtable_unicode_compare(const void *key1, const void *key2)
}
}

/* Return true if this interpreter should share the main interpreter's
intern_dict. That's important for interpreters which load basic
single-phase init extension modules (m_size == -1). There could be interned
immortal strings that are shared between interpreters, due to the
PyDict_Update(mdict, m_copy) call in import_find_extension().
It's not safe to deallocate those strings until all interpreters that
potentially use them are freed. By storing them in the main interpreter, we
ensure they get freed after all other interpreters are freed.
*/
static bool
has_shared_intern_dict(PyInterpreterState *interp)
{
PyInterpreterState *main_interp = _PyInterpreterState_Main();
return interp != main_interp && interp->feature_flags & Py_RTFLAGS_USE_MAIN_OBMALLOC;
}

static int
init_interned_dict(PyInterpreterState *interp)
{
Expand All @@ -284,9 +301,16 @@ init_interned_dict(PyInterpreterState *interp)
}
}
assert(get_interned_dict(interp) == NULL);
PyObject *interned = interned = PyDict_New();
if (interned == NULL) {
return -1;
PyObject *interned;
if (has_shared_intern_dict(interp)) {
interned = get_interned_dict(_PyInterpreterState_Main());
Py_INCREF(interned);
}
else {
interned = PyDict_New();
if (interned == NULL) {
return -1;
}
}
_Py_INTERP_CACHED_OBJECT(interp, interned_strings) = interned;
return 0;
Expand All @@ -297,7 +321,10 @@ clear_interned_dict(PyInterpreterState *interp)
{
PyObject *interned = get_interned_dict(interp);
if (interned != NULL) {
PyDict_Clear(interned);
if (!has_shared_intern_dict(interp)) {
// only clear if the dict belongs to this interpreter
PyDict_Clear(interned);
}
Py_DECREF(interned);
_Py_INTERP_CACHED_OBJECT(interp, interned_strings) = NULL;
}
Expand Down Expand Up @@ -14861,6 +14888,13 @@ _PyUnicode_ClearInterned(PyInterpreterState *interp)
}
assert(PyDict_CheckExact(interned));

if (has_shared_intern_dict(interp)) {
// the dict doesn't belong to this interpreter, skip the debug
// checks on it and just clear the pointer to it
clear_interned_dict(interp);
return;
}

/* TODO:
* Currently, the runtime is not able to guarantee that it can exit without
* allocations that carry over to a future initialization of Python within
Expand Down Expand Up @@ -15364,8 +15398,10 @@ _PyUnicode_Fini(PyInterpreterState *interp)
{
struct _Py_unicode_state *state = &interp->unicode;

// _PyUnicode_ClearInterned() must be called before _PyUnicode_Fini()
assert(get_interned_dict(interp) == NULL);
if (!has_shared_intern_dict(interp)) {
// _PyUnicode_ClearInterned() must be called before _PyUnicode_Fini()
assert(get_interned_dict(interp) == NULL);
}

_PyUnicode_FiniEncodings(&state->fs_codec);

Expand Down

0 comments on commit 5dd07eb

Please sign in to comment.