Skip to content

Commit fa514c7

Browse files
net/netmon: do not abandon a subscriber when exiting early (tailscale#17899) (tailscale#17905)
LinkChangeLogLimiter keeps a subscription to track rate limits for log messages. But when its context ended, it would exit the subscription loop, leaving the subscriber still alive. Ensure the subscriber gets cleaned up when the context ends, so we don't stall event processing. Updates tailscale/corp#34311 Change-Id: I82749e482e9a00dfc47f04afbc69dd0237537cb2 (cherry picked from commit ab4b990) Signed-off-by: M. J. Fromberger <fromberger@tailscale.com> Co-authored-by: M. J. Fromberger <fromberger@tailscale.com>
1 parent ea8eeeb commit fa514c7

File tree

1 file changed

+3
-19
lines changed

1 file changed

+3
-19
lines changed

net/netmon/loghelper.go

Lines changed: 3 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -18,13 +18,13 @@ import (
1818
// done.
1919
func LinkChangeLogLimiter(ctx context.Context, logf logger.Logf, nm *Monitor) logger.Logf {
2020
var formatSeen sync.Map // map[string]bool
21-
nm.b.Monitor(nm.changeDeltaWatcher(nm.b, ctx, func(cd ChangeDelta) {
21+
sub := eventbus.SubscribeFunc(nm.b, func(cd ChangeDelta) {
2222
// If we're in a major change or a time jump, clear the seen map.
2323
if cd.Major || cd.TimeJumped {
2424
formatSeen.Clear()
2525
}
26-
}))
27-
26+
})
27+
context.AfterFunc(ctx, sub.Close)
2828
return func(format string, args ...any) {
2929
// We only store 'true' in the map, so if it's present then it
3030
// means we've already logged this format string.
@@ -42,19 +42,3 @@ func LinkChangeLogLimiter(ctx context.Context, logf logger.Logf, nm *Monitor) lo
4242
logf(format, args...)
4343
}
4444
}
45-
46-
func (nm *Monitor) changeDeltaWatcher(ec *eventbus.Client, ctx context.Context, fn func(ChangeDelta)) func(*eventbus.Client) {
47-
sub := eventbus.Subscribe[ChangeDelta](ec)
48-
return func(ec *eventbus.Client) {
49-
for {
50-
select {
51-
case <-ctx.Done():
52-
return
53-
case <-sub.Done():
54-
return
55-
case change := <-sub.Events():
56-
fn(change)
57-
}
58-
}
59-
}
60-
}

0 commit comments

Comments
 (0)