RFR JDK-8087113: Websocket API and implementation

[Pavel Rappo]

> On 6 May 2016, at 09:16, Felix Yang <felix.yang at oracle.com> wrote:
> 
> Hi Pavel,
>     several comments:
> 
> 1.  WebSocket.request(long n) is documented as "
> 
> @throws IllegalArgumentException if n < -1
> "
> 
> It looks meaningless to allow 0.

First of all, the way `request` is defined now is by no means meaningless.

     * @return resulting unfulfilled demand with this request taken into account
     */
    long request(long n);

Passing zero allows you, for instance, to get current demand without modifying it. 
But, this is going to be removed [1] (WebSocket API tasks, item 10).

Think of the ability to pass 0 as of a degree of robustness. That's a corner
case. It allows you to put some computed value, not necessarily a literal as it
does not require you to check the value for 0. As far as it is non-negative, no
harm is done.

If you're still not convinced, consider any of the following:

        "abc".substring(1, 1);
        collection.removeAll(Collections.emptyList());
        inputStream.read(new byte[0]);
        byteChannel.read(ByteBuffer.allocate(0));

> 2.  Some concern on the way of handling close. Consider following scenario.
> 
> 	• obtain a ws connection
> 	• message communications balabala...
> 	• initialize closing handshake on either client/server peer
> 	• if the client side didn't request adequate size or not requested at all, it actually doesn't know connection is closed. "onClose" will never be invoked, because close message is at the end of "queue".

True. That's the cost of providing reactive back-pressure. Otherwise we would
have to read off the socket in the background looking for the Close message to
appear.
 
> 	• it looks to me that:
> 		• the closing handshake can be blocked by UN-requested messages. 
> 		• a websocket can be in a state that isClosed() return false but it can't send any message. Note that the state is not transient.

WebSocket is not necessarily closed after you send a Close message:

    /**
     * Tells whether the {@code WebSocket} is closed.
     *
     * <p> A {@code WebSocket} deemed closed when either the underlying socket
     * is closed or the closing handshake is completed.
     *
     * @return {@code true} if the {@code WebSocket} is closed,
     *         {@code false} otherwise
     */
    boolean isClosed();

> Filed a bug https://bugs.openjdk.java.net/browse/JDK-8156191. It looks current design treats CloseMessage in similar way with other messages. I'm not sure if it is worth something like WSClosingHandshake.
> 3. I'm concerning the bug in #2 is side-effect of reactive mode. May I ask what is the benefit of reactive mode. WebSocket.request(long n) seem to be good in first glance but:
> 	• It looks end users need to calculate how many messages are expected. The another question comes 
> 		• what messages will be taken into account. How about PING/PONG or else?
> 	• We may use request( a big value up to Long.MAX). But, in this case, why we need it?  This looks simple but a bit problematic.

You can read on benefits of reactive mode here [2, 3]. And feel free to read the
API thread [4] as well.

--------------------------------------------------------------------------------
[1] https://bugs.openjdk.java.net/browse/JDK-8155621
[2] http://www.reactive-streams.org
[3] https://github.com/reactive-streams/reactive-streams-jvm/
[4] http://mail.openjdk.java.net/pipermail/net-dev/2015-August/009077.html
    http://mail.openjdk.java.net/pipermail/net-dev/2015-September/009079.html
    http://mail.openjdk.java.net/pipermail/net-dev/2015-October/009223.html