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/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) {

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();
+
+    }
 }

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

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

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;
+    }
+}

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: ", ""));
+    }
+}