[MEDIUM] Goroutine Leak in WebSocket Connection

[MacJediWizard]

Description

The ping loop goroutine in WebSocket connections and the update check goroutine after report have no mechanism to stop when the connection closes or parent context is cancelled.

Locations

WebSocket ping loop

`cmd/patchmon-agent/commands/serve.go` lines 372-379

go func() {
    t := time.NewTicker(30 * time.Second)
    defer t.Stop()
    for range t.C {
        _ = conn.WriteControl(websocket.PingMessage, nil, time.Now().Add(5*time.Second))
    }
}()

Post-report update check

`cmd/patchmon-agent/commands/report.go` lines 247-280

go func() {
    time.Sleep(5 * time.Second)
    // ... update check logic with no cancellation mechanism
}()

Impact

• Goroutines accumulate over time, causing memory leaks
• Resources not properly released when connections close

Recommended Fix

Pass context to goroutines and check for cancellation:

go func(ctx context.Context) {
    t := time.NewTicker(30 * time.Second)
    defer t.Stop()
    for {
        select {
        case <-ctx.Done():
            return
        case <-t.C:
            if err := conn.WriteControl(websocket.PingMessage, nil, time.Now().Add(5*time.Second)); err != nil {
                return
            }
        }
    }
}(ctx)

Severity

🟡 MEDIUM - Resource leak

Labels

bug, medium, code-quality

[MacJediWizard]

Fixed: Added proper goroutine cancellation mechanisms.

Changes made:

1. serve.go - WebSocket ping loop:

   • Added done channel that closes when connection ends
   • Ping loop now uses select to listen for done signal
   • Also handles write errors to exit gracefully

2. serve.go - Docker events goroutine:

   • Updated to use select with done channel
   • Properly exits when connection closes

3. report.go - Post-report update check:

   • Added sync.WaitGroup to wait for goroutine completion
   • Added context.WithTimeout (2 minutes) to prevent indefinite hanging
   • Initial delay now uses select for cancellation support

Code pattern used:

done := make(chan struct{})
defer close(done)

go func() {
    select {
    case <-done:
        return
    case <-t.C:
        // Do work
    }
}()

This ensures goroutines are properly cleaned up when the parent function returns.