Repository navigation
test_pickle/test_pickletools fail when running tests sequentially. #103247
Description
When running the test suite sequentially (as is done as part of the release process), an interaction between test_imp, test_importlib and subinterpreters is causing test_pickle and test_pickletools to fail. The easiest reproducer::
% ./python -m test test_imp test_importlib test_pickletools
0:00:00 load avg: 0.22 Run tests sequentially
0:00:00 load avg: 0.22 [1/3] test_imp
0:00:00 load avg: 0.22 [2/3] test_importlib
0:00:03 load avg: 0.22 [3/3] test_pickletools
test test_pickletools failed -- Traceback (most recent call last):
File "Lib/test/pickletester.py", line 1989, in test_builtin_types
s = self.dumps(t, proto)
^^^^^^^^^^^^^^^^^^^^
File "Lib/test/test_pickletools.py", line 11, in dumps
return pickletools.optimize(pickle.dumps(arg, proto, **kwargs))
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
_pickle.PicklingError: Can't pickle <class 'importlib._bootstrap.BuiltinImporter'>: it's not the same object as importlib._bootstrap.BuiltinImporter
test_pickletools failed (1 error)
== Tests result: FAILURE ==
2 tests OK.
1 test failed:
test_pickletools
Total duration: 4.2 sec
Tests result: FAILURE
The failure happens in both test_pickle and test_pickletools (test_pickletools's traceback is more obvious), after in the same process running both test_imp and either test_import or test_importlib. Disabling test_imp.ImportTests.test_create_builtin_subinterp() makes the tests pass. (This was happening before #102982 as well as after it, so I don't think it is involved here.)
I expect this is an unintentional side-effect of the test, but this was not happening in the 3.12.0a6 release, and as far as I can tell the relevant tests haven't changed since before a6. I'm delaying the 3.12.0a7 release until it's clear this isn't a fundamental problem with subinterpreters.
Activity
Yhg1s commented on Apr 4, 2023
The problem is actually reproducible all the way back to 3.12.0a4, even though it was never caught in the release testing (I don't know why, yet). The problem was exposed by #100142, but that merely adds testing of picklability of builtins, which all things considered is not an unreasonable test.
Yhg1s commented on Apr 4, 2023
As it appears this is merely uncovering a long standing issue with a hole in subinterpreter isolation and importlib (the same test combination fails in 3.10 and 3.11), I'm unblocking a7 for now, but leaving this as a release blocker for beta 1.
erlend-aasland commented on Apr 5, 2023
I did some bisecting in 3.10 and 3.11, and it landed like this:
Strangely, I do not get the same result in main; the "parent" commit (cb2ef8b, gh-99642) that those backports were created from, reproduces the failure, as expected, but cb2ef8b2acbb231c207207d3375b2f8b0077a6ee~ (1cae31d) is not a "good" commit; the failure also occurs there.
The first bad commit I get in main is:
The linked issues for those PRs contain some interesting history and debugging:
(It would be nice if someone else could bisect on their machine and verify my findings.)
sunmy2019 commented on Apr 5, 2023
Is it because the release testing in running in 4 processes?
Yhg1s commented on Apr 5, 2023
No, the release testing is done sequentially, and that hasn't changed in 2 years: https://github.com/python/release-tools/blame/master/run_release.py#L486
4 remaining items
JelleZijlstra commented on May 4, 2023
test_imp is no more, so the original reproduction steps no longer work. However, I see this failure on current main (no pickle tests necessary):
% ./python.exe -m test test_importlib test_import Raised RLIMIT_NOFILE: 256 -> 1024 0:00:00 load avg: 1.91 Run tests sequentially 0:00:00 load avg: 1.91 [1/2] test_importlib 0:00:10 load avg: 2.29 [2/2] test_import test test_import failed -- Traceback (most recent call last): File "/Users/jelle/py/cpython/Lib/test/test_import/__init__.py", line 1815, in test_multi_init_extension_compat require_extension(module) File "/Users/jelle/py/cpython/Lib/test/test_import/__init__.py", line 90, in require_extension _require_loader(module, ExtensionFileLoader, skip) File "/Users/jelle/py/cpython/Lib/test/test_import/__init__.py", line 76, in _require_loader actual = MODULE_KINDS[actual] ~~~~~~~~~~~~^^^^^^^^ KeyError: <class 'importlib._bootstrap_external.ExtensionFileLoader'> test_import failed (1 error) == Tests result: FAILURE == 1 test OK. 1 test failed: test_import Total duration: 13.5 sec Tests result: FAILUREpythonGH-103247: bypass the import cache on the _require_loader helper
sunmy2019 commented on May 5, 2023
I looked into it. The failure (changing the env) comes from
cpython/Lib/test/test_importlib/extension/test_loader.py
Lines 263 to 273 in e5b8b19
Commenting on it solves this issue.
It seems not to follow the with util.uncache(self.name) idiom in adjacent tests. (I did not check the code logic, just did A/B test)
FFY00 commented on May 6, 2023
Good catch! Do you want to open a PR with the fix?
I think it'd make sense to add a teardown method which unloads self.name instead of relying on util.uncache to be used in all tests.
sunmy2019 commented on May 6, 2023
#104226
Merge branch 'main' into pythongh-103247
gh-103247: clear the module cache in a test in test_importlib/extensi…
pythongh-103247: clear the module cache in a test in test_importlib/e…
[3.11] gh-103247: clear the module cache in a test in test_importlib/…
Merge branch 'main' into fixloadsuperspec
Merge branch 'main' into pythongh-103247-_require_loader
GH-103247: bypass the import cache on the _require_loader helper