From 5b18c5469e40529fad0d3d014ab4237a56616d81 Mon Sep 17 00:00:00 2001 From: Artur Signell Date: Wed, 25 May 2016 21:37:56 +0300 Subject: [PATCH] Send an ack message after push has removed connectors (#19822) The server side needs to know the client has removed the connectors to be able to do cleanup Change-Id: Ic3d41cc5cbab035a53bf5c99496d74858c376e73 --- .../client/communication/MessageHandler.java | 19 ++-- .../com/vaadin/client/ui/ui/UIConnector.java | 10 ++ server/src/main/java/com/vaadin/ui/UI.java | 5 + .../com/vaadin/shared/ui/ui/UIServerRpc.java | 3 + .../tests/push/PushRemoveConnectors.java | 92 +++++++++++++++++++ .../tests/push/PushRemoveConnectorsTest.java | 36 ++++++++ 6 files changed, 158 insertions(+), 7 deletions(-) create mode 100644 uitest/src/main/java/com/vaadin/tests/push/PushRemoveConnectors.java create mode 100644 uitest/src/test/java/com/vaadin/tests/push/PushRemoveConnectorsTest.java diff --git a/client/src/main/java/com/vaadin/client/communication/MessageHandler.java b/client/src/main/java/com/vaadin/client/communication/MessageHandler.java index 3ffadeb9ce..15e94ca8d9 100644 --- a/client/src/main/java/com/vaadin/client/communication/MessageHandler.java +++ b/client/src/main/java/com/vaadin/client/communication/MessageHandler.java @@ -493,8 +493,12 @@ public class MessageHandler { json.getValueMap("dd")); } - unregisterRemovedConnectors(connectorHierarchyUpdateResult.detachedConnectorIds); - + int removed = unregisterRemovedConnectors(connectorHierarchyUpdateResult.detachedConnectorIds); + if (removed > 0 && !isResponse(json)) { + // Must acknowledge the removal using an XHR or server + // memory usage will keep growing + getUIConnector().sendAck(); + } getLogger() .info("handleUIDLMessage: " + (Duration.currentTimeMillis() - processUidlStart) @@ -802,12 +806,13 @@ public class MessageHandler { Profiler.leave("verifyConnectorHierarchy - this is only performed in debug mode"); } - private void unregisterRemovedConnectors( + private int unregisterRemovedConnectors( FastStringSet detachedConnectors) { Profiler.enter("unregisterRemovedConnectors"); JsArrayString detachedArray = detachedConnectors.dump(); - for (int i = 0; i < detachedArray.length(); i++) { + int nrDetached = detachedArray.length(); + for (int i = 0; i < nrDetached; i++) { ServerConnector connector = getConnectorMap().getConnector( detachedArray.get(i)); @@ -822,10 +827,10 @@ public class MessageHandler { verifyConnectorHierarchy(); } - getLogger().info( - "* Unregistered " + detachedArray.length() - + " connectors"); + getLogger() + .info("* Unregistered " + nrDetached + " connectors"); Profiler.leave("unregisterRemovedConnectors"); + return nrDetached; } private JsArrayString createConnectorsIfNeeded(ValueMap json) { diff --git a/client/src/main/java/com/vaadin/client/ui/ui/UIConnector.java b/client/src/main/java/com/vaadin/client/ui/ui/UIConnector.java index 6f4872729d..ca9cb879af 100644 --- a/client/src/main/java/com/vaadin/client/ui/ui/UIConnector.java +++ b/client/src/main/java/com/vaadin/client/ui/ui/UIConnector.java @@ -1131,4 +1131,14 @@ public class UIConnector extends AbstractSingleComponentContainerConnector private static Logger getLogger() { return Logger.getLogger(UIConnector.class.getName()); } + + /** + * Send an acknowledgement RPC to the server. This allows the server to know + * which messages the client has received, even when the client is not + * sending any other traffic. + */ + public void sendAck() { + getRpcProxy(UIServerRpc.class).acknowledge(); + + } } diff --git a/server/src/main/java/com/vaadin/ui/UI.java b/server/src/main/java/com/vaadin/ui/UI.java index c429b64491..0a0cd46816 100644 --- a/server/src/main/java/com/vaadin/ui/UI.java +++ b/server/src/main/java/com/vaadin/ui/UI.java @@ -179,6 +179,11 @@ public abstract class UI extends AbstractSingleComponentContainer implements public void poll() { fireEvent(new PollEvent(UI.this)); } + + @Override + public void acknowledge() { + // Nothing to do, just need the message to be sent and processed + } }; private DebugWindowServerRpc debugRpc = new DebugWindowServerRpc() { @Override diff --git a/shared/src/main/java/com/vaadin/shared/ui/ui/UIServerRpc.java b/shared/src/main/java/com/vaadin/shared/ui/ui/UIServerRpc.java index 887ea760b3..554feaac76 100644 --- a/shared/src/main/java/com/vaadin/shared/ui/ui/UIServerRpc.java +++ b/shared/src/main/java/com/vaadin/shared/ui/ui/UIServerRpc.java @@ -35,4 +35,7 @@ public interface UIServerRpc extends ClickRpc, ServerRpc { * should always be called to ensure the message is flushed right away. */ public void poll(); + + @NoLoadingIndicator + public void acknowledge(); } diff --git a/uitest/src/main/java/com/vaadin/tests/push/PushRemoveConnectors.java b/uitest/src/main/java/com/vaadin/tests/push/PushRemoveConnectors.java new file mode 100644 index 0000000000..528492c33a --- /dev/null +++ b/uitest/src/main/java/com/vaadin/tests/push/PushRemoveConnectors.java @@ -0,0 +1,92 @@ +package com.vaadin.tests.push; + +import java.util.concurrent.Executors; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.ScheduledFuture; +import java.util.concurrent.TimeUnit; + +import org.apache.commons.lang.SerializationUtils; + +import com.vaadin.annotations.Push; +import com.vaadin.data.Property.ValueChangeEvent; +import com.vaadin.data.Property.ValueChangeListener; +import com.vaadin.server.VaadinRequest; +import com.vaadin.tests.components.AbstractTestUIWithLog; +import com.vaadin.ui.AbstractOrderedLayout; +import com.vaadin.ui.Button; +import com.vaadin.ui.Button.ClickEvent; +import com.vaadin.ui.Button.ClickListener; +import com.vaadin.ui.CheckBox; +import com.vaadin.ui.HorizontalLayout; +import com.vaadin.ui.Label; + +@Push +public class PushRemoveConnectors extends AbstractTestUIWithLog { + + private transient final ScheduledExecutorService threadPool = Executors + .newScheduledThreadPool(5); + static final String START = "start"; + static final String STOP = "stop"; + private AbstractOrderedLayout verticalLayout; + private transient ScheduledFuture task = null; + + @Override + protected void setup(VaadinRequest request) { + final CheckBox pollingEnabled = new CheckBox("Polling enabled"); + pollingEnabled.addValueChangeListener(new ValueChangeListener() { + @Override + public void valueChange(ValueChangeEvent event) { + setPollInterval(pollingEnabled.getValue() ? 1000 : -1); + } + }); + + Button start = new Button("start"); + start.setId(START); + start.addClickListener(new ClickListener() { + + @Override + public void buttonClick(ClickEvent event) { + task = threadPool.scheduleAtFixedRate(new Runnable() { + @Override + public void run() { + access(new Runnable() { + public void run() { + populate(); + log("Serialized session size: " + + getSessionSize()); + } + }); + } + }, 1, 1, TimeUnit.SECONDS); + } + }); + Button stop = new Button("stop"); + stop.setId(STOP); + stop.addClickListener(new ClickListener() { + @Override + public void buttonClick(ClickEvent event) { + if (task != null) { + task.cancel(true); + task = null; + } + + } + }); + verticalLayout = new HorizontalLayout(); + populate(); + addComponents(pollingEnabled, start, stop, verticalLayout); + } + + private void populate() { + verticalLayout.removeAllComponents(); + for (int i = 0; i < 500; i++) { + Label l = new Label("."); + l.setSizeUndefined(); + verticalLayout.addComponent(l); + } + } + + private int getSessionSize() { + return SerializationUtils.serialize(getSession()).length; + } +} diff --git a/uitest/src/test/java/com/vaadin/tests/push/PushRemoveConnectorsTest.java b/uitest/src/test/java/com/vaadin/tests/push/PushRemoveConnectorsTest.java new file mode 100644 index 0000000000..bcf93b0aa9 --- /dev/null +++ b/uitest/src/test/java/com/vaadin/tests/push/PushRemoveConnectorsTest.java @@ -0,0 +1,36 @@ +package com.vaadin.tests.push; + +import org.junit.Assert; +import org.junit.Test; + +import com.vaadin.testbench.elements.ButtonElement; +import com.vaadin.tests.tb3.SingleBrowserTest; + +public class PushRemoveConnectorsTest extends SingleBrowserTest { + + @Test + public void testNoMemoryLeak() throws InterruptedException { + openTestURL(); + $(ButtonElement.class).id(PushRemoveConnectors.START).click(); + Thread.sleep(5000); + int last = getMemoryUsage(); + int i = 0; + while (i++ < 10) { + Thread.sleep(5000); + int now = getMemoryUsage(); + System.out.println("Memory usage: "+now); + if (last == now) + break; + + last = now; + } + $(ButtonElement.class).id(PushRemoveConnectors.STOP).click(); + + Assert.assertNotEquals(10, i); + } + + private int getMemoryUsage() { + return Integer.parseInt(getLogRow(0).replaceFirst( + ".*Serialized session size: ", "")); + } +} -- 2.39.5