Skip to content

Fix process and file descriptor leaks in Salt Master#69227

Open
dwoz wants to merge 1 commit into
saltstack:3006.xfrom
dwoz:3006leak
Open

Fix process and file descriptor leaks in Salt Master#69227
dwoz wants to merge 1 commit into
saltstack:3006.xfrom
dwoz:3006leak

Conversation

@dwoz
Copy link
Copy Markdown
Contributor

@dwoz dwoz commented May 26, 2026

Ensure proper resource lifecycle management and process reaping to resolve leaks introduced between 3006.20 and 3006.25.

  • Call wait() after kill() in TimedProc to prevent zombie processes.
  • Implement context manager protocol and destroy() in SaltEvent, RunnerClient, WheelClient, and MasterMinion.
  • Update masterapi.py to ensure RunnerClient is used within a with statement.
  • Explicitly destroy persistent objects in RemoteFuncs and LocalFuncs during teardown.
  • Initialize internal attributes to None and fix variable scope issues to achieve 10/10 pylint rating.

self.process.terminate()

threading.Timer(10, terminate).start()
self.process.wait()
Copy link
Copy Markdown
Contributor

@Sxderp Sxderp May 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Won't this hang if the subprocess never exits (doesn't respect the kill?).

Nevermind, I got mixed up between term and kill. The kill should work..

Ensure proper resource lifecycle management and process reaping to resolve leaks introduced between 3006.20 and 3006.25.

- Call wait() after kill() in TimedProc to prevent zombie processes.
- Implement context manager protocol and destroy() in SaltEvent, RunnerClient, WheelClient, and MasterMinion.
- Update masterapi.py to ensure RunnerClient is used within a with statement.
- Explicitly destroy persistent objects in RemoteFuncs and LocalFuncs during teardown.
- Initialize internal attributes to None and fix variable scope issues to achieve 10/10 pylint rating.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

test:full Run the full test suite

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants