Merge pull request #217 from PHOENIXCONTACT/fix/notification-race-con…

…dition Prevent Notification from throwing InvalidOperationException in the event of a race condition in the NotificationAdapter
PHOENIXCONTACT · Jul 11, 2023 · ce2ccba · ce2ccba
2 parents a44e1d4 + 63e5900
commit ce2ccba
Show file tree

Hide file tree

Showing 2 changed files with 25 additions and 8 deletions.
diff --git a/src/Moryx.Notifications/Adapter/NotificationAdapter.cs b/src/Moryx.Notifications/Adapter/NotificationAdapter.cs
@@ -1,6 +1,7 @@
 // Copyright (c) 2020, Phoenix Contact GmbH & Co. KG
 // Licensed under the Apache License, Version 2.0
 
+using Moryx.Logging;
 using System;
 using System.Collections.Generic;
 using System.Linq;
@@ -20,6 +21,11 @@ public class NotificationAdapter : INotificationAdapter, INotificationSourceAdap
 
         private readonly ReaderWriterLockSlim _listLock = new ReaderWriterLockSlim(LockRecursionPolicy.NoRecursion);
 
+        /// <summary>
+        /// Logger for the Notification Adapter
+        /// </summary>
+        public IModuleLogger Logger { get; set; }
+
         #region Adapter <> Facade
 
         /// <inheritdoc />
@@ -61,7 +67,7 @@ public void Publish(INotificationSender sender, INotification notification, obje
 
             if (notification == null)
                 throw new ArgumentNullException(nameof(notification), "Notification must be set");
-            
+
             _listLock.EnterUpgradeableReadLock();
 
             // Lets check if the notification was already published
@@ -75,7 +81,7 @@ public void Publish(INotificationSender sender, INotification notification, obje
             }
 
             _listLock.EnterWriteLock();
-            
+
             var managed = (IManagedNotification)notification;
             managed.Identifier = Guid.NewGuid();
             managed.Created = DateTime.Now;
@@ -222,17 +228,25 @@ void INotificationSourceAdapter.PublishProcessed(INotification notification)
                 _pendingPubs.Remove(map);
                 _published.Add(map);
                 _listLock.ExitWriteLock();
+                return;
             }
-            else
-            {
-                // Notification is maybe not pending anymore - we only can acknowledge it
-                _listLock.ExitWriteLock();
 
-                var managed = (IManagedNotification)notification;
+            // Notification is maybe not pending anymore
+            _listLock.ExitWriteLock();
+            var managed = (IManagedNotification)notification;
+
+            // If necessary we can acknowledge it
+            if (managed.Acknowledged is null)
+            {
+                Logger.Log(LogLevel.Error, "Notification was removed from the pending publications " +
+                    "before being published but is not acknowledged.");
                 managed.Acknowledged = DateTime.Now;
                 managed.Acknowledger = nameof(NotificationAdapter);
                 Acknowledged?.Invoke(this, notification);
             }
+
+            Logger.Log(LogLevel.Warning, "Notification was removed from the pending publications. " +
+                "It was already acknowledged by {0} at {1}.", managed.Acknowledger, managed.Acknowledged);
         }
 
         /// <inheritdoc />

diff --git a/src/Moryx.Notifications/NotificationExtensions.cs b/src/Moryx.Notifications/NotificationExtensions.cs
@@ -1,4 +1,5 @@
 using Moryx.Container;
+using Moryx.Logging;
 
 namespace Moryx.Notifications
 {
@@ -12,7 +13,9 @@ public static class NotificationExtensions
         /// </summary>
         public static IContainer RegisterNotifications(this IContainer container)
         {
-            var adapter = new NotificationAdapter();
+            var logger = container.Resolve<IModuleLogger>();
+
+            var adapter = new NotificationAdapter { Logger = logger };
 
             container.SetInstance((INotificationAdapter)adapter, "NotificationAdapter");
             container.SetInstance((INotificationSourceAdapter)adapter, "NotificationSenderAdapter");