Skip to content

publish: NewTimer+Stop instead of time.After in the fan-out hot path - #35

Merged
matthew-pilot merged 1 commit into
mainfrom
fix/publish-timer-leak
Jul 29, 2026
Merged

publish: NewTimer+Stop instead of time.After in the fan-out hot path#35
matthew-pilot merged 1 commit into
mainfrom
fix/publish-timer-leak

Conversation

@TeoSlayer

Copy link
Copy Markdown
Contributor

publishWith spawns one goroutine per subscriber per event, and each used time.After(publishWriteTimeout).

time.After allocates a timer that the runtime pins for the full 5 seconds, even when the write completes in microseconds. At the 100 events/s publish limit with S subscribers, steady state is up to 100 x S x 5 = 500*S live timers plus the matching timer-heap entries — 50,000 pinned timers at S=100, purely as GC and timer-heap pressure.

Now time.NewTimer with defer Stop(), so the timer is released as soon as the write returns.

This is the same correction already applied in the daemon (withRegistryDeadline), where the rationale is documented in-line for exactly this reason.

Build, vet and tests pass.

Found during a codebase-wide sweep for resource leaks.

🤖 Generated with Claude Code

publishWith spawns one goroutine per subscriber per event, and each used
time.After(publishWriteTimeout).

time.After allocates a timer the runtime pins for the FULL 5 seconds, even
when the write completes in microseconds. At the 100 events/s publish limit
with S subscribers, steady state is up to 100*S*5 = 500*S live timers plus
matching timer-heap entries — 50,000 pinned timers at S=100 — purely as GC
and timer-heap pressure.

Now time.NewTimer with defer Stop(), releasing the timer as soon as the
write returns.

Same correction already applied in the daemon's withRegistryDeadline, where
the rationale is documented in-line for exactly this reason.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@codecov

codecov Bot commented Jul 28, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@matthew-pilot
matthew-pilot merged commit 688b855 into main Jul 29, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants