[3.13] gh-124785: alternative interned string clear fix

Include/internal/pycore_interp.h

@@ -123,6 +123,10 @@ struct _is {
     /* Has been fully initialized via pylifecycle.c. */
     int _ready;
     int finalizing;
+    /* Set if a subinterpreter imports basic single-phase extensions and
+     * therefore needs to leak interned strings (it's too tricky to safely
+     * free them). */
+    int leak_interned_strings;
 
     uintptr_t last_restart_version;
     struct pythreads {

Misc/NEWS.d/next/Core_and_Builtins/2024-09-30-11-49-58.gh-issue-124785.sSKaJF.rst

@@ -0,0 +1,2 @@
+Use simpler fix for interned string cleanup.  This avoids the potential
+use-after-free crashes but doesn't break the trace-refs debug build.

Objects/unicodeobject.c

@@ -277,37 +277,13 @@ 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)
 {
     assert(get_interned_dict(interp) == NULL);
-    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;
-        }
+    PyObject *interned = interned = PyDict_New();
+    if (interned == NULL) {
+        return -1;
     }
     _Py_INTERP_CACHED_OBJECT(interp, interned_strings) = interned;
     return 0;
@@ -318,10 +294,7 @@ clear_interned_dict(PyInterpreterState *interp)
 {
     PyObject *interned = get_interned_dict(interp);
     if (interned != NULL) {
-        if (!has_shared_intern_dict(interp)) {
-            // only clear if the dict belongs to this interpreter
-            PyDict_Clear(interned);
-        }
+        PyDict_Clear(interned);
         Py_DECREF(interned);
         _Py_INTERP_CACHED_OBJECT(interp, interned_strings) = NULL;
     }
@@ -15323,6 +15296,42 @@ PyUnicode_InternFromString(const char *cp)
     return s;
 }
 
+void
+clear_interned_dict_legacy(PyInterpreterState *interp)
+{
+    // See GH-116510 for why this extra care is needed.  Immortal interned
+    // strings can be shared between subinterpreters in this case and we
+    // can't safely free them without a potential use-after-free crash.
+    PyObject *interned = get_interned_dict(interp);
+    Py_ssize_t pos = 0;
+    PyObject *s, *ignored_value;
+    while (PyDict_Next(interned, &pos, &s, &ignored_value)) {
+        assert(PyUnicode_IS_READY(s));
+        switch (PyUnicode_CHECK_INTERNED(s)) {
+        case SSTATE_INTERNED_IMMORTAL:
+        case SSTATE_INTERNED_IMMORTAL_STATIC:
+            // immortal strings leak since we can't safely free them
+            break;
+        case SSTATE_INTERNED_MORTAL:
+            // Restore 2 references held by the interned dict; these will
+            // be decref'd by clear_interned_dict's PyDict_Clear.
+            Py_SET_REFCNT(s, Py_REFCNT(s) + 2);
+#ifdef Py_REF_DEBUG
+            /* let's be pedantic with the ref total */
+            _Py_IncRefTotal(_PyThreadState_GET());
+            _Py_IncRefTotal(_PyThreadState_GET());
+#endif
+            break;
+        case SSTATE_NOT_INTERNED:
+            /* fall through */
+        default:
+            Py_UNREACHABLE();
+        }
+        _PyUnicode_STATE(s).interned = SSTATE_NOT_INTERNED;
+    }
+    PyDict_Clear(interned);
+    _Py_INTERP_CACHED_OBJECT(interp, interned_strings) = NULL;
+}
 
 void
 _PyUnicode_ClearInterned(PyInterpreterState *interp)
@@ -15333,10 +15342,8 @@ _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);
+    if (interp->leak_interned_strings) {
+        clear_interned_dict_legacy(interp);
         return;
     }
 
@@ -15861,10 +15868,8 @@ _PyUnicode_Fini(PyInterpreterState *interp)
 {
     struct _Py_unicode_state *state = &interp->unicode;
 
-    if (!has_shared_intern_dict(interp)) {
-        // _PyUnicode_ClearInterned() must be called before _PyUnicode_Fini()
-        assert(get_interned_dict(interp) == NULL);
-    }
+    // _PyUnicode_ClearInterned() must be called before _PyUnicode_Fini()
+    assert(get_interned_dict(interp) == NULL);
 
     _PyUnicode_FiniEncodings(&state->fs_codec);
 

Python/import.c

@@ -1897,6 +1897,11 @@ import_find_extension(PyThreadState *tstate,
         return NULL;
     }
 
+    /* Interned strings used by this module can be shared between
+     * subinterpreters, due to the PyDict_Update() call in basic single-phase
+     * module import case. */
+    tstate->interp->leak_interned_strings = 1;
+
     PyObject *mod = reload_singlephase_extension(tstate, cached, info);
     if (mod == NULL) {
         return NULL;