Skip to content

Commit f790e0f

Browse files
committed
gh-157230: test_importlib restore builtins
Our CI (https://github.com/facebookincubator/cinder/actions/runs/29382592631/job/87249270535) was failing on test_pickle and test_pickletool with: ``` PicklingError: Can't pickle <class 'importlib._bootstrap.BuiltinImporter'>: it's not the same object as importlib._bootstrap.BuiltinImporter ``` I believe this is happening because we run multiple test modules in the same interpreter, so the following sequence happens: * `test_importlib.util.import_importlib()` imports a source copy of `importlib` while blocking `_frozen_importlib`. During this import, `importlib._bootstrap._setup()` initializes import metadata on existing built-in modules. If `builtins.__loader__` or `builtins.__spec__` was originally absent, the source copy installs its own `BuiltinImporter`. * Although `import_fresh_module()` restores `sys.modules`, it does not restore attributes mutated on existing module objects. Consequently, `builtins.__loader__` continues to reference the temporary source `BuiltinImporter`, while `importlib._bootstrap.BuiltinImporter` resolves to the restored frozen class. * Pickle serializes classes by module and qualified name and verifies that the resolved global is the same object. The two `BuiltinImporter` class objects therefore cause the identity check to fail. To fix this we instead snapshot `__loader__` and `__spec__` before importing `importlib` and then restore them to their original values after.
1 parent 228b1bf commit f790e0f

2 files changed

Lines changed: 54 additions & 2 deletions

File tree

Lib/test/test_importlib/test_util.py

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,22 @@
3333
_interpreters = None
3434

3535

36+
class ImportImportlibTests(unittest.TestCase):
37+
38+
def test_restores_missing_import_metadata(self):
39+
# importlib._bootstrap._setup() installs the loader classes of the
40+
# source copy on any module that has no __spec__ of its own.
41+
for name in ('builtins', 'sys'):
42+
module = sys.modules[name]
43+
with self.subTest(module=name):
44+
with (support.swap_attr(module, '__spec__', None),
45+
support.swap_attr(module, '__loader__', None)):
46+
util.import_importlib('importlib')
47+
48+
self.assertIsNone(module.__spec__)
49+
self.assertIsNone(module.__loader__)
50+
51+
3652
class DecodeSourceBytesTests:
3753

3854
source = "string ='ü'"

Lib/test/test_importlib/util.py

Lines changed: 38 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -64,12 +64,48 @@ def _extension_details():
6464
_extension_details()
6565

6666

67+
_MISSING = object()
68+
69+
70+
@contextlib.contextmanager
71+
def _restore_import_metadata():
72+
"""Keep the source copy of importlib out of other modules' metadata.
73+
74+
Importing importlib with _frozen_importlib blocked runs
75+
importlib._bootstrap._setup(), which fills in __spec__ and __loader__ on
76+
every builtin and frozen module that lacks them. After the import, this
77+
context restores the original values of those attributes on all modules that
78+
were modified so those modifications don't leak into other tests.
79+
"""
80+
incomplete = {}
81+
for name, module in list(sys.modules.items()):
82+
if isinstance(module, types.ModuleType):
83+
for attr in ('__spec__', '__loader__'):
84+
value = getattr(module, attr, _MISSING)
85+
if value is None or value is _MISSING:
86+
incomplete[name, attr] = value
87+
try:
88+
yield
89+
finally:
90+
for (name, attr), value in incomplete.items():
91+
module = sys.modules.get(name)
92+
if module is not None and getattr(module, attr, _MISSING) is not value:
93+
if value is _MISSING:
94+
delattr(module, attr)
95+
else:
96+
setattr(module, attr, value)
97+
98+
6799
def import_importlib(module_name):
68100
"""Import a module from importlib both w/ and w/o _frozen_importlib."""
69101
fresh = ('importlib',) if '.' in module_name else ()
70102
frozen = import_helper.import_fresh_module(module_name)
71-
source = import_helper.import_fresh_module(module_name, fresh=fresh,
72-
blocked=('_frozen_importlib', '_frozen_importlib_external'))
103+
with _restore_import_metadata():
104+
source = import_helper.import_fresh_module(
105+
module_name,
106+
fresh=fresh,
107+
blocked=('_frozen_importlib', '_frozen_importlib_external'),
108+
)
73109
return {'Frozen': frozen, 'Source': source}
74110

75111

0 commit comments

Comments
 (0)