fix: delete the timer entry on natural expiry

2 files changed, 38 insertions, 2 deletions

diff --git a/internal/notify/service.go b/internal/notify/service.go
index d775aa6..22d7830 100644
--- a/internal/notify/service.go
+++ b/internal/notify/service.go
@@ -115,9 +115,21 @@ func (s *Service) arm(id uint32, ms int64) {
 	if ms <= 0 {
 		return
 	}
-	s.timers[id] = time.AfterFunc(time.Duration(ms)*time.Millisecond, func() {
-		s.store.Expire(id)
+	var t *time.Timer
+	t = time.AfterFunc(time.Duration(ms)*time.Millisecond, func() {
+		s.mu.Lock()
+		current := s.timers[id] == t
+		if current {
+			delete(s.timers, id)
+		}
+		s.mu.Unlock()
+		// Only the current timer may expire the id: a timer left over from a
+		// replaced notification must not close its successor.
+		if current {
+			s.store.Expire(id)
+		}
 	})
+	s.timers[id] = t
 }
 
 func (s *Service) stopTimer(id uint32) {
diff --git a/internal/notify/service_test.go b/internal/notify/service_test.go
index e5395db..1e9f771 100644
--- a/internal/notify/service_test.go
+++ b/internal/notify/service_test.go
@@ -143,3 +143,27 @@ func TestCloseNotificationEmitsReasonThree(t *testing.T) {
 		t.Fatal("no NotificationClosed signal")
 	}
 }
+
+// A natural expiry must drop the timer entry, or the map grows by one timer per
+// id for the whole session. No bus is needed: the store's emit is a no-op here,
+// so the test exercises arm's closure directly.
+func TestNaturalExpiryDeletesTheTimerEntry(t *testing.T) {
+	s := &Service{dir: t.TempDir(), timers: map[uint32]*time.Timer{}}
+	s.store = NewStore(func(id, reason uint32) {}, func(live, history []Popup) {})
+	id, err := s.Notify("app", 0, "", "s", "b", nil, nil, 50)
+	if err != nil {
+		t.Fatalf("Notify: %v", err)
+	}
+
+	deadline := time.Now().Add(2 * time.Second)
+	for time.Now().Before(deadline) {
+		s.mu.Lock()
+		_, present := s.timers[id]
+		s.mu.Unlock()
+		if !present {
+			return
+		}
+		time.Sleep(5 * time.Millisecond)
+	}
+	t.Fatalf("timer entry for id %d still present after expiry", id)
+}