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: [email protected]
For additional commands, e-mail: [email protected]
