From afbb0109c5f9c4ce090638d84c6f0245a5dc6333 Mon Sep 17 00:00:00 2001 From: Vaadin Bot Date: Tue, 26 Nov 2024 10:33:27 +0100 Subject: [PATCH] fix: make Popover static methods thread-safe (#6864) (CP: 24.5) (#6868) Co-authored-by: Diego Cardoso --- .../flow/component/popover/Popover.java | 7 +-- .../flow/component/popover/PopoverTest.java | 54 +++++++++++++++++++ 2 files changed, 58 insertions(+), 3 deletions(-) diff --git a/vaadin-popover-flow-parent/vaadin-popover-flow/src/main/java/com/vaadin/flow/component/popover/Popover.java b/vaadin-popover-flow-parent/vaadin-popover-flow/src/main/java/com/vaadin/flow/component/popover/Popover.java index dbe923058ac..e3c8d3dcbb5 100644 --- a/vaadin-popover-flow-parent/vaadin-popover-flow/src/main/java/com/vaadin/flow/component/popover/Popover.java +++ b/vaadin-popover-flow-parent/vaadin-popover-flow/src/main/java/com/vaadin/flow/component/popover/Popover.java @@ -21,6 +21,7 @@ import java.util.Map; import java.util.Objects; import java.util.Optional; +import java.util.concurrent.atomic.AtomicBoolean; import java.util.stream.Collectors; import com.vaadin.flow.component.AttachEvent; @@ -67,7 +68,8 @@ public class Popover extends Component implements HasAriaLabel, HasComponents, private static Integer defaultHideDelay; private static Integer defaultFocusDelay; private static Integer defaultHoverDelay; - private static boolean uiInitListenerRegistered = false; + final static AtomicBoolean uiInitListenerRegistered = new AtomicBoolean( + false); private Component target; private Registration targetAttachRegistration; @@ -148,11 +150,10 @@ private static void applyConfiguration() { applyConfigurationForUI(UI.getCurrent()); } - if (!uiInitListenerRegistered) { + if (uiInitListenerRegistered.compareAndSet(false, true)) { // Apply the popover configuration for all new UIs VaadinService.getCurrent() .addUIInitListener(e -> applyConfigurationForUI(e.getUI())); - uiInitListenerRegistered = true; } } diff --git a/vaadin-popover-flow-parent/vaadin-popover-flow/src/test/java/com/vaadin/flow/component/popover/PopoverTest.java b/vaadin-popover-flow-parent/vaadin-popover-flow/src/test/java/com/vaadin/flow/component/popover/PopoverTest.java index ec46d31d221..f7b77020d44 100644 --- a/vaadin-popover-flow-parent/vaadin-popover-flow/src/test/java/com/vaadin/flow/component/popover/PopoverTest.java +++ b/vaadin-popover-flow-parent/vaadin-popover-flow/src/test/java/com/vaadin/flow/component/popover/PopoverTest.java @@ -15,12 +15,22 @@ */ package com.vaadin.flow.component.popover; +import static org.mockito.Mockito.times; + +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.stream.IntStream; + import org.junit.Assert; import org.junit.Before; import org.junit.Test; +import org.mockito.Mockito; import com.vaadin.flow.component.Text; import com.vaadin.flow.component.html.Div; +import com.vaadin.flow.server.VaadinService; import elemental.json.JsonArray; @@ -294,4 +304,48 @@ public void popoverWithContent() { Assert.assertSame(content, popoverWithContent.getChildren().findFirst().get()); } + + @Test + public void testSetDefaultFocusDelay_threadSafety() { + testStaticSettersThreadsSafety( + () -> Popover.setDefaultFocusDelay(1000)); + } + + @Test + public void testSetDefaultHoverDelay_threadSafety() { + testStaticSettersThreadsSafety( + () -> Popover.setDefaultHoverDelay(1000)); + } + + @Test + public void testSetDefaultHideDelay_threadSafety() { + testStaticSettersThreadsSafety(() -> Popover.setDefaultHideDelay(1000)); + } + + private void testStaticSettersThreadsSafety(Runnable tester) { + // Reset the static state for each test + Popover.uiInitListenerRegistered.set(false); + final VaadinService current = Mockito.mock(VaadinService.class); + + ExecutorService executorService = Executors.newFixedThreadPool(10); + final CountDownLatch latch = new CountDownLatch(10); + final Runnable runnable = () -> { + VaadinService.setCurrent(current); + latch.countDown(); + for (int i = 0; i < 100000; i++) { + tester.run(); + } + }; + final var list = IntStream.range(0, 10) + .mapToObj(it -> executorService.submit(runnable)).toList(); + for (var future : list) { + try { + future.get(); + } catch (InterruptedException | ExecutionException e) { + throw new RuntimeException(e); + } + } + executorService.shutdown(); + Mockito.verify(current, times(1)).addUIInitListener(Mockito.any()); + } }