gh-104621: Check for Incompatible Extensions in import_find_extension() (gh-107184)

This fixes a bug where incompatible modules could still be imported if attempted multiple times.

diff --git a/Lib/test/test_capi/check_config.py b/Lib/test/test_capi/check_config.py
index aaedd82f39af50a15c277aec0fec5a220e0b799e..eb99ae16f2b69ec35c7524813cd524a0cb111977 100644 (file)
--- a/Lib/test/test_capi/check_config.py
+++ b/Lib/test/test_capi/check_config.py
@@ -12,7 +12,7 @@ def import_singlephase():
     try:
         import _testsinglephase
     except ImportError:
-        sys.modules.pop('_testsinglephase')
+        sys.modules.pop('_testsinglephase', None)
         return False
     else:
         del sys.modules['_testsinglephase']

diff --git a/Lib/test/test_import/__init__.py b/Lib/test/test_import/__init__.py
index ec8ccf0bd78d37fd1789c65e95178072f5687d8f..7a3fcc22be8d1334021c1f82c7e2deb1fde7e33b 100644 (file)
--- a/Lib/test/test_import/__init__.py
+++ b/Lib/test/test_import/__init__.py
@@ -97,7 +97,6 @@ def require_frozen(module, *, skip=True):
 def require_pure_python(module, *, skip=False):
     _require_loader(module, SourceFileLoader, skip)
 
-
 def remove_files(name):
     for f in (name + ".py",
               name + ".pyc",
@@ -147,19 +146,34 @@ def _ready_to_import(name=None, source=""):
                 del sys.modules[name]
 
 
-def requires_subinterpreters(meth):
-    """Decorator to skip a test if subinterpreters are not supported."""
-    return unittest.skipIf(_interpreters is None,
-                           'subinterpreters required')(meth)
+if _testsinglephase is not None:
+    def restore__testsinglephase(*, _orig=_testsinglephase):
+        # We started with the module imported and want to restore
+        # it to its nominal state.
+        _orig._clear_globals()
+        _testinternalcapi.clear_extension('_testsinglephase', _orig.__file__)
+        import _testsinglephase
 
 
 def requires_singlephase_init(meth):
     """Decorator to skip if single-phase init modules are not supported."""
+    if not isinstance(meth, type):
+        def meth(self, _meth=meth):
+            try:
+                return _meth(self)
+            finally:
+                restore__testsinglephase()
     meth = cpython_only(meth)
     return unittest.skipIf(_testsinglephase is None,
                            'test requires _testsinglephase module')(meth)
 
 
+def requires_subinterpreters(meth):
+    """Decorator to skip a test if subinterpreters are not supported."""
+    return unittest.skipIf(_interpreters is None,
+                           'subinterpreters required')(meth)
+
+
 class ModuleSnapshot(types.SimpleNamespace):
     """A representation of a module for testing.
 
@@ -1962,6 +1976,20 @@ class SubinterpImportTests(unittest.TestCase):
         with self.subTest(f'{module}: strict, fresh'):
             self.check_compatible_fresh(module, strict=True, isolated=True)
 
+    @requires_subinterpreters
+    @requires_singlephase_init
+    def test_disallowed_reimport(self):
+        # See https://github.com/python/cpython/issues/104621.
+        script = textwrap.dedent('''
+            import _testsinglephase
+            print(_testsinglephase)
+            ''')
+        interpid = _interpreters.create()
+        with self.assertRaises(_interpreters.RunFailedError):
+            _interpreters.run_string(interpid, script)
+        with self.assertRaises(_interpreters.RunFailedError):
+            _interpreters.run_string(interpid, script)
+
 
 class TestSinglePhaseSnapshot(ModuleSnapshot):
 
@@ -2017,6 +2045,10 @@ class SinglephaseInitTests(unittest.TestCase):
         # Start fresh.
         cls.clean_up()
 
+    @classmethod
+    def tearDownClass(cls):
+        restore__testsinglephase()
+
     def tearDown(self):
         # Clean up the module.
         self.clean_up()

diff --git a/Misc/NEWS.d/next/Core and Builtins/2023-07-24-11-11-41.gh-issue-104621.vM8Y_l.rst b/Misc/NEWS.d/next/Core and Builtins/2023-07-24-11-11-41.gh-issue-104621.vM8Y_l.rst
new file mode 100644 (file)
index 0000000..86c9762
--- /dev/null
+++ b/Misc/NEWS.d/next/Core and Builtins/2023-07-24-11-11-41.gh-issue-104621.vM8Y_l.rst
@@ -0,0 +1 @@
+Unsupported modules now always fail to be imported.

diff --git a/Python/import.c b/Python/import.c
index f8cae641a645416f7538c0664e5004fa66b5a41b..711ed5d32ca97f8f61dd3d999a2359345cbbc627 100644 (file)
--- a/Python/import.c
+++ b/Python/import.c
@@ -1215,6 +1215,15 @@ import_find_extension(PyThreadState *tstate, PyObject *name,
         return NULL;
     }
 
+    /* It may have been successfully imported previously
+       in an interpreter that allows legacy modules
+       but is not allowed in the current interpreter. */
+    const char *name_buf = PyUnicode_AsUTF8(name);
+    assert(name_buf != NULL);
+    if (_PyImport_CheckSubinterpIncompatibleExtensionAllowed(name_buf) < 0) {
+        return NULL;
+    }
+
     PyObject *mod, *mdict;
     PyObject *modules = MODULES(tstate->interp);
 
@@ -3704,16 +3713,8 @@ _imp_create_dynamic_impl(PyObject *module, PyObject *spec, PyObject *file)
 
     PyThreadState *tstate = _PyThreadState_GET();
     mod = import_find_extension(tstate, name, path);
-    if (mod != NULL) {
-        const char *name_buf = PyUnicode_AsUTF8(name);
-        assert(name_buf != NULL);
-        if (_PyImport_CheckSubinterpIncompatibleExtensionAllowed(name_buf) < 0) {
-            Py_DECREF(mod);
-            mod = NULL;
-        }
-        goto finally;
-    }
-    else if (PyErr_Occurred()) {
+    if (mod != NULL || _PyErr_Occurred(tstate)) {
+        assert(mod == NULL || !_PyErr_Occurred(tstate));
         goto finally;
     }