diff --git a/provider/mem/mem.go b/provider/mem/mem.go index 0e39ba1d5f..9e1db8a161 100644 --- a/provider/mem/mem.go +++ b/provider/mem/mem.go @@ -149,9 +149,9 @@ func max(a, b int) int { // resolved and successfully notified about. // They are not guaranteed to be in chronological order. func (a *Alerts) Subscribe() provider.AlertIterator { + a.mtx.Lock() defer a.mtx.Unlock() - var ( done = make(chan struct{}) alerts = a.alerts.List() diff --git a/provider/mem/mem_test.go b/provider/mem/mem_test.go index a45321844c..85aa97e63d 100644 --- a/provider/mem/mem_test.go +++ b/provider/mem/mem_test.go @@ -135,6 +135,63 @@ func TestAlertsSubscribePutStarvation(t *testing.T) { } } +func TestDeadLock(t *testing.T) { + t0 := time.Now() + t1 := t0.Add(5 * time.Second) + + marker := types.NewMarker(prometheus.NewRegistry()) + // Run gc every 5 milliseconds to increase the possibility of a deadlock with Subscribe() + alerts, err := NewAlerts(context.Background(), marker, 5*time.Millisecond, noopCallback{}, log.NewNopLogger(), nil) + if err != nil { + t.Fatal(err) + } + alertsToInsert := []*types.Alert{} + for i := 0; i < 200+1; i++ { + alertsToInsert = append(alertsToInsert, &types.Alert{ + Alert: model.Alert{ + // Make sure the fingerprints differ + Labels: model.LabelSet{"iteration": model.LabelValue(strconv.Itoa(i))}, + Annotations: model.LabelSet{"foo": "bar"}, + StartsAt: t0, + EndsAt: t1, + GeneratorURL: "http://example.com/prometheus", + }, + UpdatedAt: t0, + Timeout: false, + }) + } + + if err := alerts.Put(alertsToInsert...); err != nil { + t.Fatal("Unable to add alerts") + } + done := make(chan bool) + + // call subscribe repeatedly in a goroutine to increase + // the possibility of a deadlock occurring + go func() { + tick := time.NewTicker(10 * time.Millisecond) + defer tick.Stop() + stopAfter := time.After(1 * time.Second) + for { + select { + case <-tick.C: + alerts.Subscribe() + case <-stopAfter: + done <- true + break + } + } + }() + + select { + case <-done: + // no deadlock + alerts.Close() + case <-time.After(10 * time.Second): + t.Error("Deadlock detected") + } +} + func TestAlertsPut(t *testing.T) { marker := types.NewMarker(prometheus.NewRegistry()) alerts, err := NewAlerts(context.Background(), marker, 30*time.Minute, noopCallback{}, log.NewNopLogger(), nil) diff --git a/store/store.go b/store/store.go index 83f1495a0f..a0463f0d5c 100644 --- a/store/store.go +++ b/store/store.go @@ -71,16 +71,16 @@ func (a *Alerts) Run(ctx context.Context, interval time.Duration) { func (a *Alerts) gc() { a.Lock() - defer a.Unlock() - var resolved []*types.Alert + defer a.cb(resolved) + defer a.Unlock() for fp, alert := range a.c { if alert.Resolved() { delete(a.c, fp) resolved = append(resolved, alert) } } - a.cb(resolved) + } // Get returns the Alert with the matching fingerprint, or an error if it is