Skip to content

Commit c0d6e07

Browse files
mcollovatimcollovati
authored
fix: clear preseved chain cache for inactive UI (#19360) (#19475)
Prevents potential memory leaks caused by UIs being retained by the preserve on refresh cache when the browser page is closed.
1 parent 81940be commit c0d6e07

File tree

9 files changed

+580
-1
lines changed

9 files changed

+580
-1
lines changed

‎flow-server/src/main/java/com/vaadin/flow/router/internal/AbstractNavigationStateRenderer.java

Lines changed: 44 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,11 +25,14 @@
2525
import java.util.HashMap;
2626
import java.util.HashSet;
2727
import java.util.List;
28+
import java.util.Map;
2829
import java.util.Objects;
2930
import java.util.Optional;
3031
import java.util.Set;
31-
import java.util.stream.Collectors;
3232
import java.util.function.Supplier;
33+
import java.util.stream.Collectors;
34+
35+
import org.slf4j.LoggerFactory;
3336

3437
import com.vaadin.flow.component.Component;
3538
import com.vaadin.flow.component.HasElement;
@@ -40,6 +43,7 @@
4043
import com.vaadin.flow.function.DeploymentConfiguration;
4144
import com.vaadin.flow.internal.Pair;
4245
import com.vaadin.flow.internal.ReflectTools;
46+
import com.vaadin.flow.internal.StateNode;
4347
import com.vaadin.flow.router.AfterNavigationEvent;
4448
import com.vaadin.flow.router.BeforeEnterEvent;
4549
import com.vaadin.flow.router.BeforeEnterObserver;
@@ -1007,6 +1011,45 @@ private static void clearAllPreservedChains(UI ui) {
10071011
}
10081012
}
10091013

1014+
/**
1015+
* Removes preserved component cache for an inactive UI.
1016+
*
1017+
* @param inactiveUI
1018+
* the inactive UI
1019+
* @throws IllegalStateException
1020+
* if the UI is not in closing state
1021+
*/
1022+
public static void purgeInactiveUIPreservedChainCache(UI inactiveUI) {
1023+
if (!inactiveUI.isClosing()) {
1024+
throw new IllegalStateException(
1025+
"Cannot purge preserved chain cache for an active UI");
1026+
}
1027+
final VaadinSession session = inactiveUI.getSession();
1028+
final PreservedComponentCache cache = session
1029+
.getAttribute(PreservedComponentCache.class);
1030+
if (cache != null && !cache.isEmpty()) {
1031+
StateNode uiNode = inactiveUI.getElement().getNode();
1032+
Set<String> inactiveWindows = cache.entrySet().stream()
1033+
.filter(e -> {
1034+
ArrayList<HasElement> chain = e.getValue().getSecond();
1035+
// chain is never empty
1036+
StateNode chainNode = chain.get(0).getElement()
1037+
.getNode();
1038+
while (chainNode.getParent() != null) {
1039+
chainNode = chainNode.getParent();
1040+
}
1041+
return uiNode == chainNode;
1042+
}).map(Map.Entry::getKey).collect(Collectors.toSet());
1043+
if (!inactiveWindows.isEmpty()) {
1044+
LoggerFactory.getLogger(AbstractNavigationStateRenderer.class)
1045+
.debug("Removing preserved chain cache for inactive UI {} on VaadinSession {} (windows: {})",
1046+
inactiveUI.getUIId(),
1047+
session.getSession().getId(), inactiveWindows);
1048+
}
1049+
inactiveWindows.forEach(cache::remove);
1050+
}
1051+
}
1052+
10101053
private static void warnAboutPreserveOnRefreshAndLiveReloadCombo(UI ui) {
10111054
// Show a warning that live-reload may work counter-intuitively
10121055
DeploymentConfiguration configuration = ui.getSession()

‎flow-server/src/main/java/com/vaadin/flow/server/VaadinService.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@
5353
import java.util.stream.Stream;
5454
import java.util.stream.StreamSupport;
5555

56+
import com.vaadin.flow.router.internal.AbstractNavigationStateRenderer;
5657
import org.slf4j.Logger;
5758
import org.slf4j.LoggerFactory;
5859

@@ -1370,6 +1371,8 @@ private void closeInactiveUIs(VaadinSession session) {
13701371
getLogger().debug("Closing inactive UI #{} in session {}",
13711372
ui.getUIId(), sessionId);
13721373
ui.close();
1374+
AbstractNavigationStateRenderer
1375+
.purgeInactiveUIPreservedChainCache(ui);
13731376
});
13741377
}
13751378
}

