Author: markt
Date: Tue Mar 19 19:49:51 2013
New Revision: 1458479

URL: http://svn.apache.org/r1458479
Log:
Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=54724
Session should throw IllegalStateException if closed when methods on it are 
called

Modified:
    tomcat/trunk/java/org/apache/tomcat/websocket/LocalStrings.properties
    tomcat/trunk/java/org/apache/tomcat/websocket/WsSession.java
    tomcat/trunk/test/org/apache/tomcat/websocket/TestWsWebSocketContainer.java

Modified: tomcat/trunk/java/org/apache/tomcat/websocket/LocalStrings.properties
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/websocket/LocalStrings.properties?rev=1458479&r1=1458478&r2=1458479&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/tomcat/websocket/LocalStrings.properties 
(original)
+++ tomcat/trunk/java/org/apache/tomcat/websocket/LocalStrings.properties Tue 
Mar 19 19:49:51 2013
@@ -49,6 +49,7 @@ wsRemoteEndpoint.noEncoder=No encoder sp
 # as many as 4 bytes.
 wsSession.timeout=The WebSocket session timeout expired
 
+wsSession.closed=The WebSocket session has been closed and no method (apart 
from close()) may be called on a closed session
 wsSession.duplicateHandlerBinary=A binary message handler has already been 
configured
 wsSession.duplicateHandlerPong=A pong message handler has already been 
configured
 wsSession.duplicateHandlerText=A text message handler has already been 
configured

Modified: tomcat/trunk/java/org/apache/tomcat/websocket/WsSession.java
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/websocket/WsSession.java?rev=1458479&r1=1458478&r2=1458479&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/tomcat/websocket/WsSession.java (original)
+++ tomcat/trunk/java/org/apache/tomcat/websocket/WsSession.java Tue Mar 19 
19:49:51 2013
@@ -138,6 +138,7 @@ public class WsSession implements Sessio
 
     @Override
     public WebSocketContainer getContainer() {
+        checkState();
         return webSocketContainer;
     }
 
