From cdaf1512eb8f703c20eb5f4091c59b9bf16bb42a Mon Sep 17 00:00:00 2001
From: Anand Rajagopal <anrajag@amazon.com>
Date: Thu, 8 Feb 2024 18:09:55 +0000
Subject: [PATCH] A small fix to avoid deadlock that can happen as mentioned in
 issue #3682

Signed-off-by: Anand Rajagopal <anrajag@amazon.com>
---
 provider/mem/mem.go      |  1 -
 provider/mem/mem_test.go | 57 ++++++++++++++++++++++++++++++++++++++++
 store/store.go           |  3 +--
 3 files changed, 58 insertions(+), 3 deletions(-)

diff --git a/provider/mem/mem.go b/provider/mem/mem.go
index 0e39ba1d5f..9240f1c1bd 100644
--- a/provider/mem/mem.go
+++ b/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()
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..b0734cab54 100644
--- a/store/store.go
+++ b/store/store.go
@@ -71,8 +71,6 @@ 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() {
@@ -80,6 +78,7 @@ func (a *Alerts) gc() {
 			resolved = append(resolved, alert)
 		}
 	}
+	a.Unlock()
 	a.cb(resolved)
 }