Cleanup UI even if detach fails (#16651)

Change-Id: Iefc27b7047836243268970605ba2e946fac55e95

3 files changed, 115 insertions, 1 deletions

diff --git a/server/src/com/vaadin/ui/UI.java b/server/src/com/vaadin/ui/UI.java
index 66f893e04a..8dd600ddd0 100644
--- a/server/src/com/vaadin/ui/UI.java
+++ b/server/src/com/vaadin/ui/UI.java
@@ -425,7 +425,12 @@ public abstract class UI extends AbstractSingleComponentContainer implements
                             + ".");
         } else {
             if (session == null) {
-                detach();
+                try {
+                    detach();
+                } catch (Exception e) {
+                    getLogger().log(Level.WARNING,
+                            "Error while detaching UI from session", e);
+                }
                 // Disable push when the UI is detached. Otherwise the
                 // push connection and possibly VaadinSession will live on.
                 getPushConfiguration().setPushMode(PushMode.DISABLED);
diff --git a/uitest/src/com/vaadin/tests/applicationcontext/CleanupBrokenUI.java b/uitest/src/com/vaadin/tests/applicationcontext/CleanupBrokenUI.java
new file mode 100644
index 0000000000..4af4f23bfa
--- /dev/null
+++ b/uitest/src/com/vaadin/tests/applicationcontext/CleanupBrokenUI.java
@@ -0,0 +1,65 @@
+/*
+ * Copyright 2000-2014 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.tests.applicationcontext;
+
+import com.vaadin.server.VaadinRequest;
+import com.vaadin.tests.components.AbstractTestUIWithLog;
+import com.vaadin.ui.Button;
+import com.vaadin.ui.Button.ClickEvent;
+import com.vaadin.ui.Label;
+
+/**
+ * Tests that UI is cleaned from session despite any errors that happen in
+ * detach.
+ * 
+ * @author Vaadin Ltd
+ */
+public class CleanupBrokenUI extends AbstractTestUIWithLog {
+
+    @Override
+    protected void setup(VaadinRequest request) {
+        logUIs();
+        addComponent(new Label("Label with broken detach") {
+            @Override
+            public void detach() {
+                throw new IllegalStateException(
+                        "Detach does not work for this component");
+            }
+        });
+
+        addComponent(new Button("Ping", new Button.ClickListener() {
+            @Override
+            public void buttonClick(ClickEvent event) {
+                log("pong");
+            }
+        }));
+
+    }
+
+    private void logUIs() {
+        log("UIs in session: " + getSession().getUIs().size());
+    }
+
+    @Override
+    protected String getTestDescription() {
+        return "Open the page as http://localhost:8888/run/CleanupBrokenUI, then refresh the page to get a new UI. On refresh there should be an IllegalStateException in the server log but pressing 'ping' after this should log no further messages and the old ui should no longer be in the VaadinSession";
+    }
+
+    @Override
+    protected Integer getTicketNumber() {
+        return 16651;
+    }
+}
diff --git a/uitest/src/com/vaadin/tests/applicationcontext/CleanupBrokenUITest.java b/uitest/src/com/vaadin/tests/applicationcontext/CleanupBrokenUITest.java
new file mode 100644
index 0000000000..404e05eefc
--- /dev/null
+++ b/uitest/src/com/vaadin/tests/applicationcontext/CleanupBrokenUITest.java
@@ -0,0 +1,44 @@
+/*
+ * Copyright 2000-2014 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.tests.applicationcontext;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+import com.vaadin.tests.tb3.SingleBrowserTest;
+
+public class CleanupBrokenUITest extends SingleBrowserTest {
+
+    @Test
+    public void ensureUIDetached() {
+        openTestURL();
+        // UI 1 has not yet been added in UI.init where logging takes place
+        Assert.assertEquals("1. UIs in session: 0", getLogRow(0));
+
+        String url = getTestURL(getUIClass())
+                .replace("restartApplication", "1");
+        driver.get(url);
+        // UI 1 remains in session during UI2 init where logging takes place
+        Assert.assertEquals("1. UIs in session: 1", getLogRow(0));
+
+        // At this point UI1 should be removed from the session
+        driver.get(url);
+
+        // UI 2 remains in session during UI3 init where logging takes place
+        // UI 1 should have been removed
+        Assert.assertEquals("1. UIs in session: 1", getLogRow(0));
+    }
+}