On 15/03/2013 21:00, Konstantin Kolinko wrote: > 2013/3/16 <ma...@apache.org>: >> Author: markt >> Date: Fri Mar 15 20:40:53 2013 >> New Revision: 1457104 >> >> URL: http://svn.apache.org/r1457104 >> Log: >> Correctly handle two stage close process. >> >> Modified: >> tomcat/trunk/java/org/apache/tomcat/websocket/LocalStrings.properties >> tomcat/trunk/java/org/apache/tomcat/websocket/WsFrameBase.java >> >> tomcat/trunk/java/org/apache/tomcat/websocket/WsRemoteEndpointImplBase.java >> tomcat/trunk/java/org/apache/tomcat/websocket/WsSession.java >> > > > >> 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=1457104&r1=1457103&r2=1457104&view=diff >> ============================================================================== >> --- tomcat/trunk/java/org/apache/tomcat/websocket/WsSession.java (original) >> +++ tomcat/trunk/java/org/apache/tomcat/websocket/WsSession.java Fri Mar 15 >> 20:40:53 2013 >> @@ -70,8 +70,8 @@ public class WsSession implements Sessio >> private MessageHandler textMessageHandler = null; >> private MessageHandler binaryMessageHandler = null; >> private MessageHandler.Whole<PongMessage> pongMessageHandler = null; >> - private volatile boolean open = true; >> - private final Object closeLock = new Object(); >> + private State state = State.OPEN; >> + private final Object stateLock = new Object(); >> private final Map<String,Object> userProperties = new >> ConcurrentHashMap<>(); >> private volatile int maxBinaryMessageBufferSize = >> Constants.DEFAULT_BUFFER_SIZE; >> @@ -226,7 +226,7 @@ public class WsSession implements Sessio >> >> @Override >> public boolean isOpen() { >> - return open; >> + return state == State.OPEN; >> } >> >> >> @@ -293,45 +293,84 @@ public class WsSession implements Sessio >> @Override >> public void close(CloseReason closeReason) throws IOException { >> // Double-checked locking. OK because open is volatile > > The above comment is obsoleted by this commit, as "open" field is > removed and state is not volatile. > I'd say that it is ok for the state to be non volatile because it only > changes one way (open->closing->closed) and thus no harm for seeing > stale value if the stale value is State.OPEN.
I think there might be some potential harm there. I'll make it volatile and fix the comment. Thanks for the catch. Mark --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
