Hi Chris!
A couple of minor comments.
1)
if (s != -1 || (s == -1 && errno != EINVAL)) {
This can be simplified to
if (s != -1 || errno != EINVAL) {
2)
What about sockets created with accept(): Shouldn't NET_Accept be
modified to set O_CLOEXEC as well?
On Linux accept4() can be used to pass
It's unfortunate that the user sees this when a connection wasn't even
established:
java.net.http.HttpTimeoutException: request timed out
Could it use Channel::isOpen to return a more specific exception?
HttpConnectionException? Or at least a more specific message.
> How important is this for
On 25/07/2018 13:49, Chris Hegarty wrote:
:
The updates to the various site to use the NET_* functions are fine. However, I
think the new functions in net_util_md.c could be cleaner. I think it would be
better to fallback to socket/socketpair + fcntl when the initial call fails
with EINVAL.
Clearly the request builder `timeout` method can be used to avoid
extremely long connection timeouts ( as demonstrated below ), but I see
Bernd's call for more fine grained control over various aspects of the
request.
I'm not opposed to an "HttpRequest.Builder.connectTimeout` method, but
this is c
Alan,
> On 25 Jul 2018, at 08:24, Alan Bateman wrote:
>
> ...
>
> As I said previously, the patch isn't complete so native code calling
> fork/exec may still have to deal with other file descriptors that are
> inherited into the child. I don't object to doing this in phases of course
> but s
Hello,
I think the overall request Timeout is not a good substitute (if it applies to
connect or DNS at all). Typically you want a small connect Timeout to allow
failover or retry and a larger request Timeout to allow long running activities.
However propagating the request Timeout (shortened)
Hi Markus,
The API and implementation supports an overall response timeout through
the method:
HttpRequest.Builder timeout(Duration duration)
So, I don't think any application should be required to wait for 130
seconds.
It does not currently have a timeout setting specific for connection
s
On 24/07/2018 21:34, Chris Hegarty wrote:
On 19 Jul 2018, at 18:41, Andrew Luo wrote:
Just checking - is there any other changes that I should make to the patch, or
anything else you guys need me to do?
A webrev genderated from Andrew’s patch along with:
1) some additional includes of “ne
Hello, there was some discussion on the topic in the review of
https://bugs.openjdk.java.net/browse/JDK-8206145
And an outcome was that at least AIX recommends to repeat the close call
on EINTR .
Best regards, Matthias
>
> Message: 4
> Date: Wed, 27 Jun 2018 18:23:36 -0500
> From: D
