Author: remy.maucherat(a)jboss.com Date: 2014-09-03 11:52:30 -0400 (Wed, 03 Sep 2014) New Revision: 2500 Modified: branches/7.5.x/src/main/java/org/apache/tomcat/websocket/WsWebSocketContainer.java branches/7.5.x/src/main/java/org/apache/tomcat/websocket/server/WsServerContainer.java branches/7.5.x/src/main/java/org/jboss/web/WebsocketsLogger.java Log: Port websockets thread handling robustness fixes, that could have caused some issues on shutdown. Modified: branches/7.5.x/src/main/java/org/apache/tomcat/websocket/WsWebSocketContainer.java =================================================================== --- branches/7.5.x/src/main/java/org/apache/tomcat/websocket/WsWebSocketContainer.java 2014-09-03 08:05:20 UTC (rev 2499) +++ branches/7.5.x/src/main/java/org/apache/tomcat/websocket/WsWebSocketContainer.java 2014-09-03 15:52:30 UTC (rev 2500) @@ -271,6 +271,7 @@ ByteBuffer response; String subProtocol; + boolean success = false; try { fConnect.get(timeout, TimeUnit.MILLISECONDS); @@ -307,6 +308,7 @@ } else { throw new DeploymentException(MESSAGES.invalidProtocolHeader()); } + success = true; } catch (ExecutionException e) { throw new DeploymentException(MESSAGES.httpRequestFailed(), e); } catch (InterruptedException e) { @@ -317,6 +319,10 @@ throw new DeploymentException(MESSAGES.httpRequestFailed(), e); } catch (TimeoutException e) { throw new DeploymentException(MESSAGES.httpRequestFailed(), e); + } finally { + if (!success) { + channel.close(); + } } // Switch to WebSocket Modified: branches/7.5.x/src/main/java/org/apache/tomcat/websocket/server/WsServerContainer.java =================================================================== --- branches/7.5.x/src/main/java/org/apache/tomcat/websocket/server/WsServerContainer.java 2014-09-03 08:05:20 UTC (rev 2499) +++ branches/7.5.x/src/main/java/org/apache/tomcat/websocket/server/WsServerContainer.java 2014-09-03 15:52:30 UTC (rev 2500) @@ -290,13 +290,42 @@ public void destroy() { shutdownExecutor(); super.destroy(); + // If the executor hasn't fully shutdown it won't be possible to + // destroy this thread group as there will still be threads running. + // Mark the thread group as daemon one, so that it destroys itself + // when thread count reaches zero. + // Synchronization on threadGroup is needed, as there is a race between + // destroy() call from termination of the last thread in thread group + // marked as daemon versus the explicit destroy() call. + int threadCount = threadGroup.activeCount(); + boolean success = false; try { - threadGroup.destroy(); - } catch (IllegalThreadStateException itse) { - // If the executor hasn't fully shutdown it won't be possible to - // destroy this thread group as there will still be threads running - WebsocketsLogger.ROOT_LOGGER.threadGroupNotDestryed(threadGroup.getName()); + while (true) { + int oldThreadCount = threadCount; + synchronized (threadGroup) { + if (threadCount > 0) { + Thread.yield(); + threadCount = threadGroup.activeCount(); + } + if (threadCount > 0 && threadCount != oldThreadCount) { + // Value not stabilized. Retry. + continue; + } + if (threadCount > 0) { + threadGroup.setDaemon(true); + } else { + threadGroup.destroy(); + success = true; + } + break; + } + } + } catch (IllegalThreadStateException exception) { + // Fall-through } + if (!success) { + WebsocketsLogger.ROOT_LOGGER.threadGroupNotDestryed(threadGroup.getName(), Integer.valueOf(threadCount)); + } } Modified: branches/7.5.x/src/main/java/org/jboss/web/WebsocketsLogger.java =================================================================== --- branches/7.5.x/src/main/java/org/jboss/web/WebsocketsLogger.java 2014-09-03 08:05:20 UTC (rev 2499) +++ branches/7.5.x/src/main/java/org/jboss/web/WebsocketsLogger.java 2014-09-03 15:52:30 UTC (rev 2500) @@ -99,7 +99,7 @@ void noWebsocketsSupport(); @LogMessage(level = WARN) - @Message(id = 8814, value = "Thread group %s not destroyed") - void threadGroupNotDestryed(String name); + @Message(id = 8814, value = "Thread group %s not destroyed, %s threads left") + void threadGroupNotDestryed(String name, int threadCount); }