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