Client initiated close can cause ClosedChannelException if client doesn't wait for CloseWebSocketFrame

[philipwhiuk]

In 2.3.x (seen in 2.3.8) I believe there's a race condition in:

https://github.com/playframework/playframework/blob/2.3.x/framework/src/play/src/main/scala/play/core/server/netty/WebSocketHandler.scala#L115

If the client doesn't wait for CloseWebSocketFrame it can close between the call to isOpen() and the write:

java.nio.channels.ClosedChannelException
        at sun.nio.ch.SocketChannelImpl.ensureWriteOpen(SocketChannelImpl.java:269)
        at sun.nio.ch.SocketChannelImpl.write(SocketChannelImpl.java:460)
        at org.jboss.netty.channel.socket.nio.SocketSendBufferPool$UnpooledSendBuffer.transferTo(SocketSendBufferPool.java:203)
        at org.jboss.netty.channel.socket.nio.AbstractNioWorker.write0(AbstractNioWorker.java:201)
        at org.jboss.netty.channel.socket.nio.AbstractNioWorker.writeFromUserCode(AbstractNioWorker.java:146)
        at org.jboss.netty.channel.socket.nio.NioServerSocketPipelineSink.handleAcceptedSocket(NioServerSocketPipelineSink.java:99)
        at org.jboss.netty.channel.socket.nio.NioServerSocketPipelineSink.eventSunk(NioServerSocketPipelineSink.java:36)
        at org.jboss.netty.channel.DefaultChannelPipeline$DefaultChannelHandlerContext.sendDownstream(DefaultChannelPipeline.java:779)
        at org.jboss.netty.channel.Channels.write(Channels.java:725)
        at org.jboss.netty.handler.codec.oneone.OneToOneEncoder.doEncode(OneToOneEncoder.java:71)
        at org.jboss.netty.handler.codec.oneone.OneToOneEncoder.handleDownstream(OneToOneEncoder.java:59)
        at org.jboss.netty.channel.DefaultChannelPipeline.sendDownstream(DefaultChannelPipeline.java:591)
        at org.jboss.netty.channel.DefaultChannelPipeline$DefaultChannelHandlerContext.sendDownstream(DefaultChannelPipeline.java:784)
        at org.jboss.netty.channel.SimpleChannelHandler.writeRequested(SimpleChannelHandler.java:292)
        at org.jboss.netty.channel.SimpleChannelHandler.handleDownstream(SimpleChannelHandler.java:254)
        at com.typesafe.netty.http.pipelining.HttpPipeliningHandler.handleDownstream(HttpPipeliningHandler.java:106)
        at org.jboss.netty.channel.DefaultChannelPipeline.sendDownstream(DefaultChannelPipeline.java:591)
        at org.jboss.netty.channel.DefaultChannelPipeline.sendDownstream(DefaultChannelPipeline.java:582)
        at org.jboss.netty.channel.Channels.write(Channels.java:704)
        at org.jboss.netty.channel.Channels.write(Channels.java:671)
        at org.jboss.netty.channel.AbstractChannel.write(AbstractChannel.java:248)
        at play.core.server.netty.WebSocketHandler$$anon$1.closeWebSocket(WebSocketHandler.scala:118)
        at play.core.server.netty.WebSocketHandler$$anon$1.messageReceived(WebSocketHandler.scala:88)
        at org.jboss.netty.channel.SimpleChannelUpstreamHandler.handleUpstream(SimpleChannelUpstreamHandler.java:70)
        ...

I think it should catch a ClosedChannelException here (or maybe any IOException).

[jroper]

We should define a Netty exception handler that ignores the exceptions - there's no race condition, other than the usual problem of it being impossible to know whether a connection is open or closed before you write to it.

[philipwhiuk]

There's a race between isOpen getting called at it being closed. The isOpen call seems pretty pointless to me given it makes no guarantees about the future write.

[jroper]

That's what I meant about the "usual problem", there's always a race when writing, there's nothing we can do about that. The point of checking isOpen is to maximise the chances of gracefully handling the connection being closed.