Skip to content

Commit 99a271e

Browse files
author
Sunil Thaha
committed
fix(monitor): race condition between timer and context cancelation
This commit fixes a race condition when timer fires ⚡️ and context gets canceled 💥 at the same time. This race condition was detected the the TestCollectionCancellation and happens as follows ... ```` monitor_collection_test.go:201: Error Trace: .../monitor_collection_test.go:201 Error: Not equal: expected: time.Date(2025, time.June, 11, 8, 14, 45, 720156604, time.Local) actual : time.Date(2025, time.June, 11, 8, 14, 45, 732389967, time.Local) Test: TestCollectionCancellation Messages: No collections should happen after cancellation ```` Race Sequence -------------- 1. **Test Context**: `TestCollectionCancellation` runs with a 100ms timeout context 2. **Timer Setup**: Collection goroutines are scheduled with 10ms intervals 3. **Cancellation**: After ~50ms, test calls `cancel()` which triggers `pm.collectionCancel()` 4. **Run() Exit**: The `Run()` method exits and test captures `snapshotAfterRun.Timestamp` 5. **Race Window**: 💥 There's a brief window where: - Context is canceled (`pm.collectionCtx.Done()` becomes ready) - Timer also fires (`<-timer` becomes ready) - Go's `select` **randomly** chooses between the two ready channels 6. **Failure Case**: When `select` chooses `<-timer`: - `synchronizedPowerRefresh()` executes and updates snapshot with new timestamp - Test assertion fails because timestamps don't match Signed-off-by: Sunil Thaha <[email protected]>
1 parent 02d572a commit 99a271e

File tree

1 file changed

+8
-1
lines changed

1 file changed

+8
-1
lines changed

internal/monitor/monitor.go

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -175,13 +175,20 @@ func (pm *PowerMonitor) scheduleNextCollection() {
175175
go func() {
176176
select {
177177
case <-timer:
178+
// Check if context is cancelled before doing any work to avoid a race condition
179+
// where the context is cancelled after the timer has expired
180+
if err := pm.collectionCtx.Err(); err != nil {
181+
pm.logger.Info("Collection loop terminated; context canceled", "reason", err)
182+
return
183+
}
184+
178185
if err := pm.synchronizedPowerRefresh(); err != nil {
179186
pm.logger.Error("Failed to collect power data", "error", err)
180187
}
181188
pm.scheduleNextCollection()
182189

183190
case <-pm.collectionCtx.Done():
184-
pm.logger.Info("Collection loop terminated")
191+
pm.logger.Info("Collection loop terminated", "reason", pm.collectionCtx.Err())
185192
return
186193
}
187194
}()

0 commit comments

Comments
 (0)