Skip to content

Commit

Permalink
fix: clear preseved chain cache for inactive UI (#19360) (#19384)
Browse files Browse the repository at this point in the history
Prevents potential memory leaks caused by UIs being retained by the
preserve on refresh cache when the browser page is closed.
  • Loading branch information
mcollovati committed May 15, 2024
1 parent 1c25fcf commit 61892d5
Show file tree
Hide file tree
Showing 10 changed files with 595 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,14 @@
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
import java.util.Set;
import java.util.function.Supplier;
import java.util.stream.Collectors;

import org.slf4j.LoggerFactory;

import com.vaadin.flow.component.Component;
import com.vaadin.flow.component.HasElement;
Expand All @@ -36,6 +40,7 @@
import com.vaadin.flow.dom.Element;
import com.vaadin.flow.function.DeploymentConfiguration;
import com.vaadin.flow.internal.Pair;
import com.vaadin.flow.internal.StateNode;
import com.vaadin.flow.router.AfterNavigationEvent;
import com.vaadin.flow.router.BeforeEnterEvent;
import com.vaadin.flow.router.BeforeEnterObserver;
Expand Down Expand Up @@ -993,6 +998,45 @@ private static void clearAllPreservedChains(UI ui) {
}
}

/**
* Removes preserved component cache for an inactive UI.
*
* @param inactiveUI
* the inactive UI
* @throws IllegalStateException
* if the UI is not in closing state
*/
public static void purgeInactiveUIPreservedChainCache(UI inactiveUI) {
if (!inactiveUI.isClosing()) {
throw new IllegalStateException(
"Cannot purge preserved chain cache for an active UI");
}
final VaadinSession session = inactiveUI.getSession();
final PreservedComponentCache cache = session
.getAttribute(PreservedComponentCache.class);
if (cache != null && !cache.isEmpty()) {
StateNode uiNode = inactiveUI.getElement().getNode();
Set<String> inactiveWindows = cache.entrySet().stream()
.filter(e -> {
ArrayList<HasElement> chain = e.getValue().getSecond();
// chain is never empty
StateNode chainNode = chain.get(0).getElement()
.getNode();
while (chainNode.getParent() != null) {
chainNode = chainNode.getParent();
}
return uiNode == chainNode;
}).map(Map.Entry::getKey).collect(Collectors.toSet());
if (!inactiveWindows.isEmpty()) {
LoggerFactory.getLogger(AbstractNavigationStateRenderer.class)
.debug("Removing preserved chain cache for inactive UI {} on VaadinSession {} (windows: {})",
inactiveUI.getUIId(),
session.getSession().getId(), inactiveWindows);
}
inactiveWindows.forEach(cache::remove);
}
}

private static void warnAboutPreserveOnRefreshAndLiveReloadCombo(UI ui) {
// Show a warning that live-reload may work counter-intuitively
DeploymentConfiguration configuration = ui.getSession()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@
import com.vaadin.flow.internal.UsageStatistics;
import com.vaadin.flow.router.RouteData;
import com.vaadin.flow.router.Router;
import com.vaadin.flow.router.internal.AbstractNavigationStateRenderer;
import com.vaadin.flow.server.HandlerHelper.RequestType;
import com.vaadin.flow.server.communication.AtmospherePushConnection;
import com.vaadin.flow.server.communication.HeartbeatHandler;
Expand Down Expand Up @@ -1347,6 +1348,8 @@ private void closeInactiveUIs(VaadinSession session) {
getLogger().debug("Closing inactive UI #{} in session {}",
ui.getUIId(), sessionId);
ui.close();
AbstractNavigationStateRenderer
.purgeInactiveUIPreservedChainCache(ui);
});
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import java.util.Optional;
import java.util.Set;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicInteger;
Expand Down Expand Up @@ -67,6 +68,7 @@
import com.vaadin.flow.server.MockVaadinSession;
import com.vaadin.flow.server.RouteRegistry;
import com.vaadin.flow.server.ServiceException;
import com.vaadin.flow.server.WrappedSession;
import com.vaadin.flow.server.startup.ApplicationRouteRegistry;
import com.vaadin.tests.util.AlwaysLockedVaadinSession;
import com.vaadin.tests.util.MockDeploymentConfiguration;
Expand Down Expand Up @@ -743,4 +745,66 @@ public Page getPage() {
"No pushState invocation is expected when navigating to the current location.",
pushStateCalled.get());
}

