Change where MergedListener validates and sets notificationStep, so w…

…e don't report spurious double-notifies if a notification runs after destroy caused a prior notification to be skipped
rcaudy · Nov 23, 2024 · fef79f8 · fef79f8
1 parent 94ae584
commit fef79f8
Showing 1 changed file with 9 additions and 9 deletions.
diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/MergedListener.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/MergedListener.java
@@ -341,6 +341,15 @@ public void run() {
                                 + System.identityHashCode(MergedListener.this) + ": queuedNotificationStep="
                                 + lastEnqueuedStep + ", step=" + currentStep);
                     }
+                    synchronized (MergedListener.this) {
+                        if (notificationStep == lastEnqueuedStep) {
+                            // noinspection ConstantConditions
+                            throw Assert.statementNeverExecuted("Multiple notifications in the same step: listener="
+                                    + System.identityHashCode(MergedListener.this) + ", queuedNotificationStep="
+                                    + lastEnqueuedStep);
+                        }
+                        notificationStep = lastEnqueuedStep;
+                    }
                     // Retain a reference during update processing to prevent interference from concurrent destroys
                     if (!tryRetainReference()) {
                         // This listener is no longer live, there's no point to doing any work for this notification
@@ -384,15 +393,6 @@ private void runInternal(final long currentStep) {
                 entry.onUpdateStart(added, removed, modified, shifted);
             }
             try {
-                synchronized (MergedListener.this) {
-                    if (notificationStep == lastEnqueuedStep) {
-                        // noinspection ConstantConditions
-                        throw Assert.statementNeverExecuted("Multiple notifications in the same step: listener="
-                                + System.identityHashCode(MergedListener.this) + ", queuedNotificationStep="
-                                + lastEnqueuedStep);
-                    }
-                    notificationStep = lastEnqueuedStep;
-                }
                 process();
                 getUpdateGraph().logDependencies()
                         .append("MergedListener has completed execution ")