tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Mark Thomas <ma...@apache.org>
Subject Re: svn commit: r1457104 - in /tomcat/trunk/java/org/apache/tomcat/websocket: LocalStrings.properties WsFrameBase.java WsRemoteEndpointImplBase.java WsSession.java
Date Fri, 15 Mar 2013 21:08:58 GMT
On 15/03/2013 21:00, Konstantin Kolinko wrote:
> 2013/3/16  <markt@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-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Mime
View raw message