"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;
}
--- /dev/null
+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());
+
+ }
+}
--- /dev/null
+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);
+
+ }
+
+}