[Python-checkins] gh-102660: Handle m_copy Specially for the sys and builtins Modules (gh-102661)

[ericsnowcurrently]

https://github.com/python/cpython/commit/cdb21ba74d933e262bc1696b6ce78b50cb5a4443
commit: cdb21ba74d933e262bc1696b6ce78b50cb5a4443
branch: main
author: Eric Snow <ericsnowcurrently at gmail.com>
committer: ericsnowcurrently <ericsnowcurrently at gmail.com>
date: 2023-03-14T14:01:35-06:00
summary:

gh-102660: Handle m_copy Specially for the sys and builtins Modules (gh-102661)

It doesn't make sense to use multi-phase init for these modules. Using a per-interpreter "m_copy" (instead of PyModuleDef.m_base.m_copy) makes this work okay. (This came up while working on gh-101660.)

Note that we might instead end up disallowing re-load for sys/builtins since they are so special.

https://github.com/python/cpython/issues/102660

files:
M Include/internal/pycore_interp.h
M Python/bltinmodule.c
M Python/import.c
M Python/pystate.c
M Python/sysmodule.c

diff --git a/Include/internal/pycore_interp.h b/Include/internal/pycore_interp.h
index 9efed0a1cf90..84303318d218 100644
--- a/Include/internal/pycore_interp.h
+++ b/Include/internal/pycore_interp.h
@@ -111,6 +111,7 @@ struct _is {
 
     PyObject *dict;  /* Stores per-interpreter state */
 
+    PyObject *sysdict_copy;
     PyObject *builtins_copy;
     // Initialized to _PyEval_EvalFrameDefault().
     _PyFrameEvalFunction eval_frame;
diff --git a/Python/bltinmodule.c b/Python/bltinmodule.c
index 12ca0ba6c487..a8a34620b9bc 100644
--- a/Python/bltinmodule.c
+++ b/Python/bltinmodule.c
@@ -3098,6 +3098,9 @@ _PyBuiltin_Init(PyInterpreterState *interp)
     }
     Py_DECREF(debug);
 
+    /* m_copy of Py_None means it is copied some other way. */
+    builtinsmodule.m_base.m_copy = Py_NewRef(Py_None);
+
     return mod;
 #undef ADD_TO_ALL
 #undef SETBUILTIN
diff --git a/Python/import.c b/Python/import.c
index 1bf4199e125a..612fee243bd9 100644
--- a/Python/import.c
+++ b/Python/import.c
@@ -978,6 +978,16 @@ _PyImport_CheckSubinterpIncompatibleExtensionAllowed(const char *name)
     return 0;
 }
 
+static inline int
+match_mod_name(PyObject *actual, const char *expected)
+{
+    if (PyUnicode_CompareWithASCIIString(actual, expected) == 0) {
+        return 1;
+    }
+    assert(!PyErr_Occurred());
+    return 0;
+}
+
 static int
 fix_up_extension(PyObject *mod, PyObject *name, PyObject *filename)
 {
@@ -1001,7 +1011,8 @@ fix_up_extension(PyObject *mod, PyObject *name, PyObject *filename)
     // when the extension module doesn't support sub-interpreters.
     // XXX Why special-case the main interpreter?
     if (_Py_IsMainInterpreter(tstate->interp) || def->m_size == -1) {
-        if (def->m_size == -1) {
+        /* m_copy of Py_None means it is copied some other way. */
+        if (def->m_size == -1 && def->m_base.m_copy != Py_None) {
             if (def->m_base.m_copy) {
                 /* Somebody already imported the module,
                    likely under a different name.
@@ -1055,18 +1066,34 @@ import_find_extension(PyThreadState *tstate, PyObject *name,
     PyObject *modules = MODULES(tstate->interp);
 
     if (def->m_size == -1) {
+        PyObject *m_copy = def->m_base.m_copy;
         /* Module does not support repeated initialization */
-        if (def->m_base.m_copy == NULL)
+        if (m_copy == NULL) {
             return NULL;
+        }
+        else if (m_copy == Py_None) {
+            if (match_mod_name(name, "sys")) {
+                m_copy = tstate->interp->sysdict_copy;
+            }
+            else if (match_mod_name(name, "builtins")) {
+                m_copy = tstate->interp->builtins_copy;
+            }
+            else {
+                _PyErr_SetString(tstate, PyExc_ImportError, "missing m_copy");
+                return NULL;
+            }
+        }
+        /* m_copy of Py_None means it is copied some other way. */
         mod = import_add_module(tstate, name);
-        if (mod == NULL)
+        if (mod == NULL) {
             return NULL;
+        }
         mdict = PyModule_GetDict(mod);
         if (mdict == NULL) {
             Py_DECREF(mod);
             return NULL;
         }
-        if (PyDict_Update(mdict, def->m_base.m_copy)) {
+        if (PyDict_Update(mdict, m_copy)) {
             Py_DECREF(mod);
             return NULL;
         }
diff --git a/Python/pystate.c b/Python/pystate.c
index 28606e4f32f7..3a2966c54a4c 100644
--- a/Python/pystate.c
+++ b/Python/pystate.c
@@ -805,6 +805,7 @@ interpreter_clear(PyInterpreterState *interp, PyThreadState *tstate)
     assert(interp->imports.importlib == NULL);
     assert(interp->imports.import_func == NULL);
 
+    Py_CLEAR(interp->sysdict_copy);
     Py_CLEAR(interp->builtins_copy);
     Py_CLEAR(interp->dict);
 #ifdef HAVE_FORK
diff --git a/Python/sysmodule.c b/Python/sysmodule.c
index 764fb70bae6c..fc0550266bf1 100644
--- a/Python/sysmodule.c
+++ b/Python/sysmodule.c
@@ -3425,12 +3425,20 @@ _PySys_Create(PyThreadState *tstate, PyObject **sysmod_p)
         return _PyStatus_ERR("failed to create a module object");
     }
 
+    /* m_copy of Py_None means it is copied some other way. */
+    sysmodule.m_base.m_copy = Py_NewRef(Py_None);
+
     PyObject *sysdict = PyModule_GetDict(sysmod);
     if (sysdict == NULL) {
         goto error;
     }
     interp->sysdict = Py_NewRef(sysdict);
 
+    interp->sysdict_copy = PyDict_Copy(sysdict);
+    if (interp->sysdict_copy == NULL) {
+        goto error;
+    }
+
     if (PyDict_SetItemString(sysdict, "modules", modules) < 0) {
         goto error;
     }