saltstack / saltstack/salt

[BUG] EventPublisher accumulates unix sockets on 3008.x

Open
#69,857 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

bug memory-leak
Dominant language
Python
Stars
15.7k
Forks
5.6k
Avg merge
2d 44m
Merged PRs (30d)
80

Description

Description

On 3008.x, the master's EventPublisher process accumulates open unix-socket connections to master_event_pub.ipc and master_event_pull.ipc without bound. Observed on a live 3008.2 salt-master container after 24h uptime: 7584 FDs held (7572 sockets: 3793 on master_event_pub.ipc, 3791 on master_event_pull.ipc) with 151.7 GB anon RSS in the EventPublisher process alone (Tornado IOStream._read_buffer / _write_buffer bytearrays sized to ipc_write_buffer per stream).

Same fleet, same workload, same sseape engines on 3006.23 (which still has SaltEvent.__del__): steady-state socket count on EventPublisher stays in the low double digits. The regression on 3008.x has two independent causes.

Root cause 1 — PubServer doesn't detect subscriber disconnect promptly

salt/transport/ipc.py was deleted on 3008.x and replaced by salt/transport/tcp.py::PubServer. The 3006.x IPCMessagePublisher.handle_connection installed a close callback:

stream.set_close_callback(discard_after_closed)

so a subscriber closing was detected immediately and pruned from the publisher's streams set. The 3008.x PubServer._stream_read only observes close when the next read_bytes returns — but event-bus subscribers never write to the pub socket, so the read is a permanently-pending await. Close is only discovered lazily when either the reader eventually unblocks (never, for a passive subscriber) or publish_payload throws StreamClosedError on the next write attempt to that client (only when someone actually publishes to that topic).

Under bursty publish patterns and many short-lived subscribers (rest_cherrypy request handlers, salt CLI clients, sseape engines that create-and-drop MasterEvent instances), the reader-loop reference to each Subscriber keeps every closed-but-unnoticed stream alive in self.clients.

Root cause 2 — SaltEvent has no __del__ and no ResourceWarning

On 3006.x, SaltEvent.__del__ called destroy()close_pub() + close_pull(). Commit 0c3f53d9172 ("Remove del methods from leak fixes", 2026-06-04, merged forward to 3008.x) intentionally removed those __del__ methods, arguing "explicit resource cleanup is preferred."

That's the right long-term direction, but the removal broke every caller — in-tree and out-of-tree — that predated the switch. Common leak-pattern: salt.utils.event.get_master_event(...).fire_event(...) (inline; no context manager, no destroy). This works fine on 3006.23 (GC closes the socket) and leaks on 3008.2 (nothing closes the socket).

A concrete offender: sseape.engines.jobcompletion.py has two such inline fire-and-forget patterns. But sseape isn't the only fleet-wide consumer of that idiom; any operator-authored engine, reactor, or module written before mid-2026 makes the same assumption. Silent-leak is the worst mode: no warning, no error, RSS just climbs.

Also affected — missing forward-ports

Audit of 3006.x3008.x (after the last merge-forward dd4acdc6402, 2026-07-01) surfaced five commits that didn't make it forward:

  • cb098940aad (HIGH)HighState/State leak a fileclient on init failure. Any state run whose pillar compile fails leaks a ZeroMQ RequestClient (#69637).
  • 6b0e94a0592 (MEDIUM)grains.append leaks a defaultdict into persisted grain state, corrupting it over time (#69648).
  • 28ce2e6ec46 (LOW/MEDIUM)AsyncAuth AttributeError on _creds race with creds_map (#69666).
  • 12e325881a7 (LOW) — race in s3fs._write_buckets_cache_file cache removal (#69670).
  • f6ede2170f7 (LOW) — CI pipeline reliability: netapi test-fixture key cleanup, event tagger (#69733).
Fix (this issue)

Single PR against 3008.x that:

  1. Ports subscriber close-detection back into PubServer (registers a close callback on the Tornado IOStream so self.clients.discard(client) fires the instant a peer disconnects, not on the next-write attempt).
  2. Adds a SaltEvent.__del__ that emits a ResourceWarning (does not close — deliberately keeps the "explicit cleanup" contract intact but surfaces every silent-leak caller so they can be fixed).
  3. Ports the five missing 3006.x commits above.

Out of scope for this issue but tracked separately: fixing the fire-and-forget patterns in sseape (that's an sseape-repo change, not a salt change).

Target

3008.x (also relevant to 3006.x head — SaltEvent.__del__ was removed there too — but this PR only targets 3008.x; a matching 3006.x PR can follow after review).

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with salt/transport/tcp.py::PubServer and the SaltEvent implementation, then review the five named 3006.x commits and their target areas. Verify the publisher removes disconnected clients, SaltEvent emits the specified warning, and all five fixes are ported to 3008.x without expanding into the sseape changes.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
backend, distributed-systems
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
42/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.