> On 6 May 2016, at 09:16, Felix Yang <felix.y...@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