Repository navigation
Fix 2^n slowdown in wrap_sysconfig by making it idempotent (#321) - #325
Merged
Merged
Conversation
bootstrap() is re-entered every time the relenv .pth file is re-executed, and importlib.reload(site) does exactly that. Salt calls importlib.reload(site) from state.module_refresh(), so on a long-lived minion each state refresh stacks another wrapper around sysconfig.get_config_var, get_config_vars, and get_paths. Because get_config_vars_wrapper calls its wrapped function twice per invocation, N stacked layers cost 2**N calls to the innermost stdlib function. Reported measurement: 46 layers on a wedged minion, at which depth a single sysconfig.get_config_var() call takes on the order of years to return and the process spins at 100% CPU forever. Highstates never finish. Guard wrap_sysconfig with an early-return when the wrapper is already installed. functools.wraps sets __wrapped__ on our wrapper, so its presence is a reliable 'already wrapped' marker. Regression test walks the __wrapped__ chain to recover the raw stdlib functions (previous tests in the module may have already wrapped), calls wrap_sysconfig three times, and asserts the module attributes are still the same wrapper objects that the first call installed -- no stacking.
The previous version referenced Any without importing it, which ruff caught as F821. object is sufficient here since _unwrap only uses getattr and does not need any specific typing.Any semantics.
This was referenced Aug 26, 2026
Merged
dwoz
added a commit
to saltstack/salt
that referenced
this pull request
Aug 26, 2026
Picks up the wrap_sysconfig idempotence fix (saltstack/relenv#325, fixes saltstack/relenv#321) that stops the 2^n wrapper stacking that made state.module_refresh() hang long-lived Python 3.13+ onedir minions on highstates. Also picks up openssl 3.5.8 from 0.22.24. Regenerated .github/workflows/{ci,nightly,scheduled,staging}.yml via the generate-workflows pre-commit hook. Refs #70142
|
Thanks! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #321.
Summary
`bootstrap()` re-runs every time the relenv `.pth` file is re-executed, and `importlib.reload(site)` does exactly that. Salt calls `importlib.reload(site)` from `state.module_refresh()`, so on a long-lived minion each state refresh stacks another wrapper around `sysconfig.get_config_var`, `get_config_vars`, and `get_paths`.
Because `get_config_vars_wrapper` calls its wrapped function twice per invocation, N stacked layers cost 2^N calls to the innermost stdlib function. Reporter measured 46 layers on a wedged minion, at which depth a single `sysconfig.get_config_var()` call takes on the order of years to return; the process spins at 100% CPU forever and highstates never complete.
Fix
Guard `wrap_sysconfig` with an early-return when the wrapper is already installed. `functools.wraps` sets `wrapped` on our wrapper, so its presence is a reliable "already wrapped" marker (per the reporter's minimal verified patch in the issue).
Test plan