fix: resolve route scope UI from the detach event (CP: 24.10) (#25069)

RouteStoreWrapper#getBeanStore re-points a bean store to the UI created by a
browser refresh, but the store's detach listener stays on the previous UI.
resetUI() then asked findPreservingUI() about currentUI, which is no longer
the UI the detach event comes from.

If the refreshed UI had already been removed from the session, its session
reference is null and findPreservingUI() threw a NullPointerException. The
exception escaped before the branch that destroys the store, so the route
scoped beans were not destroyed and the store stayed in routeStores. During
session expiration the beans are still released by the session destroy
listener of RouteStoreWrapper, but when only the UIs of an idle browser
window are removed nothing cleans them up and they leak until the session
ends.

Pass the detached UI from the event into resetUI() so the lookup always gets
the UI the event originates from, which still has its session at that point.
findPreservingUI() now also returns null instead of dereferencing a UI that
has no session anymore.

Fixes #25027

Author: mcollovati

vaadin-spring/src/main/java/com/vaadin/flow/spring/scopes/VaadinRouteScope.java

@@ -297,7 +297,7 @@ private RouteBeanStore(UI ui, VaadinSession session,
         public void onComponentEvent(DetachEvent event) {
             assert getVaadinSession().hasLock();
             uiDetachRegistration.remove();
-            if (resetUI()) {
+            if (resetUI(event.getUI())) {
                 uiDetachRegistration = currentUI.addDetachListener(this);
             } else {
                 destroy();
@@ -334,8 +334,21 @@ BeanNamesWrapper getBeanNamesWrapper() {
             return beanNames;
         }
 
-        private boolean resetUI() {
-            UI ui = findPreservingUI(currentUI);
+        /**
+         * Re-points this store to the UI that replaces the given detached UI on
+         * the same browser window, if any.
+         * <p>
+         * The detached UI is passed in explicitly because {@code currentUI} may
+         * already have been re-assigned to a newer UI by
+         * {@link RouteStoreWrapper#getBeanStore(UI)}, in which case it is not
+         * the UI this detach event originates from.
+         *
+         * @param detachedUI
+         *            the UI being detached
+         * @return {@code true} if a replacement UI was found
+         */
+        private boolean resetUI(UI detachedUI) {
+            UI ui = findPreservingUI(detachedUI);
             if (ui == null) {
                 return false;
             }
@@ -457,11 +470,26 @@ private static UI getUI() {
         return ui;
     }
 
+    /**
+     * Finds the UI that took over the browser window of the given UI, e.g. on a
+     * page refresh.
+     *
+     * @param ui
+     *            the UI being detached
+     * @return the UI on the same browser window, or {@code null} if there is
+     *         none
+     */
     private static UI findPreservingUI(UI ui) {
         VaadinSession session = ui.getSession();
         String windowName = getWindowName(ui);
+        if (session == null || windowName == null) {
+            // Without a window name there is nothing to match the session UIs
+            // against, and a UI that has already been detached from the session
+            // cannot be matched at all.
+            return null;
+        }
         for (UI sessionUi : session.getUIs()) {
-            if (sessionUi != ui && windowName != null
+            if (sessionUi != ui
                     && windowName.equals(getWindowName(sessionUi))) {
                 return sessionUi;
             }

vaadin-spring/src/test/java/com/vaadin/flow/spring/scopes/AbstractScopeTest.java

@@ -24,6 +24,7 @@
 import org.springframework.beans.factory.config.Scope;
 
 import com.vaadin.flow.di.Lookup;
+import com.vaadin.flow.internal.CurrentInstance;
 import com.vaadin.flow.server.DefaultDeploymentConfiguration;
 import com.vaadin.flow.server.RouteRegistry;
 import com.vaadin.flow.server.VaadinContext;
@@ -61,6 +62,10 @@ public ReentrantLock getLockInstance() {
     @After
     public void clearSession() {
         session = null;
+        // mockSession() sets the current VaadinSession, which is stored in a
+        // thread local. Surefire reuses the JVM and its threads across test
+        // classes, so it has to be cleared to not leak into unrelated tests.
+        CurrentInstance.clearAll();
     }
 
     @Test(expected = IllegalStateException.class)

vaadin-spring/src/test/java/com/vaadin/flow/spring/scopes/VaadinRouteScopeTest.java

@@ -10,6 +10,7 @@
 
 import jakarta.servlet.ServletContext;
 
+import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
 import java.util.concurrent.atomic.AtomicInteger;
@@ -219,6 +220,102 @@ public void detachUI_uiWithDifferentWindowName_beanInScopeIsDestroyedwhenUIIsDet
         Assert.assertEquals(0, count.get());
     }
 
+    @Test
+    public void detachUI_refreshedUIDetachedFirst_beanIsDestroyedWithoutReportingError() {
+        UI ui = mockUI();
+
+        UI anotherUI = makeAnotherUI(ui);
+
+        ExtendedClientDetails details = Mockito
+                .mock(ExtendedClientDetails.class);
+        Mockito.when(details.getWindowName()).thenReturn("bar");
+        ui.getInternals().setExtendedClientDetails(details);
+        anotherUI.getInternals().setExtendedClientDetails(details);
+
+        VaadinSession session = ui.getSession();
+        session.addUI(ui);
+        session.addUI(anotherUI);
+
+        mockServletContext(ui);
+
+        VaadinRouteScope scope = initScope(ui);
+
+        AtomicInteger count = new AtomicInteger();
+        scope.registerDestructionCallback("foo", count::getAndIncrement);
+
+        navigateTo(ui, new NavigationTarget());
+
+        putObjectIntoScope(scope);
+
+        // Refresh: a request on the new UI re-points the store to it, while the
+        // store's detach listener stays on the first UI.
+        UI.setCurrent(anotherUI);
+        initScope(anotherUI);
+
+        List<Throwable> reportedErrors = new ArrayList<>();
+        Mockito.when(session.getErrorHandler())
+                .thenReturn(event -> reportedErrors.add(event.getThrowable()));
+
+        // Session expiration removes the UIs one by one. The refreshed UI, the
+        // one the store points at, goes first and loses its session.
+        session.removeUI(anotherUI);
+
+        // Detaching the UI that carries the store's detach listener must not
+        // fail, and must destroy the bean since the browser window is left
+        // without a UI.
+        UI.setCurrent(ui);
+        session.removeUI(ui);
+
+        Assert.assertEquals(List.of(), reportedErrors);
+        Assert.assertEquals(1, count.get());
+    }
+
+    @Test
+    public void detachUI_refreshedUIDetachedLast_beanIsDestroyedWhenRefreshedUIIsDetached() {
+        UI ui = mockUI();
+
+        UI anotherUI = makeAnotherUI(ui);
+
+        ExtendedClientDetails details = Mockito
+                .mock(ExtendedClientDetails.class);
+        Mockito.when(details.getWindowName()).thenReturn("bar");
+        ui.getInternals().setExtendedClientDetails(details);
+        anotherUI.getInternals().setExtendedClientDetails(details);
+
+        VaadinSession session = ui.getSession();
+        session.addUI(ui);
+        session.addUI(anotherUI);
+
+        mockServletContext(ui);
+
+        VaadinRouteScope scope = initScope(ui);
+
+        AtomicInteger count = new AtomicInteger();
+        scope.registerDestructionCallback("foo", () -> count.getAndIncrement());
+
+        navigateTo(ui, new NavigationTarget());
+
+        putObjectIntoScope(scope);
+
+        // Refresh: a request on the new UI re-points the store to it, while the
+        // store's detach listener stays on the first UI.
+        UI.setCurrent(anotherUI);
+        initScope(anotherUI);
+
+        // The stale UI is closed later on, e.g. by the inactive UI cleanup.
+        UI.setCurrent(ui);
+        session.removeUI(ui);
+
+        // the bean is not removed since the refreshed UI still hosts it
+        Assert.assertEquals(0, count.get());
+
+        // Detaching the refreshed UI must destroy the bean.
+        UI.setCurrent(anotherUI);
+        session.removeUI(anotherUI);
+
+        Assert.assertEquals(1, count.get());
+    }
+
     private void navigateTo(UI ui, Component component) {
         AfterNavigationEvent event = Mockito.mock(AfterNavigationEvent.class);
         Mockito.when(event.getActiveChain())