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.

> -        if (!open) {
> +        if (state != State.OPEN) {
>              return;
>          }
> -        synchronized (closeLock) {
> -            if (!open) {
> +        synchronized (stateLock) {
> +            if (state != State.OPEN) {
>                  return;
>              }
> -            open = false;
> +            state = State.CLOSING;
>
> -            // Send the close message
> -            // 125 is maximum size for the payload of a control message
> -            ByteBuffer msg = ByteBuffer.allocate(125);
> -            msg.putShort((short) closeReason.getCloseCode().getCode());
> -            String reason = closeReason.getReasonPhrase();
> -            if (reason != null && reason.length() > 0) {
> -                msg.put(reason.getBytes(UTF8));
> +            sendCloseMessage(closeReason);
> +        }
> +    }

Best regards,
Konstantin Kolinko

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

Reply via email to