Return-Path: 
 <dev-return-136861-apmail-tomcat-dev-archive=tomcat.apache.org@tomcat.apache.org>
X-Original-To: apmail-tomcat-dev-archive@www.apache.org
Delivered-To: apmail-tomcat-dev-archive@www.apache.org
Received: from mail.apache.org (hermes.apache.org [140.211.11.3])
	by minotaur.apache.org (Postfix) with SMTP id 4F391D4B6
	for <apmail-tomcat-dev-archive@www.apache.org>;
 Fri, 15 Mar 2013 21:09:08 +0000 (UTC)
Received: (qmail 60320 invoked by uid 500); 15 Mar 2013 21:09:07 -0000
Delivered-To: apmail-tomcat-dev-archive@tomcat.apache.org
Received: (qmail 60214 invoked by uid 500); 15 Mar 2013 21:09:07 -0000
Mailing-List: contact dev-help@tomcat.apache.org; run by ezmlm
Precedence: bulk
List-Help: <mailto:dev-help@tomcat.apache.org>
List-Unsubscribe: <mailto:dev-unsubscribe@tomcat.apache.org>
List-Post: <mailto:dev@tomcat.apache.org>
List-Id: <dev.tomcat.apache.org>
Reply-To: "Tomcat Developers List" <dev@tomcat.apache.org>
Delivered-To: mailing list dev@tomcat.apache.org
Received: (qmail 60205 invoked by uid 99); 15 Mar 2013 21:09:07 -0000
Received: from minotaur.apache.org (HELO minotaur.apache.org) (140.211.11.9)
    by apache.org (qpsmtpd/0.29) with ESMTP; Fri, 15 Mar 2013 21:09:07 +0000
Received: from localhost (HELO [192.168.23.9]) (127.0.0.1)
  (smtp-auth username markt, mechanism plain)
  by minotaur.apache.org (qpsmtpd/0.29) with ESMTP;
 Fri, 15 Mar 2013 21:09:07 +0000
Message-ID: <51438DEA.6010708@apache.org>
Date: Fri, 15 Mar 2013 21:08:58 +0000
From: Mark Thomas <markt@apache.org>
User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64;
 rv:17.0) Gecko/20130307 Thunderbird/17.0.4
MIME-Version: 1.0
To: Tomcat Developers List <dev@tomcat.apache.org>
Subject: Re: svn commit: r1457104 - in
 /tomcat/trunk/java/org/apache/tomcat/websocket:
 LocalStrings.properties WsFrameBase.java WsRemoteEndpointImplBase.java
 WsSession.java
References: <20130315204054.9EBA82388994@eris.apache.org>
 <CABzHfVmJpNbn2ZET9+G-KKOu0P9bY7YEJrmiXL1qxEz8-He=Ag@mail.gmail.com>
In-Reply-To: 
 <CABzHfVmJpNbn2ZET9+G-KKOu0P9bY7YEJrmiXL1qxEz8-He=Ag@mail.gmail.com>
X-Enigmail-Version: 1.5.1
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit

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