From 1c78cf0334061395519eb381f434822b7ae2eb96 Mon Sep 17 00:00:00 2001 From: DABH Date: Thu, 10 Sep 2026 00:42:54 -0500 Subject: [PATCH 1/3] Skip importlib for already-loaded modules in the workflow sandbox The sandbox importer routed every import, including re-imports of modules already present in the sandbox's sys.modules, through the pure-Python importlib.__import__. On Python 3.10 that path always acquires the per-module lock in importlib._bootstrap._find_and_load, and _ModuleLock.acquire is not re-entrant there: it stores the current thread in the single-slot _blocking_on dict and deletes it in a finally block (fixed in 3.12 by python/cpython#91351). A cyclic GC pass can run inside that window while a workflow module is being loaded. Finalizing a never-awaited coroutine (or showing a warning with a source object) makes the C runtime call PyImport_Import("warnings"), which goes through the sandbox's builtins.__import__ and so re-entered _ModuleLock.acquire on the same thread. The nested call removed the _blocking_on entry and the outer acquire failed with KeyError(), surfacing as "RuntimeError: Failed validating workflow" when a worker started. CPython's C import never takes the lock for an initialized module, so plain Python does not hit this for already-imported modules. Mirror that: when the target module (and, for from-imports of a package, every requested attribute) is already fully imported, return it directly and only fall back to importlib.__import__ for real loads. Module identity, passthrough handling and restriction wrapping are unchanged, and imports executed inside workflow code get cheaper on every Python version. --- CHANGELOG.md | 7 ++++ .../worker/workflow_sandbox/_importer.py | 30 +++++++++++++- .../worker/workflow_sandbox/test_importer.py | 41 +++++++++++++++++++ 3 files changed, 77 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 40e806916..d15a37759 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,6 +35,13 @@ to include examples, links to docs, or any other relevant information. ### Deprecated +### Fixed + +- Sandboxed workflow imports of already-loaded modules no longer go through importlib's module + locks, fixing intermittent `Failed validating workflow` errors on Python 3.10 caused by a + `KeyError` in `importlib._bootstrap._ModuleLock.acquire` when a garbage-collection finalizer + imported `warnings` during a workflow load ([#585](https://github.com/temporalio/sdk-python/issues/585)). + ### :boom: Breaking Changes - Experimental external storage: `ExternalStorage.driver_selector` is now called with a diff --git a/temporalio/worker/workflow_sandbox/_importer.py b/temporalio/worker/workflow_sandbox/_importer.py index 1ab0a1dd6..bc19baed5 100644 --- a/temporalio/worker/workflow_sandbox/_importer.py +++ b/temporalio/worker/workflow_sandbox/_importer.py @@ -258,7 +258,9 @@ def _import( sys.modules[full_name] = new_mod new_spec.loader.exec_module(new_mod) - mod = importlib.__import__(name, globals, locals, fromlist, level) + mod = _already_imported(name, full_name, fromlist, level) + if mod is None: + mod = importlib.__import__(name, globals, locals, fromlist, level) # Check for restrictions if necessary and apply if mod.__name__ not in self.modules_checked_for_restrictions: self.modules_checked_for_restrictions.add(mod.__name__) @@ -539,6 +541,32 @@ def _get_thread_local_builtin(name: str) -> _ThreadLocalCallable: return ret +def _already_imported( + name: str, full_name: str, fromlist: Sequence[str], level: int +) -> types.ModuleType | None: + # Mirrors importlib.__import__ for loaded modules without taking module locks + mod = _fully_imported(full_name) + if mod is None: + return None + if fromlist: + if hasattr(mod, "__path__") and any( + not isinstance(x, str) or x == "*" or not hasattr(mod, x) for x in fromlist + ): + return None + return mod + if level != 0: + return None + top = name.partition(".")[0] + return mod if top == full_name else _fully_imported(top) + + +def _fully_imported(name: str) -> types.ModuleType | None: + mod = sys.modules.get(name) + if mod is None or getattr(getattr(mod, "__spec__", None), "_initializing", False): + return None + return mod + + def _resolve_module_name( name: str, globals: Mapping[str, object] | None, level: int ) -> str: diff --git a/tests/worker/workflow_sandbox/test_importer.py b/tests/worker/workflow_sandbox/test_importer.py index 0ed478c03..394f67403 100644 --- a/tests/worker/workflow_sandbox/test_importer.py +++ b/tests/worker/workflow_sandbox/test_importer.py @@ -1,5 +1,7 @@ import dataclasses +import importlib import sys +from typing import Any import pytest @@ -27,6 +29,45 @@ def test_workflow_sandbox_importer_invalid_module(): ) +def test_workflow_sandbox_importer_repeat_import_skips_import_machinery( + monkeypatch: pytest.MonkeyPatch, +): + imported: list[str] = [] + orig_import = importlib.__import__ + + def recording_import( + name: str, + globals: Any = None, + locals: Any = None, + fromlist: Any = (), + level: int = 0, + ) -> Any: + imported.append(name) + return orig_import(name, globals, locals, fromlist, level) + + monkeypatch.setattr(importlib, "__import__", recording_import) + with Importer(restrictions, RestrictionContext()).applied(): + import tests.worker.workflow_sandbox.testmodules.passthrough_module as passthrough + import tests.worker.workflow_sandbox.testmodules.stateful_module as stateful + + assert imported + imported.clear() + + # Loaded modules are served from sys.modules without re-entering importlib + import typing + + import tests.worker.workflow_sandbox.testmodules.passthrough_module as passthrough_again + import tests.worker.workflow_sandbox.testmodules.stateful_module as stateful_again + from tests.worker.workflow_sandbox import testmodules + from tests.worker.workflow_sandbox.testmodules import stateful_module + + assert passthrough_again is passthrough + assert stateful_again is stateful is stateful_module + assert getattr(testmodules, "stateful_module") is stateful + assert typing is sys.modules["typing"] + assert imported == [] + + def test_workflow_sandbox_importer_passthrough_module(): # Import outside of importer import tests.worker.workflow_sandbox.testmodules.passthrough_module as outside1 From f2e9b992940ddaea5bb9f91a87b9da359f148de7 Mon Sep 17 00:00:00 2001 From: DABH Date: Thu, 10 Sep 2026 02:47:11 -0500 Subject: [PATCH 2/3] Check fromlist names statically in the sandbox import fast path The fast path used hasattr for fromlist names, which runs a package's module-level __getattr__ before importlib runs it again on the fallback, so a missing name was probed twice. Look only at the module dict; dynamic attributes keep going through importlib exactly as before. --- .../worker/workflow_sandbox/_importer.py | 8 ++++-- .../worker/workflow_sandbox/test_importer.py | 26 +++++++++++++++++++ .../dynamic_attr_package/__init__.py | 8 ++++++ 3 files changed, 40 insertions(+), 2 deletions(-) create mode 100644 tests/worker/workflow_sandbox/testmodules/dynamic_attr_package/__init__.py diff --git a/temporalio/worker/workflow_sandbox/_importer.py b/temporalio/worker/workflow_sandbox/_importer.py index bc19baed5..9fa2b766b 100644 --- a/temporalio/worker/workflow_sandbox/_importer.py +++ b/temporalio/worker/workflow_sandbox/_importer.py @@ -549,8 +549,12 @@ def _already_imported( if mod is None: return None if fromlist: - if hasattr(mod, "__path__") and any( - not isinstance(x, str) or x == "*" or not hasattr(mod, x) for x in fromlist + # Only statically stored attributes count; module __getattr__ stays with importlib + mod_dict = getattr(mod, "__dict__", None) + if not isinstance(mod_dict, dict): + return None + if "__path__" in mod_dict and any( + not isinstance(x, str) or x == "*" or x not in mod_dict for x in fromlist ): return None return mod diff --git a/tests/worker/workflow_sandbox/test_importer.py b/tests/worker/workflow_sandbox/test_importer.py index 394f67403..b03db30dc 100644 --- a/tests/worker/workflow_sandbox/test_importer.py +++ b/tests/worker/workflow_sandbox/test_importer.py @@ -68,6 +68,32 @@ def recording_import( assert imported == [] +def test_workflow_sandbox_importer_repeat_import_leaves_module_getattr_to_importlib(): + with Importer(restrictions, RestrictionContext()).applied(): + import tests.worker.workflow_sandbox.testmodules.dynamic_attr_package as dyn_pkg + from tests.worker.workflow_sandbox.testmodules.dynamic_attr_package import ( + dynamic_value, + ) + + assert dynamic_value == 42 + before = len(dyn_pkg.getattr_calls) + # importlib's fromlist hasattr plus the IMPORT_FROM lookup, same as without the sandbox + from tests.worker.workflow_sandbox.testmodules.dynamic_attr_package import ( # noqa: F811 + dynamic_value, + ) + + assert dynamic_value == 42 + assert dyn_pkg.getattr_calls[before:] == ["dynamic_value", "dynamic_value"] + + # A missing name is probed once by importlib and once by IMPORT_FROM, not more + before = len(dyn_pkg.getattr_calls) + with pytest.raises(ImportError): + from tests.worker.workflow_sandbox.testmodules.dynamic_attr_package import ( # type: ignore[attr-defined] + missing_value, + ) + assert dyn_pkg.getattr_calls[before:] == ["missing_value", "missing_value"] + + def test_workflow_sandbox_importer_passthrough_module(): # Import outside of importer import tests.worker.workflow_sandbox.testmodules.passthrough_module as outside1 diff --git a/tests/worker/workflow_sandbox/testmodules/dynamic_attr_package/__init__.py b/tests/worker/workflow_sandbox/testmodules/dynamic_attr_package/__init__.py new file mode 100644 index 000000000..0809882fd --- /dev/null +++ b/tests/worker/workflow_sandbox/testmodules/dynamic_attr_package/__init__.py @@ -0,0 +1,8 @@ +getattr_calls: list[str] = [] + + +def __getattr__(name: str) -> int: + getattr_calls.append(name) + if name == "dynamic_value": + return 42 + raise AttributeError(name) From 45de1cb238b2bb9777845d0a377028071817cdb4 Mon Sep 17 00:00:00 2001 From: DABH Date: Thu, 10 Sep 2026 02:49:35 -0500 Subject: [PATCH 3/3] Exercise dynamic names through __import__ in the importer test --- .../worker/workflow_sandbox/test_importer.py | 26 +++++++------------ 1 file changed, 10 insertions(+), 16 deletions(-) diff --git a/tests/worker/workflow_sandbox/test_importer.py b/tests/worker/workflow_sandbox/test_importer.py index b03db30dc..5390ab93a 100644 --- a/tests/worker/workflow_sandbox/test_importer.py +++ b/tests/worker/workflow_sandbox/test_importer.py @@ -69,28 +69,22 @@ def recording_import( def test_workflow_sandbox_importer_repeat_import_leaves_module_getattr_to_importlib(): + pkg_name = "tests.worker.workflow_sandbox.testmodules.dynamic_attr_package" with Importer(restrictions, RestrictionContext()).applied(): - import tests.worker.workflow_sandbox.testmodules.dynamic_attr_package as dyn_pkg - from tests.worker.workflow_sandbox.testmodules.dynamic_attr_package import ( - dynamic_value, - ) - - assert dynamic_value == 42 + dyn_pkg = importlib.import_module(pkg_name) + assert dyn_pkg.dynamic_value == 42 before = len(dyn_pkg.getattr_calls) - # importlib's fromlist hasattr plus the IMPORT_FROM lookup, same as without the sandbox - from tests.worker.workflow_sandbox.testmodules.dynamic_attr_package import ( # noqa: F811 - dynamic_value, - ) - assert dynamic_value == 42 + # importlib's fromlist hasattr plus the attribute read, same as without the sandbox + pkg = __import__(pkg_name, fromlist=["dynamic_value"]) + assert pkg.dynamic_value == 42 assert dyn_pkg.getattr_calls[before:] == ["dynamic_value", "dynamic_value"] - # A missing name is probed once by importlib and once by IMPORT_FROM, not more + # A missing name is probed once by importlib and once by the read, not more before = len(dyn_pkg.getattr_calls) - with pytest.raises(ImportError): - from tests.worker.workflow_sandbox.testmodules.dynamic_attr_package import ( # type: ignore[attr-defined] - missing_value, - ) + pkg = __import__(pkg_name, fromlist=["missing_value"]) + with pytest.raises(AttributeError): + getattr(pkg, "missing_value") assert dyn_pkg.getattr_calls[before:] == ["missing_value", "missing_value"]