A small fix to avoid deadlock that can happen as mentioned in issue #3682 (#3715)

Signed-off-by: Anand Rajagopal <[email protected]>

Author: rajagopalanand

provider/mem/mem.go

@@ -151,7 +151,6 @@ func max(a, b int) int {
 func (a *Alerts) Subscribe() provider.AlertIterator {
 	a.mtx.Lock()
 	defer a.mtx.Unlock()
-
 	var (
 		done   = make(chan struct{})
 		alerts = a.alerts.List()

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)

store/store.go

@@ -71,15 +71,14 @@ func (a *Alerts) Run(ctx context.Context, interval time.Duration) {
 
 func (a *Alerts) gc() {
 	a.Lock()
-	defer a.Unlock()
-
 	var resolved []*types.Alert
 	for fp, alert := range a.c {
 		if alert.Resolved() {
 			delete(a.c, fp)
 			resolved = append(resolved, alert)
 		}
 	}
+	a.Unlock()
 	a.cb(resolved)
 }