Re: websockets

[James Roper]

Hi Simone,

Sorry, I think there's been some missed information here. I am in no way
suggesting that the API as provided currently should be changed or
replaced. What I'm suggesting is that a high level adapter be provided out
of the box that provides Reactive Streams support on top of this API. Yes,
there's no formal definition of high/low level, but in the case of
WebSockets, we can draw some lines based roughly on what web browsers
(which is the target application developer experience that WebSockets were
designed for) support as being high level, and what the WebSocket protocol
allows beyond this as low level. I think a Reactive Streams adapter on top
of the existing low level API that has feature parity with the JavaScript
API for WebSockets offered by browsers is a worthwhile thing to aim for.
Would you agree with that?

So, for example, the browser APIs offer one error callback for handling
errors from both directions. All WebSocket messages handled by the browser
APIs are discrete, fixed length in memory, any chunking is handled
transparently underneath. Developers in the browser don't have to
explicitly send or implement any sending or receiving of the close
messages, they just handle the high level concept of the socket being
closed, and the browser implements the handshake underneath transparently.

I'd say WebSocket proxies would be completely out of scope for a high level
API, they definitely would want to deal with splitting messages, custom
close handling, very precise error semantics etc, and such a use case would
be well served by the current low level API.

Regards,

James

On 26 February 2018 at 22:15, Simone Bordet  wrote:

