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

This PR cherry-picks changes from the original PR #25069 to branch 24.9.
---
#### Original PR description
> This PR cherry-picks changes from the original PR #25064 to branch
24.10.
> ---
> #### Original PR description
> > 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

Co-authored-by: Marco Collovati <marco@vaadin.com>

Author: vaadin-bot

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())