Repository navigation
GH-114781 potentially breaks gevent: threading becomes pre-imported at startup #117983
Description
Bug report
Bug description:
Hello up there.
I've discovered that starting from CPython >= 3.11.10 gevent-based applications
become potentially broken because, in a venv with gevent installed, threading
module becomes pre-imported by python stdlib itself. As gevent injects its own
implementation of threading primitives, it helps to know that there are no
instances of C-implemented locks at the time of the monkey-patching: if there
are no such instances(*) the program is known to have only instances of
gevent-provided locks and it will not run into deadlock scenarios
described at e.g. gevent/gevent#1865 where the code
tries to take a lock, that lock turns to be standard thread lock, but there are
only greenlets running and there is no other real thread to release that lock.
So not having threading pre-imported at startup is very desirable property in
the context of gevent. For this reason gpython from pygolang actually verifies
that invariant and it is through which I've discovered the issue when testing
pygolang on py3.11.10:
(py311-venv) kirr@deca:~/src/tools/go/pygolang-master$ gpython
Traceback (most recent call last):
File "/home/kirr/src/tools/go/py311-venv/bin/gpython", line 8, in <module>
sys.exit(main())
^^^^^^
File "/home/kirr/src/tools/go/pygolang-master/gpython/__init__.py", line 368, in main
raise RuntimeError('gpython: internal error: the following modules are pre-imported, but must be not:'
RuntimeError: gpython: internal error: the following modules are pre-imported, but must be not:
['threading']
sys.modules:
['__editable___pygolang_0_1_finder', '__future__', '__main__', '_abc', '_codecs', '_collections', '_collections_abc', '_distutils_hack', '_frozen_importlib', '_frozen_importlib_external', '_functools', '_imp', '_io', '_operator', '_signal', '_sitebuiltins', '_sre', '_stat', '_thread', '_warnings', '_weakref', '_weakrefset', 'abc', 'builtins', 'codecs', 'collections', 'contextlib', 'copyreg', 'encodings', 'encodings.aliases', 'encodings.utf_8', 'enum', 'errno', 'fnmatch', 'functools', 'genericpath', 'gpython', 'importlib', 'importlib._abc', 'importlib._bootstrap', 'importlib._bootstrap_external', 'importlib.machinery', 'importlib.util', 'io', 'ipaddress', 'itertools', 'keyword', 'marshal', 'ntpath', 'operator', 'os', 'os.path', 'pathlib', 'posix', 'posixpath', 're', 're._casefix', 're._compiler', 're._constants', 're._parser', 'reprlib', 'site', 'stat', 'sys', 'threading', 'time', 'types', 'urllib', 'urllib.parse', 'warnings', 'zipimport', 'zope']The problem is there because importlib.util started to import threading after
46f821d62b5a, and because setuptools
emits importlib.util usage in installed *-nspkg.pth and in generated finders for packages installed in editable mode:
https://github.com/pypa/setuptools/blob/92b45e9817ae829a5ca5a5962313a56b943cad91/setuptools/namespaces.py#L46-L61
https://github.com/pypa/setuptools/blob/92b45e98/setuptools/command/editable_wheel.py#L786-L790
So for example the following breaks because e.g. zope.event is installed via nspkg way:
kirr@deca:~/tmp/trashme/X$ python3.12 -m venv 1.venv
kirr@deca:~/tmp/trashme/X$ . 1.venv/bin/activate
(1.venv) kirr@deca:~/tmp/trashme/X$ pip list
Package Version
------- -------
pip 24.0
(1.venv) kirr@deca:~/tmp/trashme/X$ pip install zope.event
Collecting zope.event
Using cached zope.event-5.0-py3-none-any.whl.metadata (4.4 kB)
Collecting setuptools (from zope.event)
Using cached setuptools-69.5.1-py3-none-any.whl.metadata (6.2 kB)
Using cached zope.event-5.0-py3-none-any.whl (6.8 kB)
Using cached setuptools-69.5.1-py3-none-any.whl (894 kB)
Installing collected packages: setuptools, zope.event
Successfully installed setuptools-69.5.1 zope.event-5.0
(1.venv) kirr@deca:~/tmp/trashme/X$ python -c 'import sys; assert "threading" not in sys.modules, sys.modules'
Traceback (most recent call last):
File "<string>", line 1, in <module>
AssertionError: {'threading': <module 'threading' ...>, 'importlib.util': <module 'importlib.util' (frozen)>, ...}So would you please consider applying the following suggested patch to fix this problem:
--- a/Lib/importlib/util.py
+++ b/Lib/importlib/util.py
@@ -15,7 +15,7 @@
import _imp
import functools
import sys
-import threading
+#import threading delayed to avoid pre-importing threading early at startup
import types
import warnings
@@ -316,7 +316,7 @@ def exec_module(self, module):
loader_state = {}
loader_state['__dict__'] = module.__dict__.copy()
loader_state['__class__'] = module.__class__
- loader_state['lock'] = threading.RLock()
+ loader_state['lock'] = __import__('threading').RLock()
loader_state['is_loading'] = False
module.__spec__.loader_state = loader_state
module.__class__ = _LazyModule?
Thanks beforehand,
Kirill
/cc @effigies, @vstinner, @ericsnowcurrently, @doko42, @brettcannon
/cc @jamadden, @arcivanov, @maciejp-ro
/cc @eduardo-elizondo, @wilson3q, @vsajip
(*) or the list of C-implemented lock instances is limited and well defined -
for example gevent reinstantiates thrading._active_limbo_lock on the
monkey-patching.
CPython versions tested on:
3.11, 3.12, CPython main branch
Operating systems tested on:
Linux
Activity
[3.12] gh-117983: Defer import of threading for lazy module loading (G…
[3.13] gh-117983: Defer import of threading for lazy module loading (G…
brettcannon commentedon Jul 3, 2024
All merged!
brettcannon commentedon Jul 3, 2024
And thanks to @effigies for staying on top of this!
navytux commentedon Jul 4, 2024
@brettcannon, thanks a lot for merging the fixes, and @effigies, thanks, once again, for improving things here.
I see that the patch landed to 3.12, 3.13 and main, but, as original description of the issue explains, the bug was introduced in 3.11.10, so is there a possibility to backport to 3.11 as well?
vsajip commentedon Jul 4, 2024
Probably not, as versions 3.11 and earlier receive security fixes only.
navytux commentedon Jul 4, 2024
@vsajip, thanks for feedback. I understand your reasoning, but it was a regression introduced in 3.11.10. From this point of view this patch is not a new addition, but reverts that regression back. In other words it does not add something new, but removes what was added in a recent point release and found to cause problems. For me it goes as common sense to apply the fix to 3.11.x as well, because else 3.11 leaves the whole gevent ecosystem in broken state.
effigies commentedon Jul 4, 2024
#120233 (comment)
jaraco commentedon Jul 4, 2024
@pablogsal Would you consider this request to backport to 3.11 one more time in light of this additional information (that the downstream regression was introduced in a security-fix release)? (apologies if that was already a factor in your consideration)
brettcannon commentedon Jul 4, 2024
@jaraco the change was actually made while 3.11 was open to bugfixes: 46f821d was committed on Feb 26 and Python 3.11 entered security-only on April 2 w/ the 3.11.9 release.
pythongh-117983: Defer import of threading for lazy module loading (p…
pythongh-117983: Defer import of threading for lazy module loading (p…
chore: upgrade testrunner image (#10405)
sapling: demandimport: Fix unknown python exception, module `threadin…
sapling: demandimport: Fix unknown python exception, module `threadin…
sapling: demandimport: Fix unknown python exception, module `threadin… (