> James,
>
> On Mon, Feb 26, 2018 at 4:37 AM, James Roper  wrote:
> > On the topic of error handling. A high level API doesn't need to report
> each
> > individual error with sending.
>
> Depends on what you mean by "high level APIs"... I guess there is no
> formal definition of that, as one can even think that the Java socket
> APIs are high level.
>
> > So firstly, it is impossible to report *all*
> > errors with sending, since it's impossible to know, once you send a
> message
> > across the network, whether it got there or not. So if an application
> > developer has a requirement to handle errors in sending (and
> realistically,
> > there is never a business require to handle just the errors that can be
> > detected in sending, it's always about handling all errors), do we expect
> > them to implement that logic in two places, both on the sending side to
> > handle errors that can be detected on the sending side, and then
> > additionally write logic to handle errors that can't be detected on the
> > sending side (such as, for example, having the remote side send ACKs, and
> > track the ACKs on the receiving end)? I doubt it, having to handle logic
> on
> > both sides is going to mean the application developer has to spend more
> time
> > implementing boiler plate that could otherwise be spent solving their
> actual
> > business problem.
>
> I think you are oversimplifying this.
>
> If my business is to write a WebSocket Proxy, I may totally want to
> care about writing code that handles errors differently whether they
> happened on the read side or on the write side, if just for logging
> reasons.
>

> If my business is to write a protocol on top of WebSocket, I may want
> to use custom WebSocket error codes (depending on errors) when I close
> the connection.
> I may want to send a WS message the moment I receive a WebSocket
> "close" event from a peer.
>
> Sure, maybe in 80% of the cases read and write errors are handled the
> same way, but as an application writer I would like to have the choice
> between using an API that is "lower" level and allows me full
> flexibility with respect to the WebSocket protocol, and an opinionated
> (correctly opinionated in the majority of the cases) framework that
> for some case reduces the API surface at the cost of something else
> like flexibility, increased allocation, ecc.
> Both have their use cases.
>
> I believe the JDK net group always stated they went for the first choice.
> The lack of the second choice is under discussion, and as I have
> already stated may be a matter of resources.
>
> > If an application developer needs to handle errors in
> > sending messages, it would be much simpler for them to treat both types
> of
> > errors in the same way. And hence, a high level API should expose
> detectable
> > errors in sending on the receiving end, by cancelling upstream,
> terminating
> > the WebSocket with an error, and then emitting the error in the
> downstream
> > onError.
> >
> > When it comes to protocol specific concerns - a high level API should not
> > expose that to developers.  A developer should not be allowed to send a
> text
> > message between two split binary frames. A developer should not be
> > responsible for implementing the closing handshake 

RFR [11] 8198358 : Align organization of DualStackPlainSocketImpl with TwoStacksPlainSocketImp [win]

[Ivan Gerasimov]

Hello!

I'd like to do the next step toward removing the TwoStacks socket 
implementation on Windows.


It would be aligning the two implementations (DualStack and TwoStacks), 
so they can be easier merged together during the next step.


There are three PlainSocketImpl implementations in JDK:
java.base/windows/classes/java/net/DualStackPlainSocketImpl.java
java.base/windows/classes/java/net/TwoStacksPlainSocketImpl.java
java.base/unix/classes/java/net/PlainSocketImpl.java

While two later have very similar organization (in particular, set of 
native methods), the former is organized slightly differently.
In order to merge the two Windows implementation together, they first 
need to be organized in a similar way.
For consistency, DualStack implementation will be reorganized to be 
aligned with TwoStacks and unix/PlainSocketImpl.


Bug: https://bugs.openjdk.java.net/browse/JDK-8198358
Webrev: http://cr.openjdk.java.net/~igerasim/8198358/00/webrev/

The change looks somewhat messy, but in fact it was a series of 
incremental changes, which I still keep in the mercurial 'mq'.


(I wish the webrev could be made incremental based on the mq patches, to 
make it easier to review.)


The patched JDK builds fine and all the regression tests pass Okay.


Thanks in advance!
--

With kind regards,
Ivan Gerasimov

RFR: JDK-8144300 : Java does not honor http.nonProxyHosts value having wildcard * both at end and start

[Pallavi Sonal]

Hi,

Please review the following change to fix JDK-8144300. 

Bug : https://bugs.openjdk.java.net/browse/JDK-8144300 

Webrev : http://cr.openjdk.java.net/~rpatil/8144300/webrev.00/ 

Whenever there is a wildcard(*) at either the beginning or the end of a 
hostname specified in the list of http.nonProxyHosts , the code works as 
expected and DIRECT connection is used for them. But in scenarios, where there 
is a wildcard both at the beginning and end of the hostname, proxy is used 
instead of DIRECT connection.

 

All the tier1 and tier2 Mach 5 tests have passed.

 

Thanks,

Pallavi Sonal

Re: httpserver does not close connections when RejectedExecutionException occurs

[KUBOTA Yuji]

Hi all,

Could you please review my patch(s)?

Thanks,
Yuji

2018-02-20 14:21 GMT+09:00 KUBOTA Yuji :
> Hi Daniel,
>
> Thank you for your comment.
>
> Dispatcher is package-private class and handle method is called at
> only this file in the package (sun.net.httpserver), and all call sites
> look like to handle exception suitably. So we can remove `throws
> IOException` clause without modifying the call site logic as like
> webrev.03.
> http://cr.openjdk.java.net/~ykubota/8169358/webrev.03
>
> However, I could not find whether can I change a signature of public
> method without specified steps. If we need to write CSR to remove
> `throws IOException` clause, we should remove the clause by other
> issue. And I want to commit webrev.02  at this issue.
> http://cr.openjdk.java.net/~ykubota/8169358/webrev.02
>
> I'd like to get your thoughts on that.
>
> P.S. I'm an author, not a committer, so I cannot access Mach5.
>
> Thanks,
> Yuji
>
> 2018-02-17 1:11 GMT+09:00 Daniel Fuchs :
>> Hi,
>>
>> On the surface I'd say that this change looks reasonable.
>> However, handle is declared to throw IOException, and
>> with this change it is guaranteed to never throw even
>> RuntimeException.
>>
>> It seems that the code that calls handle() - at least in
>> this file, has some logic to handle IOException - or
>> other exception possibly thrown by Dispatcher::handle,
>> which as far as I can see is not doing much more than what
>> closeConnection does. So it looks as if calling
>> closeConnection in handle() instead of throwing could be OK.
>>
>> However it would be good to at least try to figure out
>> whether there are any other call sites for Dispatcher::handle,
>> validate that no longer throwing will not modify the call site
>> logic, and possibly investigate if the 'throws IOException' clause
>> can be removed from Dispatcher::handle (that is: look
>> whether Dispatcher is exposed and/or can be subclassed and
>> if there are any existing subclasses).
>>
>> best regards,
>>
>> -- daniel
>>
>>
>> On 16/02/2018 15:29, KUBOTA Yuji wrote:
>>>
>>> Hi Chris and Yasumasa,
>>>
>>> Sorry to have remained this issue for a long time. I come back.
>>>
>>> I updated my patch for the following three reasons.
>>> http://cr.openjdk.java.net/~ykubota/8169358/webrev.02/
>>>
>>> * applies to the latest jdk/jdk instead of jdk9/dev
>>> * adds test by modifying reproducer as like tests on
>>> test/jdk/com/sun/net/httpserver
>>> * prevents potential connection leak as Yasumasa said. For example, a
>>> user sets own implementation of the thread pool to HttpServer and then
>>> throws unexpected exceptions and errors. I think that we should save
>>> existing exception handler to keep compatibility and minimize the risk
>>> of connection leak by catching Throwable.
>>>
>>> I've tested test/jdk/com/sun/net/httpserver and passed.
>>> I'm not a committer, so I can not access March 5.
>>>
>>> Could you review and sponsor it?
>>>
>>> Thanks,
>>> Yuji
>>>
>>> 2016-11-11 12:11 GMT+09:00 KUBOTA Yuji :

 Hi Chris and Yasumasa,

 Thank you for your comments!

 I had faced connection leak on production by this handler. It seems
 like a corner-case but it's a real risk to me.
 I had focused REE on this issue, but it is a subclass of
 RuntimeException, so I think that we should investigate
 RuntimeException, too.

 If Chris agrees with the covering RuntimeException by JDK-8169358, I
 will investigate it and update my patch.
 If we should investigate on another issue, I want to add a test case
 to check fixing about REE like existing jtreg, and I will create a new
 issue after an investigation about RuntimeException.

 Any thoughts?

 Thank you!
 Yuji

 2016-11-11 0:56 GMT+09:00 Chris Hegarty :
>
>
>> On 10 Nov 2016, at 14:43, Yasumasa Suenaga  wrote:
>>
>> Hi,
>>
>>> I think it best to just handle the specific case of REE, as it done in
>>> Yuji’s patch.
>>
>>
>> Will it be a cause of connection leak if RuntimeException is occurred
>> in handler method?
>
>
> No, at least not from this point in the code.
>
>> I think we should catch RuntimeException at least.
>
>
> Possibly, but it seems like a corner-case of a corner-case.
>
 I think you can use finally statement to close the connection in this
 case.
>>>
>>>
>>> I don’t believe that this is possible. The handling of the connection
>>> may be
>>> done in a separate thread, some time after execute() returns.
>>
>>
>> So I said we need to check the implementation of HTTP connection and
>> dispatcher.
>
>
> To me, this goes beyond the scope of the issue describe in JIRA, but if
> you want to investigate that, then that’s fine.
>
> -Chris.
>