Author: markt
Date: Tue Nov 24 22:20:22 2015
New Revision: 1716269

URL: http://svn.apache.org/viewvc?rev=1716269&view=rev
Log:
Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=58624
Correct a potential deadlock if the WebSocket connection is closed when a 
message write is in progress.

Added:
    
tomcat/trunk/test/org/apache/tomcat/websocket/server/TestWsRemoteEndpointImplServer.java
   (with props)
Modified:
    tomcat/trunk/java/org/apache/tomcat/websocket/WsRemoteEndpointImplBase.java
    tomcat/trunk/webapps/docs/changelog.xml

Modified: 
tomcat/trunk/java/org/apache/tomcat/websocket/WsRemoteEndpointImplBase.java
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/websocket/WsRemoteEndpointImplBase.java?rev=1716269&r1=1716268&r2=1716269&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/tomcat/websocket/WsRemoteEndpointImplBase.java 
(original)
+++ tomcat/trunk/java/org/apache/tomcat/websocket/WsRemoteEndpointImplBase.java 
Tue Nov 24 22:20:22 2015
@@ -289,15 +289,13 @@ public abstract class WsRemoteEndpointIm
         }
 
         long timeout = timeoutExpiry - System.currentTimeMillis();
-        synchronized (messagePartLock) {
-            try {
-                if (!messagePartInProgress.tryAcquire(timeout, 
TimeUnit.MILLISECONDS)) {
-                    throw new SocketTimeoutException();
-                }
-            } catch (InterruptedException e) {
-                // TODO i18n
-                throw new IOException(e);
+        try {
+            if (!messagePartInProgress.tryAcquire(timeout, 
TimeUnit.MILLISECONDS)) {
+                throw new SocketTimeoutException();
             }
+        } catch (InterruptedException e) {
+            // TODO i18n
+            throw new IOException(e);
         }
 
         for (MessagePart mp : messageParts) {

Added: 
tomcat/trunk/test/org/apache/tomcat/websocket/server/TestWsRemoteEndpointImplServer.java
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/tomcat/websocket/server/TestWsRemoteEndpointImplServer.java?rev=1716269&view=auto
==============================================================================
--- 
tomcat/trunk/test/org/apache/tomcat/websocket/server/TestWsRemoteEndpointImplServer.java
 (added)
+++ 
tomcat/trunk/test/org/apache/tomcat/websocket/server/TestWsRemoteEndpointImplServer.java
 Tue Nov 24 22:20:22 2015
@@ -0,0 +1,180 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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 org.apache.tomcat.websocket.server;
+
+import java.io.IOException;
+import java.net.URI;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+
+import javax.servlet.ServletContextEvent;
+import javax.websocket.CloseReason;
+import javax.websocket.ContainerProvider;
+import javax.websocket.DeploymentException;
+import javax.websocket.EncodeException;
+import javax.websocket.Encoder;
+import javax.websocket.EndpointConfig;
+import javax.websocket.OnClose;
+import javax.websocket.OnError;
+import javax.websocket.OnMessage;
+import javax.websocket.OnOpen;
+import javax.websocket.Session;
+import javax.websocket.WebSocketContainer;
+import javax.websocket.server.ServerContainer;
+import javax.websocket.server.ServerEndpointConfig;
+
+import org.junit.Ignore;
+import org.junit.Test;
+
+import org.apache.catalina.Context;
+import org.apache.catalina.servlets.DefaultServlet;
+import org.apache.catalina.startup.Tomcat;
+import org.apache.catalina.startup.TomcatBaseTest;
+import org.apache.tomcat.websocket.pojo.TesterUtil.SimpleClient;
+
+public class TestWsRemoteEndpointImplServer extends TomcatBaseTest {
+
+    /*
+     * https://bz.apache.org/bugzilla/show_bug.cgi?id=58624
+     *
+     * This test requires three breakpoints to be set. Two in this file (marked
+     * A & B with comments) and one (C) at the start of
+     * WsRemoteEndpointImplServer.doWrite().
+     *
+     * With the breakpoints in place, run this test.
+     * Once breakpoints A & B are reached, progress the thread at breakpoint A
+     * one line to close the connection.
+     * Once breakpoint C is reached, allow the thread at breakpoint B to
+     * continue.
+     * Then allow the thread at breakpoint C to continue.
+     *
+     * In the failure mode, the thread at breakpoint B will not progress past
+     * the call to sendObject(). If the issue is fixed, the thread at 
breakpoint
+     * B will continue past sendObject() and terminate with a TimeoutException.
+     */
+    @Ignore // This test requires manual intervention to create breakpoints 
etc.
+    @Test
+    public void testClientDropsConnection() throws Exception {
+        Tomcat tomcat = getTomcatInstance();
+        // No file system docBase required
+        Context ctx = tomcat.addContext("", null);
+        ctx.addApplicationListener(Bug58624Config.class.getName());
+        Tomcat.addServlet(ctx, "default", new DefaultServlet());
+        ctx.addServletMapping("/", "default");
+
+        WebSocketContainer wsContainer =
+                ContainerProvider.getWebSocketContainer();
+
+        tomcat.start();
+
+        SimpleClient client = new SimpleClient();
+        URI uri = new URI("ws://localhost:" + getPort() + Bug58624Config.PATH);
+
+        Session session = wsContainer.connectToServer(client, uri);
+        // Break point A required on following line
+        session.close();
+    }
+
+    public static class Bug58624Config extends WsContextListener {
+
+        public static String PATH = "/bug58624";
+        @Override
+        public void contextInitialized(ServletContextEvent sce) {
+            super.contextInitialized(sce);
+
+            ServerContainer sc = (ServerContainer) 
sce.getServletContext().getAttribute(
+                    Constants.SERVER_CONTAINER_SERVLET_CONTEXT_ATTRIBUTE);
+
+            List<Class<? extends Encoder>> encoders = new ArrayList<>();
+            encoders.add(Bug58624Encoder.class);
+            ServerEndpointConfig sec = ServerEndpointConfig.Builder.create(
+                    Bug58624Endpoint.class, PATH).encoders(encoders).build();
+
+            try {
+                sc.addEndpoint(sec);
+            } catch (DeploymentException e) {
+                throw new RuntimeException(e);
+            }
+        }
+    }
+
+    public static class Bug58624Endpoint {
+
+        private static final ExecutorService ex = 
Executors.newFixedThreadPool(1);
+
+        @OnOpen
+        public void onOpen(Session session) {
+            // Disabling blocking timeouts for this test
+            session.getUserProperties().put(
+                    
org.apache.tomcat.websocket.Constants.BLOCKING_SEND_TIMEOUT_PROPERTY,
+                    Long.valueOf(-1));
+            ex.submit(new Bug58624SendMessage(session));
+        }
+
+        @OnMessage
+        public void onMessage(String message) {
+            System.out.println("OnMessage: " + message);
+        }
+
+        @OnError
+        public void onError(Throwable t) {
+            System.err.println("OnError:");
+            t.printStackTrace();
+        }
+
+        @OnClose
+        public void onClose(@SuppressWarnings("unused") Session session, 
CloseReason cr) {
+            System.out.println("Closed " + cr);
+        }
+    }
+
+    public static class Bug58624SendMessage implements Runnable {
+        private Session session;
+
+        public Bug58624SendMessage(Session session) {
+            this.session = session;
+        }
+
+        @Override
+        public void run() {
+            try {
+                // Breakpoint B required on following line
+                session.getBasicRemote().sendObject("test");
+            } catch (IOException | EncodeException e) {
+                e.printStackTrace();
+            }
+        }
+    }
+
+    public static class Bug58624Encoder implements Encoder.Text<Object> {
+
+        @Override
+        public void destroy() {
+        }
+
+        @Override
+        public void init(EndpointConfig endpointConfig) {
+        }
+
+        @Override
+        public String encode(Object object) throws EncodeException {
+            return (String) object;
+        }
+    }
+}

Propchange: 
tomcat/trunk/test/org/apache/tomcat/websocket/server/TestWsRemoteEndpointImplServer.java
------------------------------------------------------------------------------
    svn:eol-style = native

Modified: tomcat/trunk/webapps/docs/changelog.xml
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1716269&r1=1716268&r2=1716269&view=diff
==============================================================================
--- tomcat/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/trunk/webapps/docs/changelog.xml Tue Nov 24 22:20:22 2015
@@ -124,6 +124,10 @@
         HTTP type) when establishing WebSocket connections to servers. Based on
         a patch by Niki Dokovski. (markt)
       </add>
+      <fix>
+        <bug>58624</bug>: Correct a potential deadlock if the WebSocket
+        connection is closed when a message write is in progress. (markt)
+      </fix>
     </changelog>
   </subsection>
   <subsection name="Web Applications">



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

Reply via email to