@Test
public void purgeInactiveUIPreservedChainCache_activeUI_throws() {
MockVaadinServletService service = createMockServiceWithInstantiator();
MockVaadinSession session = new AlwaysLockedVaadinSession(service);

MockUI activeUI = new MockUI(session);
Component attachedToActiveUI = new PreservedView();
activeUI.add(attachedToActiveUI);

Assert.assertThrows(IllegalStateException.class,
() -> AbstractNavigationStateRenderer
.purgeInactiveUIPreservedChainCache(activeUI));

}

@Test
public void purgeInactiveUIPreservedChainCache_inactiveUI_clearsCache() {
MockVaadinServletService service = createMockServiceWithInstantiator();
WrappedSession wrappedSession = Mockito.mock(WrappedSession.class);
Mockito.when(wrappedSession.getId()).thenReturn("A-SESSION-ID");
MockVaadinSession session = new AlwaysLockedVaadinSession(service) {
@Override
public WrappedSession getSession() {
return wrappedSession;
}
};

MockUI activeUI = new MockUI(session);
Component attachedToActiveUI = new PreservedView();
activeUI.add(attachedToActiveUI);

MockUI inActiveUI = new MockUI(session);
Component attachedToInactiveUI = new PreservedView();
inActiveUI.add(attachedToInactiveUI);
inActiveUI.close();

// Simulate two tabs on the same view, but one has been closed
Location location = new Location("preserved");
AbstractNavigationStateRenderer.setPreservedChain(session, "ACTIVE",
location,
new ArrayList<>(Collections.singletonList(attachedToActiveUI)));
AbstractNavigationStateRenderer.setPreservedChain(session, "INACTIVE",
location, new ArrayList<>(
Collections.singletonList(attachedToInactiveUI)));

AbstractNavigationStateRenderer
.purgeInactiveUIPreservedChainCache(inActiveUI);

Optional<ArrayList<HasElement>> active = AbstractNavigationStateRenderer
.getPreservedChain(session, "ACTIVE", location);
Assert.assertTrue(
"Expected preserved chain for active window to be present",
active.isPresent());

Optional<ArrayList<HasElement>> inactive = AbstractNavigationStateRenderer
.getPreservedChain(session, "INACTIVE", location);
Assert.assertFalse(
"Expected preserved chain for inactive window to be removed",
inactive.isPresent());

}
}
6 changes: 6 additions & 0 deletions flow-tests/test-misc/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,12 @@
<artifactId>flow-test-resources</artifactId>
<version>${project.version}</version>
</dependency>
<dependency>
<groupId>com.vaadin</groupId>
<artifactId>flow-html-components-testbench</artifactId>
<version>${project.version}</version>
<scope>test</scope>
</dependency>
</dependencies>

<build>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
/*
* Copyright 2000-2024 Vaadin Ltd.
*
* Licensed under the Apache License, Version 2.0 (the "License"); you may not
* use this file except in compliance with the License. You may obtain a copy of
* the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
* License for the specific language governing permissions and limitations under
* the License.
*/

package com.vaadin.flow.misc.ui;

import com.vaadin.flow.component.ComponentUtil;
import com.vaadin.flow.component.UI;
import com.vaadin.flow.component.html.Div;
import com.vaadin.flow.component.html.H1;
import com.vaadin.flow.component.html.NativeButton;
import com.vaadin.flow.router.Route;

