Fix #73: notify SSE goroutine lifecycle #82

Merged

crueber merged 1 commit from fix/issue-73 into main

2026-09-05 00:41:24 +00:00

crueber commented

2026-09-05 00:39:02 +00:00

Owner

Change (sender-closes discipline, 13 channel rule):

internal/notify/stream.go: sseWriter gains a sender-owned done channel (closed once under the ended guard) + the request context; the keepalive loop selects on both instead of ranging ka.C.
Sibling scope: the same latent shape existed in internal/api/sse.go (the task-stream SSE envelope) — Close() only stopped the ticker. Now ends the stream via stopLocked (sets ended, closes done, stops ticker). Wire contract (10 s keepalive, exit on disconnect) unchanged in both.
Checked and clear: internal/notify/tasks.go Run and internal/events/bridge.go Run both select on ctx.Done() — no change needed.
Doc note appended to docs/features/06_notifications.md Decisions (law 12).

Regression tests (poll NumGoroutine to baseline, 5s deadline, no fixed sleeps): TestSSEWriterKeepaliveExitsOnClose / ...OnContextCancel (notify) and TestSSEKeepaliveExitsOnClose / ...OnContextCancel (api). Verified both close-path tests FAIL on pre-fix code (5 leaked goroutines, stacks naming the range) and pass after.

Results: gofmt clean, go vet clean, go test -race green for internal/notify (coverage 97.2%, gate >=95%) and internal/api.

Fixes #73 (codex major): the per-user notify SSE keepalive goroutine leaked one per disconnected stream — stream.go ranged over s.ka.C while close() only called Ticker.Stop(), which never closes the channel. Change (sender-closes discipline, 13 channel rule): - internal/notify/stream.go: sseWriter gains a sender-owned done channel (closed once under the ended guard) + the request context; the keepalive loop selects on both instead of ranging ka.C. - Sibling scope: the same latent shape existed in internal/api/sse.go (the task-stream SSE envelope) — Close() only stopped the ticker. Now ends the stream via stopLocked (sets ended, closes done, stops ticker). Wire contract (10 s keepalive, exit on disconnect) unchanged in both. - Checked and clear: internal/notify/tasks.go Run and internal/events/bridge.go Run both select on ctx.Done() — no change needed. - Doc note appended to docs/features/06_notifications.md Decisions (law 12). Regression tests (poll NumGoroutine to baseline, 5s deadline, no fixed sleeps): TestSSEWriterKeepaliveExitsOnClose / ...OnContextCancel (notify) and TestSSEKeepaliveExitsOnClose / ...OnContextCancel (api). Verified both close-path tests FAIL on pre-fix code (5 leaked goroutines, stacks naming the range) and pass after. Results: gofmt clean, go vet clean, go test -race green for internal/notify (coverage 97.2%, gate >=95%) and internal/api.

crueber added 1 commit

2026-09-05 00:39:03 +00:00

Fix #73 : terminate SSE keepalive goroutines on stream teardown f2ac4d17e3

notify/stream.go sseWriter ranged over ka.C; close() only Stopped the
ticker, which never closes its channel, leaking one goroutine (#13) per
disconnected per-user stream. The writer now owns a done channel (closed
once under the ended guard) plus the request context; the keepalive loop
selects on both (13 channel rule: sender owns and closes).

Same latent shape fixed in the sibling internal/api SSE envelope:
Close() ends the stream via stopLocked instead of only stopping the
ticker. Wire contract (10 s keepalive, exit on disconnect) unchanged;
note appended to 06_notifications.md Decisions.

Regression tests poll NumGoroutine back to baseline with a 5 s deadline
(no fixed sleeps): close path (the handler defer) and ctx-cancel path,
both packages. Verified failing on pre-fix code (5 leaked), passing
after.

crueber referenced this pull request

2026-09-05 00:39:05 +00:00

[codex major] Per-user notify SSE leaks one goroutine per disconnected stream #73

crueber referenced this pull request

2026-09-05 00:41:08 +00:00

[codex major] Per-user notify SSE leaks one goroutine per disconnected stream #73

crueber merged commit 7a42f484d5 into main

2026-09-05 00:41:24 +00:00

crueber referenced this pull request from a commit

2026-09-05 00:41:25 +00:00

Merge pull request 'Fix #73: notify SSE goroutine lifecycle' (#82) from fix/issue-73 into main

crueber referenced this pull request