@@ -145,6 +146,9 @@ public class WsSession implements Sessio
     @SuppressWarnings("unchecked")
     @Override
     public void addMessageHandler(MessageHandler listener) {
+
+        checkState();
+
         Type t = Util.getMessageType(listener);
 
         if (t.equals(String.class)) {
@@ -180,6 +184,7 @@ public class WsSession implements Sessio
 
     @Override
     public Set<MessageHandler> getMessageHandlers() {
+        checkState();
         Set<MessageHandler> result = new HashSet<>();
         if (binaryMessageHandler != null) {
             result.add(binaryMessageHandler);
@@ -196,6 +201,7 @@ public class WsSession implements Sessio
 
     @Override
     public void removeMessageHandler(MessageHandler listener) {
+        checkState();
         if (listener == null) {
             return;
         }
@@ -216,24 +222,28 @@ public class WsSession implements Sessio
 
     @Override
     public String getProtocolVersion() {
+        checkState();
         return Constants.WS_VERSION_HEADER_VALUE;
     }
 
 
     @Override
     public String getNegotiatedSubprotocol() {
+        checkState();
         return subProtocol;
     }
 
 
     @Override
     public List<Extension> getNegotiatedExtensions() {
+        checkState();
         return Collections.EMPTY_LIST;
     }
 
 
     @Override
     public boolean isSecure() {
+        checkState();
         return secure;
     }
 
@@ -246,54 +256,63 @@ public class WsSession implements Sessio
 
     @Override
     public long getMaxIdleTimeout() {
+        checkState();
         return maxIdleTimeout;
     }
 
 
     @Override
     public void setMaxIdleTimeout(long timeout) {
+        checkState();
         this.maxIdleTimeout = timeout;
     }
 
 
     @Override
     public void setMaxBinaryMessageBufferSize(int max) {
+        checkState();
         this.maxBinaryMessageBufferSize = max;
     }
 
 
     @Override
     public int getMaxBinaryMessageBufferSize() {
+        checkState();
         return maxBinaryMessageBufferSize;
     }
 
 
     @Override
     public void setMaxTextMessageBufferSize(int max) {
+        checkState();
         this.maxTextMessageBufferSize = max;
     }
 
 
     @Override
     public int getMaxTextMessageBufferSize() {
+        checkState();
         return maxTextMessageBufferSize;
     }
 
 
     @Override
     public Set<Session> getOpenSessions() {
+        checkState();
         return webSocketContainer.getOpenSessions(localEndpoint.getClass());
     }
 
 
     @Override
     public RemoteEndpoint.Async getAsyncRemote() {
+        checkState();
         return remoteEndpointAsync;
     }
 
 
     @Override
     public RemoteEndpoint.Basic getBasicRemote() {
+        checkState();
         return remoteEndpointBasic;
     }
 
@@ -314,6 +333,9 @@ public class WsSession implements Sessio
             if (state != State.OPEN) {
                 return;
             }
+
+            fireEndpointOnClose(closeReason);
+
             state = State.CLOSING;
 
             sendCloseMessage(closeReason);
@@ -331,6 +353,7 @@ public class WsSession implements Sessio
         synchronized (stateLock) {
             if (state == State.OPEN) {
                 sendCloseMessage = true;
+                fireEndpointOnClose(closeReason);
             }
 
             state = State.CLOSED;
@@ -345,6 +368,19 @@ public class WsSession implements Sessio
     }
 
 
+    private void fireEndpointOnClose(CloseReason closeReason) {
+
+        // Fire the onClose event
+        Thread t = Thread.currentThread();
+        ClassLoader cl = t.getContextClassLoader();
+        t.setContextClassLoader(applicationClassLoader);
+        try {
+            localEndpoint.onClose(this, closeReason);
+        } finally {
+            t.setContextClassLoader(cl);
+        }
+    }
+
     private void sendCloseMessage(CloseReason closeReason) {
         // 125 is maximum size for the payload of a control message
         ByteBuffer msg = ByteBuffer.allocate(125);
@@ -366,16 +402,6 @@ public class WsSession implements Sessio
         } finally {
             webSocketContainer.unregisterSession(
                     localEndpoint.getClass(), this);
-
-            // Fire the onClose event
-            Thread t = Thread.currentThread();
-            ClassLoader cl = t.getContextClassLoader();
-            t.setContextClassLoader(applicationClassLoader);
-            try {
-                localEndpoint.onClose(this, closeReason);
-            } finally {
-                t.setContextClassLoader(cl);
-            }
         }
 
     }
@@ -383,30 +409,35 @@ public class WsSession implements Sessio
 
     @Override
     public URI getRequestURI() {
+        checkState();
         return requestUri;
     }
 
 
     @Override
     public Map<String,List<String>> getRequestParameterMap() {
+        checkState();
         return requestParameterMap;
     }
 
 
     @Override
     public String getQueryString() {
+        checkState();
         return queryString;
     }
 
 
     @Override
     public Principal getUserPrincipal() {
+        checkState();
         return userPrincipal;
     }
 
 
     @Override
     public Map<String,String> getPathParameters() {
+        checkState();
         return pathParameters;
     }
 
@@ -419,6 +450,7 @@ public class WsSession implements Sessio
 
     @Override
     public Map<String,Object> getUserProperties() {
+        checkState();
         return userProperties;
     }
 
@@ -465,6 +497,12 @@ public class WsSession implements Sessio
     }
 
 
+    private void checkState() {
+        if (!isOpen()) {
+            throw new IllegalStateException(sm.getString("wsSession.closed"));
+        }
+    }
+
     private static enum State {
         OPEN,
         CLOSING,

Modified: 
tomcat/trunk/test/org/apache/tomcat/websocket/TestWsWebSocketContainer.java
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/tomcat/websocket/TestWsWebSocketContainer.java?rev=1458479&r1=1458478&r2=1458479&view=diff
==============================================================================
--- tomcat/trunk/test/org/apache/tomcat/websocket/TestWsWebSocketContainer.java 
(original)
+++ tomcat/trunk/test/org/apache/tomcat/websocket/TestWsWebSocketContainer.java 
Tue Mar 19 19:49:51 2013
@@ -605,15 +605,26 @@ public class TestWsWebSocketContainer ex
         Assert.assertEquals(3, setA.size());
 
         int count = 0;
-        setA = s3a.getOpenSessions();
-        while (setA.size() > 0 && count < 8) {
+        boolean isOpen = true;
+        while (isOpen && count < 8) {
             count ++;
             Thread.sleep(1000);
-            setA = s3a.getOpenSessions();
+            isOpen = false;
+            for (Session session : setA) {
+                if (session.isOpen()) {
+                    isOpen = true;
+                }
+            }
         }
 
-        if (setA.size() > 0) {
-            Assert.fail("There were [" + setA.size() + "] open sessions");
+        if (isOpen) {
+            for (Session session : setA) {
+                if (session.isOpen()) {
+                    System.err.println("Session with ID [" + session.getId() +
+                            "] is open");
+                }
+            }
+            Assert.fail("There were open sessions");
         }
     }
 
@@ -649,22 +660,31 @@ public class TestWsWebSocketContainer ex
 
         int expected = 3;
         while (expected > 0) {
-            Assert.assertEquals(expected, setA.size());
+            Assert.assertEquals(expected, getOpenCount(setA));
 
             int count = 0;
-            while (setA.size() == expected && count < 5) {
+            while (getOpenCount(setA) == expected && count < 5) {
                 count ++;
                 Thread.sleep(1000);
-                setA = s3a.getOpenSessions();
             }
 
             expected--;
         }
 
-        Assert.assertEquals(0, setA.size());
+        Assert.assertEquals(0, getOpenCount(setA));
     }
 
 
+    private int getOpenCount(Set<Session> sessions) {
+        int result = 0;
+        for (Session session : sessions) {
+            if (session.isOpen()) {
+                result++;
+            }
+        }
+        return result;
+    }
+
     private Session connectToEchoServerBasic(WebSocketContainer wsContainer,
             Class<? extends Endpoint> clazz) throws Exception {
         return wsContainer.connectToServer(clazz,



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

Reply via email to