fix: Prevent multiple effect invocations when reading the same signal multiple times (#23978)

When an effect reads the same signal multiple times (e.g. signal.get()
called 3 times), each call creates a separate Usage instance and
registers
a separate listener. When the signal changes, all listeners fire,
causing
the effect to run multiple times instead of once.

Added an AtomicBoolean flag in Effect.revalidate() that ensures
onDependencyChange is called only once per change event, even if
multiple
usages refer to the same signal. The flag is reset on each revalidate()
call so subsequent signal changes can trigger the effect again.

Fixes #23974

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>

---------

Co-authored-by: Claude <noreply@anthropic.com>

Author: mshabarov

flow-data/src/test/java/com/vaadin/flow/data/binder/BinderSignalTest.java

@@ -928,9 +928,9 @@ void bindingValue_multipleValueSignalCalls_revalidateTriggeredForEach() {
         assertEquals(3, validatorCalls.get());
 
         lastNameField.setValue("");
-        // value change runs revalidation for both Signal.get() calls in the
-        // validator
-        assertEquals(5, validatorCalls.get());
+        // value change runs revalidation only once even for
+        // two signal reads in the validator
+        assertEquals(4, validatorCalls.get());
     }
 
     private void testInitialStatusChangeRunEffects(

flow-server/src/main/java/com/vaadin/flow/signals/impl/Effect.java

@@ -181,17 +181,14 @@ private void revalidate() {
         activeEffects.get().add(this);
         try {
             boolean[] hasSignalUsage = { false };
+            // Ensure effect runs only once per change event, even if the same
+            // signal is read multiple times (each read registers a listener)
+            boolean[] changeHandled = { false };
             UsageTracker.track(action, usage -> {
                 hasSignalUsage[0] = true;
                 usages.add(usage);
-                // avoid lambda to allow proper deserialization
-                TransientListener usageListener = new TransientListener() {
-                    @Override
-                    public boolean invoke(boolean immediate) {
-                        return onDependencyChange(immediate);
-                    }
-                };
-                registrations.add(usage.onNextChange(usageListener));
+                registrations.add(usage
+                        .onNextChange(createChangeListener(changeHandled)));
             });
             if (!hasSignalUsage[0]) {
                 throw new MissingSignalUsageException(
@@ -203,6 +200,20 @@ public boolean invoke(boolean immediate) {
         }
     }
 
+    private TransientListener createChangeListener(boolean[] changeHandled) {
+        // avoid lambda to allow proper deserialization
+        return new TransientListener() {
+            @Override
+            public boolean invoke(boolean immediate) {
+                if (!changeHandled[0]) {
+                    changeHandled[0] = true;
+                    return onDependencyChange(immediate);
+                }
+                return false;
+            }
+        };
+    }
+
     private boolean onDependencyChange(boolean immediate) {
         /*
          * Detect loops by checking if the same effect is already active on this
@@ -317,8 +328,12 @@ public synchronized void activate() {
             // listener may still fire immediately if a change sneaks in
             // between the hasChanges check and the onNextChange call,
             // in which case firstRun is already set to true above.
+            // Ensure effect runs only once even if the same signal is
+            // registered multiple times
+            boolean[] changeHandled = { false };
             for (UsageTracker.Usage usage : usages) {
-                registrations.add(usage.onNextChange(this::onDependencyChange));
+                registrations.add(usage
+                        .onNextChange(createChangeListener(changeHandled)));
             }
             if (!invalidateScheduled.get()) {
                 // No invalidation was scheduled, so no change was

flow-server/src/test/java/com/vaadin/flow/dom/ElementEffectTest.java

@@ -1518,6 +1518,43 @@ void bindChildren_removeItemsWithSlottedChildPresent_slottedUnaffected() {
         assertEquals("footer", parent.getChild(1).getAttribute("slot"));
     }
 
+    @Test
+    void effect_reattachWithoutChanges_readSameSignalMultipleTimes_effectRunOnlyOnce() {
+        CurrentInstance.clearAll();
+        TestComponent component = new TestComponent();
+        ValueSignal<String> signal = new ValueSignal<>("initial");
+        AtomicInteger count = new AtomicInteger();
+        ArrayList<String> invocations = new ArrayList<>();
+        Signal.effect(component, () -> {
+            count.incrementAndGet();
+            invocations.add(signal.get());
+            signal.get();
+            signal.get();
+        });
+
+        MockUI ui = new MockUI();
+        ui.add(component);
+        assertEquals(1, count.get());
+        assertEquals(List.of("initial"), invocations);
+
+        // Detach and reattach without changes (passivate/activate path)
+        ui.remove(component);
+        ui.add(component);
+        assertEquals(1, count.get(), "No re-run on reattach without changes");
+
+        // Change signal - should trigger the effect exactly once,
+        // even though the effect reads the same signal three times
+        signal.set("update");
+        assertEquals(2, count.get(),
+                "Effect should run exactly once after reattach despite multiple reads of the same signal");
+        assertEquals(List.of("initial", "update"), invocations);
+
+        signal.set("again");
+        assertEquals(3, count.get(),
+                "Effect should run exactly once on subsequent change");
+        assertEquals(List.of("initial", "update", "again"), invocations);
+    }
+
     private TestLayout prepareTestLayout(ListSignal<String> listSignal) {
         TestLayout parentComponent = new TestLayout();
         new MockUI().add(parentComponent);

flow-server/src/test/java/com/vaadin/flow/signals/impl/EffectTest.java

@@ -901,4 +901,87 @@ void noOwnerUI_fallsBackToVaadinRequestCheck() {
         assertTrue(backgroundChanges.get(2),
                 "Change without VaadinRequest should be background");
     }
+
+    @Test
+    void changeTracking_readSameSignalMultipleTimes_effectRunOnlyOnce() {
+        SharedValueSignal<String> signal = new SharedValueSignal<>("initial");
+        ArrayList<String> invocations = new ArrayList<>();
+
+        Signal.unboundEffect(() -> {
+            invocations.add(signal.get());
+            signal.get();
+            signal.get();
+        });
+
+        assertEquals(List.of("initial"), invocations);
+
+        signal.set("update");
+        assertEquals(List.of("initial", "update"), invocations);
+
+        signal.set("again");
+        assertEquals(List.of("initial", "update", "again"), invocations);
+    }
+
+    @Test
+    void changeTracking_readMultipleDifferentSignals_effectRunsOncePerChange() {
+        SharedValueSignal<String> signal1 = new SharedValueSignal<>("a");
+        SharedValueSignal<String> signal2 = new SharedValueSignal<>("b");
+        ArrayList<String> invocations = new ArrayList<>();
+
+        Signal.unboundEffect(() -> {
+            invocations.add(signal1.get() + signal2.get());
+        });
+
+        assertEquals(List.of("ab"), invocations);
+
+        signal1.set("A");
+        assertEquals(List.of("ab", "Ab"), invocations);
+
+        signal2.set("B");
+        assertEquals(List.of("ab", "Ab", "AB"), invocations);
+    }
+
+    @Test
+    void changeTracking_multipleReadsFromTwoSignals_eachSignalChangeTriggersEffectOnce() {
+        SharedValueSignal<String> signal1 = new SharedValueSignal<>("a");
+        SharedValueSignal<Integer> signal2 = new SharedValueSignal<>(1);
+        AtomicInteger effectRunCount = new AtomicInteger(0);
+        ArrayList<String> invocations = new ArrayList<>();
+
+        Signal.unboundEffect(() -> {
+            effectRunCount.incrementAndGet();
+            // Read signal1 three times
+            String s1 = signal1.get();
+            signal1.get();
+            signal1.get();
+            // Read signal2 two times
+            Integer s2 = signal2.get();
+            signal2.get();
+            invocations.add(s1 + ":" + s2);
+        });
+
+        assertEquals(1, effectRunCount.get(),
+                "Effect should run exactly once initially");
+        assertEquals(List.of("a:1"), invocations);
+
+        signal1.set("b");
+        assertEquals(2, effectRunCount.get(),
+                "Effect should run exactly once when signal1 changes (despite 3 reads)");
+        assertEquals(List.of("a:1", "b:1"), invocations);
+
+        signal2.set(2);
+        assertEquals(3, effectRunCount.get(),
+                "Effect should run exactly once when signal2 changes (despite 2 reads)");
+        assertEquals(List.of("a:1", "b:1", "b:2"), invocations);
+
+        signal1.set("c");
+        assertEquals(4, effectRunCount.get(),
+                "Effect should run exactly once when signal1 changes again");
+        assertEquals(List.of("a:1", "b:1", "b:2", "c:2"), invocations);
+
+        signal2.set(3);
+        assertEquals(5, effectRunCount.get(),
+                "Effect should run exactly once when signal2 changes again");
+        assertEquals(List.of("a:1", "b:1", "b:2", "c:2", "c:3"), invocations);
+    }
 }