@Route("active-uis")
public class ActiveUIsView extends Div {

public ActiveUIsView() {
Div uis = new Div();
uis.setId("uis");
NativeButton listUIsButton = new NativeButton("List active UIs",
event -> {
UI current = UI.getCurrent();
listUIs(current, uis);
});
listUIsButton.setId("list-uis");

Div gcCollectedUIs = new Div();
gcCollectedUIs.setId("gcuis");
NativeButton listGCCollectedUIsButton = new NativeButton(
"List GC collected UIs", event -> {
listGCCollectedUIs(gcCollectedUIs);
});
listGCCollectedUIsButton.setId("list-gc-collected-uis");
NativeButton gcHintButton = new NativeButton("Run GC",
event -> System.gc());
gcHintButton.setId("gc-hint");

add(listUIsButton, new H1("Active UIs (excluding current)"), uis,
listGCCollectedUIsButton, gcHintButton,
new H1("GC collected UIs"), gcCollectedUIs);
}

private void listGCCollectedUIs(Div gcCollectedUIs) {
gcCollectedUIs.removeAll();
UI ui = UI.getCurrent();
ComponentUtil.getData(ui, UITrackerListener.UITracker.class)
.getCollectedUIs(ui.getSession()).forEach(uiId -> {
Div div = new Div();
div.setText("GC Collected UI: " + uiId);
gcCollectedUIs.add(div);
});
}

private void listUIs(UI currentUI, Div uis) {
uis.removeAll();
currentUI.getSession().getUIs().stream().filter(ui -> ui != currentUI)
.map(ui -> {
Div div = new Div();
div.setText("UI: " + ui.getUIId() + ", Path: " + ui
.getInternals().getActiveViewLocation().getPath());
return div;
}).forEach(uis::add);
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
/*
* Copyright 2000-2024 Vaadin Ltd.
*
* Licensed under the Apache License, Version 2.0 (the "License"); you may not
* use this file except in compliance with the License. You may obtain a copy of
* the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
* License for the specific language governing permissions and limitations under
* the License.
*/

package com.vaadin.flow.misc.ui;

import javax.servlet.annotation.WebInitParam;
import javax.servlet.annotation.WebServlet;

import com.vaadin.flow.server.VaadinServlet;

@WebServlet(urlPatterns = "/*", asyncSupported = true, initParams = {
@WebInitParam(name = "heartbeatInterval", value = "5") })
public class CustomServlet extends VaadinServlet {
public static long HEARTBEAT_INTERVAL = 5;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
/*
* Copyright 2000-2024 Vaadin Ltd.
*
* Licensed under the Apache License, Version 2.0 (the "License"); you may not
* use this file except in compliance with the License. You may obtain a copy of
* the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
* License for the specific language governing permissions and limitations under
* the License.
*/

package com.vaadin.flow.misc.ui;

import com.vaadin.flow.component.UI;
import com.vaadin.flow.component.html.Div;
import com.vaadin.flow.component.html.H1;
import com.vaadin.flow.component.html.H3;
import com.vaadin.flow.component.html.NativeButton;
import com.vaadin.flow.router.AfterNavigationEvent;
import com.vaadin.flow.router.AfterNavigationObserver;
import com.vaadin.flow.router.PreserveOnRefresh;
import com.vaadin.flow.router.Route;

@PreserveOnRefresh
@Route("preserve")
public class PreserveOnRefreshView extends Div
implements AfterNavigationObserver {

private final Div uiId;

public PreserveOnRefreshView() {

uiId = new Div();
uiId.setId("uiId");
NativeButton reloadButton = new NativeButton("Reload page",
ev -> UI.getCurrent().getPage().reload());
reloadButton.setId("reload");
add(new H1("This view is preserved on refresh"));
add(new H3("Initial UI: " + UI.getCurrent().getUIId()));
add(uiId, reloadButton);
}

@Override
public void afterNavigation(AfterNavigationEvent event) {
uiId.setText("UI: " + UI.getCurrent().getUIId());
}
}

0 comments on commit 61892d5

Please sign in to comment.