Skip to content

[3008.x] Fix minion PublishServer / _TCPPubServerPublisher unclosed leaks (#70175) - #70206

Open
dwoz wants to merge 1 commit into
saltstack:3008.xfrom
dwoz:dwoz/fix/70175-minion-unclosed-leaks
Open

[3008.x] Fix minion PublishServer / _TCPPubServerPublisher unclosed leaks (#70175)#70206
dwoz wants to merge 1 commit into
saltstack:3008.xfrom
dwoz:dwoz/fix/70175-minion-unclosed-leaks

Conversation

@dwoz

@dwoz dwoz commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Symptom

Issue #70175: three ResourceWarnings fire at minion shutdown

WARNING salt.transport.tcp: unclosed publish server <PublishServer ...>
WARNING salt.utils.asynchronous: unclosed SyncWrapper for cls=<_TCPPubServerPublisher>
WARNING salt.transport.tcp: unclosed publisher client <_TCPPubServerPublisher ...>

Root cause

MinionManager._bind creates the local event_publisher (a PublishServer graph) and event (a SaltEvent). Only MinionManager.stop_async (the SIGTERM handler path) closes them. MinionManager.destroy -- reached from cli.daemons.Minion.shutdown on KeyboardInterrupt / SaltSystemExit / early-exit guards, and from __del__ on GC -- did not, so any non-SIGTERM shutdown leaked the graph.

Fix

Add the missing close/destroy chain to MinionManager.destroy, guarded with try/except so a finalizer never propagates.

Test plan

  • New unit test test_minion_manager_destroy_closes_event_publisher in tests/pytests/unit/test_minion.py asserts destroy() closes event_publisher (_closing=True) and destroys event (subscriber/pusher cleared). Verified to fail against unfixed 3008.x and pass with the fix.
  • Existing test_minion_manager_async_stop / test_stop_async_calls_notify_stopping_and_terminates_subprocess_list still pass.
  • Full tests/pytests/unit/test_minion.py: 50 passed, 28 skipped.

…utdown

MinionManager.destroy() (invoked from cli.daemons.Minion.shutdown on
KeyboardInterrupt / SaltSystemExit / early-exit and from __del__ on GC)
was missing the close/destroy chain for the local event_publisher
PublishServer graph and the event SaltEvent that MinionManager._bind
creates. Only the SIGTERM stop_async path closed them, so any
non-SIGTERM shutdown leaked the graph and surfaced the three-warning
cascade reported in saltstack#70175 (unclosed publish server / SyncWrapper /
publisher client).
@dwoz
dwoz requested a review from a team as a code owner September 1, 2026 22:56
@dwoz dwoz added the test:full Run the full test suite label Sep 1, 2026
@dwoz dwoz added this to the Argon v3008.3 milestone Sep 1, 2026
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