Workaround for deadlock issue (#18436)

Change-Id: I4e32550e3d3095c2c914bb93d260819414d2e6e6

server/src/main/java/com/vaadin/ui/UI.java

@@ -470,9 +470,23 @@ public abstract class UI extends AbstractSingleComponentContainer
                             "Error while detaching UI from session", e);
                 }
                 // Disable push when the UI is detached. Otherwise the
-                // push connection and possibly VaadinSession will live on.
+                // push connection and possibly VaadinSession will live
+                // on.
                 getPushConfiguration().setPushMode(PushMode.DISABLED);
-                setPushConnection(null);
+
+                new Thread(new Runnable() {
+                    @Override
+                    public void run() {
+                        // This intentionally does disconnect without locking
+                        // the VaadinSession to avoid deadlocks where the server
+                        // uses a lock for the websocket connection
+
+                        // See https://dev.vaadin.com/ticket/18436
+                        // The underlying problem is
+                        // https://dev.vaadin.com/ticket/16919
+                        setPushConnection(null);
+                    }
+                }).start();
             }
             this.session = session;
         }

server/src/test/java/com/vaadin/ui/UITest.java

@@ -0,0 +1,155 @@
+package com.vaadin.ui;
+
+import java.util.Properties;
+import java.util.concurrent.ConcurrentLinkedQueue;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.locks.Lock;
+import java.util.concurrent.locks.ReentrantLock;
+
+import javax.servlet.ServletConfig;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+import com.vaadin.server.DefaultDeploymentConfiguration;
+import com.vaadin.server.MockServletConfig;
+import com.vaadin.server.MockVaadinSession;
+import com.vaadin.server.VaadinRequest;
+import com.vaadin.server.VaadinServlet;
+import com.vaadin.server.VaadinServletService;
+import com.vaadin.server.VaadinSession;
+import com.vaadin.server.communication.PushConnection;
+import com.vaadin.shared.communication.PushMode;
+
+public class UITest {
+
+    @Test
+    public void removeFromSessionWithExternalLock() throws Exception {
+        // See https://dev.vaadin.com/ticket/18436
+        final UI ui = new UI() {
+
+            @Override
+            protected void init(VaadinRequest request) {
+            }
+
+        };
+        final Lock externalLock = new ReentrantLock();
+
+        ServletConfig servletConfig = new MockServletConfig();
+        VaadinServlet servlet = new VaadinServlet();
+        servlet.init(servletConfig);
+
+        DefaultDeploymentConfiguration deploymentConfiguration = new DefaultDeploymentConfiguration(
+                UI.class, new Properties());
+
+        MockVaadinSession session = new MockVaadinSession(
+                new VaadinServletService(servlet, deploymentConfiguration));
+        session.lock();
+        ui.setSession(session);
+        ui.getPushConfiguration().setPushMode(PushMode.MANUAL);
+        ui.setPushConnection(new PushConnection() {
+
+            private boolean connected = true;
+
+            @Override
+            public void push() {
+            }
+
+            @Override
+            public boolean isConnected() {
+                return connected;
+            }
+
+            @Override
+            public void disconnect() {
+                externalLock.lock();
+                try {
+                    connected = false;
+                } finally {
+                    externalLock.unlock();
+                }
+
+            }
+        });
+        session.unlock();
+
+        final CountDownLatch websocketReachedCheckpoint = new CountDownLatch(1);
+        final CountDownLatch uiDisconnectReachedCheckpoint = new CountDownLatch(
+                1);
+
+        final VaadinSession uiSession = ui.getSession();
+        final ConcurrentLinkedQueue<Exception> exceptions = new ConcurrentLinkedQueue<Exception>();
+
+        // Simulates the websocket close thread
+        Runnable websocketClose = new Runnable() {
+            @Override
+            public void run() {
+                externalLock.lock();
+                // Wait for disconnect thread to lock VaadinSession
+                websocketReachedCheckpoint.countDown();
+                try {
+                    uiDisconnectReachedCheckpoint.await();
+                } catch (InterruptedException e) {
+                    e.printStackTrace();
+                    exceptions.add(e);
+                    return;
+                }
+                uiSession.lock();
+                externalLock.unlock();
+            }
+        };
+
+        Runnable disconnectPushFromUI = new Runnable() {
+            @Override
+            public void run() {
+                uiSession.lock();
+                // Wait for websocket thread to lock external lock
+                uiDisconnectReachedCheckpoint.countDown();
+                try {
+                    websocketReachedCheckpoint.await();
+                } catch (InterruptedException e) {
+                    e.printStackTrace();
+                    exceptions.add(e);
+                    return;
+                }
+
+                ui.setSession(null);
+                uiSession.unlock();
+            }
+        };
+
+        Thread websocketThread = new Thread(websocketClose);
+        websocketThread.start();
+        Thread uiDisconnectThread = new Thread(disconnectPushFromUI);
+        uiDisconnectThread.start();
+
+        websocketThread.join(5000);
+        uiDisconnectThread.join(5000);
+
+        if (websocketThread.isAlive() || uiDisconnectThread.isAlive()) {
+            websocketThread.interrupt();
+            uiDisconnectThread.interrupt();
+            Assert.fail("Threads are still running");
+        }
+        if (!exceptions.isEmpty()) {
+            for (Exception e : exceptions) {
+                e.printStackTrace();
+            }
+            Assert.fail("There were exceptions in the threads");
+        }
+
+        Assert.assertNull(ui.getSession());
+
+        // PushConnection is set to null in another thread. We need to wait for
+        // that to happen
+        for (int i = 0; i < 10; i++) {
+            if (ui.getPushConnection() == null) {
+                break;
+            }
+
+            Thread.sleep(500);
+        }
+        Assert.assertNull(ui.getPushConnection());
+
+    }
+}