‎flow-server/src/test/java/com/vaadin/flow/router/internal/NavigationStateRendererTest.java

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import java.util.Arrays;
2323
import java.util.Collections;
2424
import java.util.List;
25+
import java.util.Optional;
2526
import java.util.Set;
2627
import java.util.concurrent.atomic.AtomicBoolean;
2728
import java.util.concurrent.atomic.AtomicInteger;
@@ -69,6 +70,7 @@
6970
import com.vaadin.flow.server.RouteRegistry;
7071
import com.vaadin.flow.server.ServiceException;
7172
import com.vaadin.flow.server.VaadinServletContext;
73+
import com.vaadin.flow.server.WrappedSession;
7274
import com.vaadin.flow.server.startup.ApplicationRouteRegistry;
7375
import com.vaadin.tests.util.AlwaysLockedVaadinSession;
7476
import com.vaadin.tests.util.MockDeploymentConfiguration;
@@ -745,4 +747,66 @@ public Page getPage() {
745747
"No pushState invocation is expected when navigating to the current location.",
746748
pushStateCalled.get());
747749
}
750+
751+
@Test
752+
public void purgeInactiveUIPreservedChainCache_activeUI_throws() {
753+
MockVaadinServletService service = createMockServiceWithInstantiator();
754+
MockVaadinSession session = new AlwaysLockedVaadinSession(service);
755+
756+
MockUI activeUI = new MockUI(session);
757+
Component attachedToActiveUI = new PreservedView();
758+
activeUI.add(attachedToActiveUI);
759+
760+
Assert.assertThrows(IllegalStateException.class,
761+
() -> AbstractNavigationStateRenderer
762+
.purgeInactiveUIPreservedChainCache(activeUI));
763+
764+
}
765+
766+
@Test
767+
public void purgeInactiveUIPreservedChainCache_inactiveUI_clearsCache() {
768+
MockVaadinServletService service = createMockServiceWithInstantiator();
769+
WrappedSession wrappedSession = Mockito.mock(WrappedSession.class);
770+
Mockito.when(wrappedSession.getId()).thenReturn("A-SESSION-ID");
771+
MockVaadinSession session = new AlwaysLockedVaadinSession(service) {
772+
@Override
773+
public WrappedSession getSession() {
774+
return wrappedSession;
775+
}
776+
};
777+
778+
MockUI activeUI = new MockUI(session);
779+
Component attachedToActiveUI = new PreservedView();
780+
activeUI.add(attachedToActiveUI);
781+
782+
MockUI inActiveUI = new MockUI(session);
783+
Component attachedToInactiveUI = new PreservedView();
784+
inActiveUI.add(attachedToInactiveUI);
785+
inActiveUI.close();
786+
787+
// Simulate two tabs on the same view, but one has been closed
788+
Location location = new Location("preserved");
789+
AbstractNavigationStateRenderer.setPreservedChain(session, "ACTIVE",
790+
location,
791+
new ArrayList<>(Collections.singletonList(attachedToActiveUI)));
792+
AbstractNavigationStateRenderer.setPreservedChain(session, "INACTIVE",
793+
location, new ArrayList<>(
794+
Collections.singletonList(attachedToInactiveUI)));
795+
796+
AbstractNavigationStateRenderer
797+
.purgeInactiveUIPreservedChainCache(inActiveUI);
798+
799+
Optional<ArrayList<HasElement>> active = AbstractNavigationStateRenderer
800+
.getPreservedChain(session, "ACTIVE", location);
801+
Assert.assertTrue(
802+
"Expected preserved chain for active window to be present",
803+
active.isPresent());
804+
805+
Optional<ArrayList<HasElement>> inactive = AbstractNavigationStateRenderer
806+
.getPreservedChain(session, "INACTIVE", location);
807+
Assert.assertFalse(
808+
"Expected preserved chain for inactive window to be removed",
809+
inactive.isPresent());
810+
811+
}
748812
}

