Merge pull request #2303 from ControlSystemStudio/alarm_delayed_nonlatch

Detect race between "OK" update from PV and delayed alarm

services/alarm-server/src/main/java/org/phoebus/applications/alarm/server/AlarmLogic.java

@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2010-2021 Oak Ridge National Laboratory.
+ * Copyright (c) 2010-2022 Oak Ridge National Laboratory.
  * All rights reserved. This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License v1.0
  * which accompanies this distribution, and is available at
@@ -13,6 +13,7 @@
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.concurrent.atomic.AtomicReference;
+import java.util.logging.Level;
 
 import org.phoebus.applications.alarm.Messages;
 import org.phoebus.applications.alarm.model.AlarmState;
@@ -439,7 +440,29 @@ private void updateState(final AlarmState received_state,
     public void delayedStateUpdate(final AlarmState delayed_state)
     {
         if (isEnabled())
+        {
+            // It's been observed that an "OK" update arrived which cancels
+            // a delayed state update, but the delay expired just before
+            // the cancellation took effect.
+            // This effectively caused a reversal: current state was OK,
+            // but delayed update then raised it to an alarm.
+            // Detect this case by checking the time stamp for a current OK.
+            // Note that a delayed MAJOR would still take precedence over
+            // a more recently received MINOR.
+            // Only a recently received OK is checked for its time stamp.
+            synchronized (this)
+            {
+                if (current_state.severity == SeverityLevel.OK  &&
+                    current_state.time.isAfter(delayed_state.time))
+                {
+                    logger.log(Level.FINE, () ->
+                               "Ignoring outdated delayed state " + delayed_state +
+                               " for " + current_state);
+                    return;
+                }
+            }
             updateState(delayed_state, false);
+        }
     }
 
     /** Check if the new state adds up to 'count' alarms within 'delay'

services/alarm-server/src/main/java/org/phoebus/applications/alarm/server/DelayedAlarmUpdate.java

@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2010-2020 Oak Ridge National Laboratory.
+ * Copyright (c) 2010-2022 Oak Ridge National Laboratory.
  *  All rights reserved. This program and the accompanying materials
  *  are made available under the terms of the Eclipse Public License v1.0
  *  which accompanies this distribution, and is available at
@@ -122,8 +122,12 @@ AlarmState getState()
     public void cancel()
     {
         state.set(null);
-        final ScheduledFuture<?> task = scheduled_task;
-        scheduled_task = null;
+        final ScheduledFuture<?> task;
+        synchronized (this)
+        {
+            task = scheduled_task;
+            scheduled_task = null;
+        }
         if (task != null)
         {
             task.cancel(false);

services/alarm-server/src/test/java/org/phoebus/applications/alarm/server/AlarmLogicUnitTest.java

@@ -527,6 +527,45 @@ public void testDelayedButShort() throws Exception
         assertEquals("b", logic.getAlarmState().getValue());
     }
 
+    @Test
+    public void testUnlatchedDelayedButShortThenAnother() throws Exception
+    {
+        System.out.println("* Unlatched, annunciated, delayed: Major, clear, no alarm, then alarm which persists, clear");
+        final int delay = 2;
+        final AlarmLogicDemo logic = new AlarmLogicDemo(false, true, delay);
+        logic.check(false, false, SeverityLevel.OK, OK, SeverityLevel.OK, OK);
+        assertEquals("", logic.getAlarmState().getValue());
+
+        // MAJOR alarm has no immediate effect
+        logic.computeNewState("a", SeverityLevel.MAJOR, "very high");
+        logic.check(true, false, SeverityLevel.MAJOR, "very high", SeverityLevel.OK, OK);
+        assertEquals("", logic.getAlarmState().getValue());
+
+        // .. if it clears in time (1/2 the delay time)
+        Thread.sleep(delay * 500);
+        logic.computeNewState("b", SeverityLevel.OK, OK);
+        logic.check(true, false, SeverityLevel.OK, OK, SeverityLevel.OK, OK);
+        assertEquals("b", logic.getAlarmState().getValue());
+
+        // Assert that it stays that way
+        System.out.println("wait...");
+        Thread.sleep(delay * 1500);
+        logic.check(false, false, SeverityLevel.OK, OK, SeverityLevel.OK, OK);
+        assertEquals("b", logic.getAlarmState().getValue());
+
+        // Enter alarm
+        logic.computeNewState("c", SeverityLevel.MAJOR, "VERY high");
+        logic.check(true, false, SeverityLevel.MAJOR, "VERY high", SeverityLevel.OK, OK);
+        System.out.println("wait...");
+        Thread.sleep(delay * 1500);
+        logic.check(true, true, SeverityLevel.MAJOR, "VERY high", SeverityLevel.MAJOR, "VERY high");
+        assertEquals("c", logic.getAlarmState().getValue());
+
+        // Clear
+        logic.computeNewState("d", SeverityLevel.OK, OK);
+        logic.check(true, false, SeverityLevel.OK, OK, SeverityLevel.OK, OK);
+    }
+
     @Test
     public void testLatchedAnnunciatedDelayed() throws Exception
     {