uitest/src/main/java/com/vaadin/tests/push/PushWebsocketDeadlockUI.java

@@ -0,0 +1,120 @@
+package com.vaadin.tests.push;
+
+import com.vaadin.annotations.Push;
+import com.vaadin.server.SessionDestroyEvent;
+import com.vaadin.server.SessionDestroyListener;
+import com.vaadin.server.VaadinRequest;
+import com.vaadin.server.WrappedSession;
+import com.vaadin.shared.ui.ui.Transport;
+import com.vaadin.tests.components.AbstractTestUIWithLog;
+import com.vaadin.ui.Button;
+import com.vaadin.ui.Button.ClickEvent;
+import com.vaadin.ui.Button.ClickListener;
+import com.vaadin.ui.Label;
+
+@Push(transport = Transport.WEBSOCKET)
+public class PushWebsocketDeadlockUI extends AbstractTestUIWithLog {
+
+    // Test for https://dev.vaadin.com/ticket/18436
+    // Needs breakpoints to test, see ticket for more information
+    // Can reproduce on Tomcat 8, can't seem to reproduce using
+    // DevelopmentServerLauncher
+
+    // Rough steps to reproduce
+    // 1. Open test in a new Chrome window
+    // 2. Set breakpoint in PushHandler.connectionLost
+    // 3. Set breakpoint in UI.close
+    // 4. Set breakpoint in PushRequestHandler.handleRequest
+    // 5. Click the "schedule UI close" button
+    // 6. Close the Chrome window before the 5s timeout expires and ensure it
+    // really closes
+    // 7. Wait for three threads to hit their breakpoints
+    // 8. Continue/step forward in proper order (see ticket)
+
+    @Override
+    protected void setup(VaadinRequest request) {
+        WrappedSession wrappedSession = getSession().getSession();
+        request.getService()
+                .addSessionDestroyListener(new SessionDestroyListener() {
+                    @Override
+                    public void sessionDestroy(SessionDestroyEvent e) {
+                        System.out.println(
+                                "Session " + e.getSession() + " destroyed");
+                    }
+                });
+        final Label l = new Label("Session timeout is "
+                + wrappedSession.getMaxInactiveInterval() + "s");
+        addComponents(l);
+
+        Button button = new Button("Invalidate session");
+        button.addClickListener(new ClickListener() {
+            @Override
+            public void buttonClick(ClickEvent e) {
+                System.out.println(
+                        "invalidating " + getSession() + " for http session "
+                                + getSession().getSession().getId());
+                getSession().getSession().invalidate();
+                System.out.println("invalidated " + getSession());
+            }
+        });
+        addComponents(button);
+        button = new Button("Close UI");
+        button.addClickListener(new ClickListener() {
+            @Override
+            public void buttonClick(ClickEvent e) {
+                System.out.println("closing UI " + getUIId() + " in session "
+                        + getSession() + " for http session "
+                        + getSession().getSession().getId());
+                close();
+            }
+        });
+        addComponents(button);
+        button = new Button("Schedule Close UI (5s delay)");
+        button.addClickListener(new ClickListener() {
+            @Override
+            public void buttonClick(ClickEvent e) {
+                new Thread(new Runnable() {
+                    @Override
+                    public void run() {
+                        try {
+                            Thread.sleep(5000);
+                        } catch (InterruptedException e1) {
+                            // TODO Auto-generated catch block
+                            e1.printStackTrace();
+                        }
+                        // Breakpoint here
+                        access(new Runnable() {
+                            @Override
+                            public void run() {
+                                close();
+                                System.out.println("closing UI " + getUIId()
+                                        + " in session " + getSession()
+                                        + " for http session "
+                                        + getSession().getSession().getId());
+
+                            }
+                        });
+
+                    }
+                }).start();
+            }
+        });
+        addComponents(button);
+        button = new Button("Slow (5s) operation");
+        button.addClickListener(new ClickListener() {
+            @Override
+            public void buttonClick(ClickEvent e) {
+                try {
+                    Thread.sleep(5000);
+                } catch (InterruptedException e1) {
+                    e1.printStackTrace();
+                }
+                addComponent(new Label("Slow operation done"));
+            }
+        });
+
+        addComponents(button);
+
+    }
+
+}