Skip to content

Commit ca6ab4e

Browse files
committed
The "Parallel cache failure" test failure
this had two root causes, both related to Python 3.14's new default multiprocessing start method (forkserver, via PEP 741): Root Cause 1: `spawning_platform()` didn't recognize `forkserver` salt/utils/platform.py only checked for "spawn", not "forkserver". This caused AttributeError: 'Process' object has no attribute '_args_for_getstate' because Process.__new__ didn't set up pickling support for forkserver-spawned children. Fix: Changed spawning_platform() to return True for both "spawn" and "forkserver". Root Cause 2: Circular import when `extmods/utils/platform.py` shadows stdlib In Python 3.14, the forkserver itself is a fresh Python process (spawned via exec). When it creates child processes: 1. It sets sys.path from the parent salt-call process via preparation_data 2. The parent's sys.path had extmods/utils/ at index 0 (inserted by insert_system_path) 3. In the fresh child, import platform found extmods/utils/platform.py (salt's platform util) before stdlib's platform.py 4. extmods/utils/platform.py does from salt.utils.decorators import memoize which creates a circular import: • salt.utils.decorators → salt.utils.versions → salt.version → import platform → (itself) → salt.utils.decorators ♻️ Fix 1 (targeted): Replaced from salt.utils.decorators import memoize as real_memoize in salt/utils/platform.py with functools.lru_cache(maxsize=None) — a stdlib-only alternative that breaks the circular dependency. Fix 2 (defensive): Changed insert_system_path() in salt/config/__init__.py from sys.path.insert(0, ...) to sys.path.append(...), so extension module directories never appear before stdlib paths.
1 parent 7e2de42 commit ca6ab4e

3 files changed

Lines changed: 63 additions & 6 deletions

File tree

salt/config/__init__.py

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2352,14 +2352,22 @@ def prepend_root_dir(opts, path_options):
23522352
def insert_system_path(opts, paths):
23532353
"""
23542354
Inserts path into python path taking into consideration 'root_dir' option.
2355+
2356+
Paths are appended rather than prepended so that stdlib modules are never
2357+
shadowed by extension module directories (e.g. extmods/utils/). In Python
2358+
3.14+ the ``forkserver`` start method spawns child processes with a fresh
2359+
interpreter and passes the parent's ``sys.path`` via preparation_data. If
2360+
an extmods directory sits before the stdlib entries it can accidentally
2361+
shadow stdlib modules (e.g. ``platform``, ``functools``), triggering
2362+
circular imports that crash the child.
23552363
"""
23562364
if isinstance(paths, str):
23572365
paths = [paths]
23582366
for path in paths:
23592367
path_options = {"path": path, "root_dir": opts["root_dir"]}
23602368
prepend_root_dir(path_options, path_options)
23612369
if os.path.isdir(path_options["path"]) and path_options["path"] not in sys.path:
2362-
sys.path.insert(0, path_options["path"])
2370+
sys.path.append(path_options["path"])
23632371

23642372

23652373
def minion_config(

salt/utils/platform.py

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
"""
44

55
import contextlib
6+
import functools
67
import multiprocessing
78
import os
89
import platform
@@ -11,7 +12,16 @@
1112

1213
import distro
1314

14-
from salt.utils.decorators import memoize as real_memoize
15+
# Use functools.lru_cache rather than importing from salt.utils.decorators.
16+
# This module is synced to the remote's extmods/utils/platform.py, and in
17+
# Python 3.14+ (forkserver default start method) it can be accidentally
18+
# imported as the stdlib ``platform`` module when extmods/utils/ sits at
19+
# sys.path[0]. Importing from salt.utils.decorators in that context
20+
# creates a circular import:
21+
# salt.utils.decorators → salt.utils.versions → salt.version
22+
# → import platform (ourselves!) → salt.utils.decorators (cycle)
23+
# functools is part of the stdlib and has no such dependency.
24+
real_memoize = functools.cache
1525

1626

1727
def linux_distribution(full_distribution_name=True):
@@ -237,12 +247,20 @@ def is_aarch64():
237247

238248
def spawning_platform():
239249
"""
240-
Returns True if multiprocessing.get_start_method(allow_none=False) returns "spawn"
250+
Returns True if the multiprocessing start method requires pickling to transfer
251+
process state to the child. This is the case for both "spawn" and "forkserver".
241252
242-
This is the default for Windows Python >= 3.4 and macOS on Python >= 3.8.
243-
Salt, however, will force macOS to spawning by default on all python versions
253+
"spawn" is the default on Windows (Python >= 3.4) and macOS (Python >= 3.8).
254+
Salt forces macOS to spawning by default on all Python versions.
255+
256+
"forkserver" became the Linux default in Python 3.14 (via PEP 741). Like
257+
"spawn", it transfers the Process object to the child via pickle rather than
258+
inheriting it through a plain fork of the parent process. Salt must therefore
259+
treat it identically: capture *args/**kwargs in __new__ so that __getstate__
260+
can reconstruct the object on the other side, and skip parent-inherited
261+
logging teardown since the child starts with a clean file-descriptor table.
244262
"""
245-
return multiprocessing.get_start_method(allow_none=False) == "spawn"
263+
return multiprocessing.get_start_method(allow_none=False) in ("spawn", "forkserver")
246264

247265

248266
def get_machine_identifier():

tests/pytests/unit/utils/test_platform.py

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import multiprocessing
12
import subprocess
23

34
import salt.utils.platform
@@ -45,3 +46,33 @@ def test_linux_distribution():
4546
distro_version,
4647
distro_codename,
4748
)
49+
50+
51+
def test_spawning_platform_spawn():
52+
"""
53+
spawning_platform() must return True when the multiprocessing start method
54+
is "spawn" (Windows default, macOS default on Python >= 3.8).
55+
"""
56+
with patch.object(multiprocessing, "get_start_method", return_value="spawn"):
57+
assert salt.utils.platform.spawning_platform() is True
58+
59+
60+
def test_spawning_platform_forkserver():
61+
"""
62+
spawning_platform() must return True when the multiprocessing start method
63+
is "forkserver". Like "spawn", forkserver transfers the Process object to
64+
the child via pickle, so Salt must prepare __getstate__/__setstate__ for it.
65+
This is the Linux default starting with Python 3.14.
66+
"""
67+
with patch.object(multiprocessing, "get_start_method", return_value="forkserver"):
68+
assert salt.utils.platform.spawning_platform() is True
69+
70+
71+
def test_spawning_platform_fork():
72+
"""
73+
spawning_platform() must return False when the multiprocessing start method
74+
is "fork" (Linux default on Python < 3.14). Fork inherits process state
75+
directly, so pickling is not required.
76+
"""
77+
with patch.object(multiprocessing, "get_start_method", return_value="fork"):
78+
assert salt.utils.platform.spawning_platform() is False

0 commit comments

Comments
 (0)