‎flow-tests/test-misc/src/main/java/com/vaadin/flow/misc/ui/ActiveUIsView.java

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
/*
2+
* Copyright 2000-2024 Vaadin Ltd.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License"); you may not
5+
* use this file except in compliance with the License. You may obtain a copy of
6+
* the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
12+
* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
13+
* License for the specific language governing permissions and limitations under
14+
* the License.
15+
*/
16+
17+
package com.vaadin.flow.misc.ui;
18+
19+
import com.vaadin.flow.component.ComponentUtil;
20+
import com.vaadin.flow.component.UI;
21+
import com.vaadin.flow.component.html.Div;
22+
import com.vaadin.flow.component.html.H1;
23+
import com.vaadin.flow.component.html.NativeButton;
24+
import com.vaadin.flow.router.Route;
25+
26+
@Route("active-uis")
27+
public class ActiveUIsView extends Div {
28+
29+
public ActiveUIsView() {
30+
Div uis = new Div();
31+
uis.setId("uis");
32+
NativeButton listUIsButton = new NativeButton("List active UIs",
33+
event -> {
34+
UI current = UI.getCurrent();
35+
listUIs(current, uis);
36+
});
37+
listUIsButton.setId("list-uis");
38+
39+
Div gcCollectedUIs = new Div();
40+
gcCollectedUIs.setId("gcuis");
41+
NativeButton listGCCollectedUIsButton = new NativeButton(
42+
"List GC collected UIs", event -> {
43+
listGCCollectedUIs(gcCollectedUIs);
44+
});
45+
listGCCollectedUIsButton.setId("list-gc-collected-uis");
46+
NativeButton gcHintButton = new NativeButton("Run GC",
47+
event -> System.gc());
48+
gcHintButton.setId("gc-hint");
49+
50+
add(listUIsButton, new H1("Active UIs (excluding current)"), uis,
51+
listGCCollectedUIsButton, gcHintButton,
52+
new H1("GC collected UIs"), gcCollectedUIs);
53+
}
54+
55+
private void listGCCollectedUIs(Div gcCollectedUIs) {
56+
gcCollectedUIs.removeAll();
57+
UI ui = UI.getCurrent();
58+
ComponentUtil.getData(ui, UITrackerListener.UITracker.class)
59+
.getCollectedUIs(ui.getSession()).forEach(uiId -> gcCollectedUIs
60+
.add(makeDiv("GC Collected UI: " + uiId)));
61+
}
62+
63+
private void listUIs(UI currentUI, Div uis) {
64+
uis.removeAll();
65+
currentUI.getSession().getUIs().stream().filter(ui -> ui != currentUI)
66+
.map(ui -> makeDiv("UI: " + ui.getUIId() + ", Path: "
67+
+ ui.getInternals().getActiveViewLocation().getPath()))
68+
.forEach(uis::add);
69+
}
70+
71+
private static Div makeDiv(String text) {
72+
Div div = new Div();
73+
div.setText(text);
74+
return div;
75+
}
76+
}

‎flow-tests/test-misc/src/main/java/com/vaadin/flow/misc/ui/CustomServlet.java

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
/*
2+
* Copyright 2000-2024 Vaadin Ltd.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License"); you may not
5+
* use this file except in compliance with the License. You may obtain a copy of
6+
* the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
12+
* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
13+
* License for the specific language governing permissions and limitations under
14+
* the License.
15+
*/
16+
17+
package com.vaadin.flow.misc.ui;
18+
19+
import javax.servlet.annotation.WebInitParam;
20+
import javax.servlet.annotation.WebServlet;
21+
22+
import com.vaadin.flow.server.VaadinServlet;
23+
24+
@WebServlet(urlPatterns = "/*", asyncSupported = true, initParams = {
25+
@WebInitParam(name = "heartbeatInterval", value = "5") })
26+
public class CustomServlet extends VaadinServlet {
27+
public static long HEARTBEAT_INTERVAL = 5;
28+
}

‎flow-tests/test-misc/src/main/java/com/vaadin/flow/misc/ui/PreserveOnRefreshView.java

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
/*
2+
* Copyright 2000-2024 Vaadin Ltd.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License"); you may not
5+
* use this file except in compliance with the License. You may obtain a copy of
6+
* the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
12+
* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
13+
* License for the specific language governing permissions and limitations under
14+
* the License.
15+
*/
16+
17+
package com.vaadin.flow.misc.ui;
18+
19+
import com.vaadin.flow.component.UI;
20+
import com.vaadin.flow.component.html.Div;
21+
import com.vaadin.flow.component.html.H1;
22+
import com.vaadin.flow.component.html.H3;
23+
import com.vaadin.flow.component.html.NativeButton;
24+
import com.vaadin.flow.router.AfterNavigationEvent;
25+
import com.vaadin.flow.router.AfterNavigationObserver;
26+
import com.vaadin.flow.router.PreserveOnRefresh;
27+
import com.vaadin.flow.router.Route;
28+
29+
@PreserveOnRefresh
30+
@Route("preserve")
31+
public class PreserveOnRefreshView extends Div
32+
implements AfterNavigationObserver {
33+
34+
private final Div uiId;
35+
36+
public PreserveOnRefreshView() {
37+
uiId = new Div();
38+
uiId.setId("uiId");
39+
NativeButton reloadButton = new NativeButton("Reload page",
40+
ev -> UI.getCurrent().getPage().reload());
41+
reloadButton.setId("reload");
42+
add(new H1("This view is preserved on refresh"));
43+
H3 initialUIId = new H3("Initial UI: " + UI.getCurrent().getUIId());
44+
initialUIId.setId("initialUIId");
45+
add(initialUIId);
46+
add(uiId, reloadButton);
47+
}
48+
49+
@Override
50+
public void afterNavigation(AfterNavigationEvent event) {
51+
uiId.setText("UI: " + UI.getCurrent().getUIId());
52+
}
53+
}

0 commit comments

Comments
 (0)