From 8d320ef56bdeb56cfdbb466681daf025e812a217 Mon Sep 17 00:00:00 2001 From: rcourtman Date: Tue, 11 Nov 2025 23:58:18 +0000 Subject: [PATCH] Fix notification manager deadlock in Stop() Critical deadlock fix: - Stop() was holding n.mu lock while calling queue.Stop() - queue.Stop() waits for worker goroutines to finish - Worker goroutines call ProcessQueuedNotification() which needs n.mu lock - This created a classic lock-order deadlock Fix: - Unlock n.mu before calling queue.Stop() - Relock after queue shutdown completes - Workers can now finish and acquire lock as needed This resolves 30-second test timeouts in notifications package. Tests now complete in <1s instead of timing out at 30s. --- internal/notifications/notifications.go | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/internal/notifications/notifications.go b/internal/notifications/notifications.go index 0f3e38928..26e54a3a9 100644 --- a/internal/notifications/notifications.go +++ b/internal/notifications/notifications.go @@ -2537,16 +2537,26 @@ func (n *NotificationManager) cleanupOldNotificationRecords() { // Stop gracefully stops the notification manager func (n *NotificationManager) Stop() { n.mu.Lock() - defer n.mu.Unlock() // Stop cleanup goroutine close(n.stopCleanup) + // Get queue reference before unlocking + queue := n.queue + + // Unlock before stopping queue to avoid deadlock with queue workers + // that may need to acquire n.mu during ProcessQueuedNotification + n.mu.Unlock() + // Stop the notification queue if it exists - if n.queue != nil { - n.queue.Stop() + if queue != nil { + queue.Stop() } + // Relock for remaining cleanup + n.mu.Lock() + defer n.mu.Unlock() + // Cancel any pending group timer if n.groupTimer != nil { n.groupTimer.Stop()