diff options
| -rw-r--r-- | server/src/main/java/com/vaadin/ui/UI.java | 18 | ||||
| -rw-r--r-- | server/src/test/java/com/vaadin/ui/UITest.java | 155 | ||||
| -rw-r--r-- | uitest/src/main/java/com/vaadin/tests/push/PushWebsocketDeadlockUI.java | 120 |
3 files changed, 291 insertions, 2 deletions
diff --git a/server/src/main/java/com/vaadin/ui/UI.java b/server/src/main/java/com/vaadin/ui/UI.java index dbca873cc1..8926ec6bd4 100644 --- a/server/src/main/java/com/vaadin/ui/UI.java +++ b/server/src/main/java/com/vaadin/ui/UI.java @@ -508,9 +508,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; } diff --git a/server/src/test/java/com/vaadin/ui/UITest.java b/server/src/test/java/com/vaadin/ui/UITest.java new file mode 100644 index 0000000000..322bc81a04 --- /dev/null +++ b/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()); + + } +} diff --git a/uitest/src/main/java/com/vaadin/tests/push/PushWebsocketDeadlockUI.java b/uitest/src/main/java/com/vaadin/tests/push/PushWebsocketDeadlockUI.java new file mode 100644 index 0000000000..4d6e9c4c21 --- /dev/null +++ b/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); + + } + +} |