Close #15386: There was a loophole that meant importlib.machinery and imp would somet...

diff --git a/Lib/importlib/__init__.py b/Lib/importlib/__init__.py
index 0935ada28836280eadb5ab4df845e926a8e9f602..d30691a165373437bb8a8c73f6206f2b68f2529d 100644 (file)
--- a/Lib/importlib/__init__.py
+++ b/Lib/importlib/__init__.py
@@ -2,14 +2,21 @@
 __all__ = ['__import__', 'import_module', 'invalidate_caches']
 
 # Bootstrap help #####################################################
-import imp
+
+# Until bootstrapping is complete, DO NOT import any modules that attempt
+# to import importlib._bootstrap (directly or indirectly). Since this
+# partially initialised package would be present in sys.modules, those
+# modules would get an uninitialised copy of the source version, instead
+# of a fully initialised version (either the frozen one or the one
+# initialised below if the frozen one is not available).
+import _imp  # Just the builtin component, NOT the full Python module
 import sys
 
 try:
     import _frozen_importlib as _bootstrap
 except ImportError:
     from . import _bootstrap
-    _bootstrap._setup(sys, imp)
+    _bootstrap._setup(sys, _imp)
 else:
     # importlib._bootstrap is the built-in import, ensure we don't create
     # a second copy of the module.
@@ -22,6 +29,8 @@ else:
 _w_long = _bootstrap._w_long
 _r_long = _bootstrap._r_long
 
+# Fully bootstrapped at this point, import whatever you like, circular
+# dependencies and startup overhead minimisation permitting :)
 
 # Public API #########################################################
 

diff --git a/Lib/pkgutil.py b/Lib/pkgutil.py
index 8407b6d0b3484faffd62d14baca07dc8f2768d62..8bdeb32e8188e64b32a0d663bf66a93b11d6c98b 100644 (file)
--- a/Lib/pkgutil.py
+++ b/Lib/pkgutil.py
@@ -2,8 +2,8 @@
 
 import os
 import sys
-import imp
 import importlib
+import imp
 import os.path
 from warnings import warn
 from types import ModuleType

diff --git a/Lib/runpy.py b/Lib/runpy.py
index d612727c6f1ca15101e62d6f7ee4531e6b28f642..39c0e9f7dd31ad8eba08d4c5f979d9c9eeb42925 100644 (file)
--- a/Lib/runpy.py
+++ b/Lib/runpy.py
@@ -12,8 +12,8 @@ importers when locating support scripts as well as when importing modules.
 
 import os
 import sys
+import importlib.machinery # importlib first so we can test #15386 via -m
 import imp
-import importlib.machinery
 from pkgutil import read_code, get_loader, get_importer
 
 __all__ = [

diff --git a/Lib/test/regrtest.py b/Lib/test/regrtest.py
index 28655f0e654f475019b7a038905226244026296c..3c8359aca1cd1efef8cf890591c62b90ed4ed1ba 100755 (executable)
--- a/Lib/test/regrtest.py
+++ b/Lib/test/regrtest.py
@@ -165,6 +165,9 @@ example, to run all the tests except for the gui tests, give the
 option '-uall,-gui'.
 """
 
+# We import importlib *ASAP* in order to test #15386
+import importlib
+
 import builtins
 import faulthandler
 import getopt

diff --git a/Lib/test/test_import.py b/Lib/test/test_import.py
index 89ec8dcedc21d3cc4c7d9ca9ac1530fa6a8ebf95..51b52c7e0b605199c28f5dd737a284e2a24ce497 100644 (file)
--- a/Lib/test/test_import.py
+++ b/Lib/test/test_import.py
@@ -1,8 +1,9 @@
+# We import importlib *ASAP* in order to test #15386
+import importlib
 import builtins
 import imp
 from importlib.test.import_ import test_suite as importlib_import_test_suite
 from importlib.test.import_ import util as importlib_util
-import importlib
 import marshal
 import os
 import platform
@@ -777,6 +778,15 @@ class ImportlibBootstrapTests(unittest.TestCase):
         self.assertEqual(mod.__package__, 'importlib')
         self.assertTrue(mod.__file__.endswith('_bootstrap.py'), mod.__file__)
 
+    def test_there_can_be_only_one(self):
+        # Issue #15386 revealed a tricky loophole in the bootstrapping
+        # This test is technically redundant, since the bug caused importing
+        # this test module to crash completely, but it helps prove the point
+        from importlib import machinery
+        mod = sys.modules['_frozen_importlib']
+        self.assertIs(machinery.FileFinder, mod.FileFinder)
+        self.assertIs(imp.new_module, mod.new_module)
+
 
 class ImportTracebackTests(unittest.TestCase):