Re: RFR: 8220083: Use InetAddress.getLoopbackAddress() in place of 127.0.0.1 for some tests

[Vyom Tiwari]

Hi Arthur,

Changes looks good to me, one minor comment, in HTTPTestServer.java  we can
avoid  new local variable (InetAddress address) .

Thanks,
Vyom

On Tue, Mar 12, 2019 at 10:30 PM Arthur Eubanks  wrote:

> Thanks for creating the umbrella bug.
>
> Split long line.
> Added the HTTPTestServer.java and UrgentDataTest.java changes in Chris's
> suggestions. The other 2 involve URI/URLs, and there are a lot of those
> changes, will group them in a later change.
>
> PTAL: http://cr.openjdk.java.net/~aeubanks/8220083/webrev.02/
>
>
> On Tue, Mar 12, 2019 at 5:00 AM Chris Hegarty 
> wrote:
>
>> Hi Arthur,
>>
>>
>> On 11 Mar 2019, at 18:14, Arthur Eubanks  wrote:
>>
>> Updated copyright years (I asked around, it should be fine), updated
>> commit message.
>> http://cr.openjdk.java.net/~aeubanks/8220083/webrev.01
>>
>>
>> This is a welcome improvement to the tests. Reviewed, with some minor
>> comments and additions below.
>>
>> It will not be easy to get all the JDK networking tests passing
>> successfully on IPv6-only environments ( ignoring the other combinations
>> for now ). But this is a good improvement, and hopefully the start of
>> a number of changes that add similar incremental improvements. To this
>> end, I've created an umbrella task, 8220499, to track these test changes
>> ( in one place ), and updated this issue, 8220083, to be a sub-task of
>> it. Let's add more sub-tasks as needed.
>>
>> Specific webrev comment:
>>  * Can you please split the long line in TunnelThroughProxy.java
>>
>> Suggestion for similar changes while here:
>>  * http://cr.openjdk.java.net/~chegar/8220083.additional/index.html
>>
>> -Chris.
>>
>

-- 
Thanks,
Vyom

Re: RFR [13] 8153508: ContentHandler API contains link to private contentPathProp

[Vyom Tiwari]

Hi Chris,

Change looks good to me.
Thanks,
Vyom

On Tue, Mar 19, 2019 at 4:12 PM Chris Hegarty 
wrote:

> This review request is to resolve a documentation regression in Java 9,
> resulting from part of the changes for 8132478, where {@value
> java.net.URLConnection#contentPathProp} was inadvertently replaced with
> {@link java.net.URLConnection#contentPathProp}. The former will inline
> with the constant field value, where the latter will link to the
> ( private ) field.
>
> The fix is to just revert the incorrect change.
>
> --- a/src/java.base/share/classes/java/net/ContentHandler.java
> +++ b/src/java.base/share/classes/java/net/ContentHandler.java
> @@ -47,7 +47,7 @@
>   * If no content handler could be {@linkplain URLConnection#getContent()
> found},
>   * URLConnection will look for a content handler in a user-definable set
> of places.
>   * Users can define a vertical-bar delimited set of class prefixes
> - * to search through by defining the {@link
> java.net.URLConnection#contentPathProp}
> + * to search through by defining the {@value
> java.net.URLConnection#contentPathProp}
>   * property. The class name must be of the form:
>   * 
>   * {package-prefix}.{major}.{minor}
>
> -Chris.



-- 
Thanks,
Vyom

Re: RFR 8170494 : JNI exception pending in PlainDatagramSocketImpl.c

[Vyom Tiwari]

Hi Ivan,

Looks OK to me,  in case of exception "ni"  will be null  you can use the
macro(CHECK_NULL_RETURN) if you are not clearing the JNI exception, this
will save at least one additional function( "(*env)->ExceptionOccurred(env)"
) call.

Thanks,
Vyom

On Wed, Mar 20, 2019 at 9:49 PM Ivan Gerasimov 
wrote:

> Hello!
>
> The function Java_java_net_NetworkInterface_getByInetAddress0 may throw,
> so after calling it we need to check if an exception is pending.
>
> Would you please help review a one-line fix?
>
> BUGURL: https://bugs.openjdk.java.net/browse/JDK-8170494
> WEBREV: http://cr.openjdk.java.net/~igerasim/8170494/00/webrev/
>
> Thanks in advance!
>
> --
> With kind regards,
> Ivan Gerasimov
>
>

-- 
Thanks,
Vyom

Re: RFR 8170494 : JNI exception pending in PlainDatagramSocketImpl.c

[Vyom Tiwari]

Hi Ivan,

thanks for explanation, code change looks good to me.

Thanks,
Vyom

On Thu, Mar 21, 2019 at 11:26 AM Ivan Gerasimov 
wrote:

> Thanks Vyom!
>
> On 3/20/19 10:13 PM, Vyom Tiwari wrote:
>
> Hi Ivan,
>
> Looks OK to me,  in case of exception "ni"  will be null  you can use the
> macro(CHECK_NULL_RETURN) if you are not clearing the JNI exception, this
> will save at least one additional function( "
> (*env)->ExceptionOccurred(env)" ) call.
>
>
> Java_java_net_NetworkInterface_getByInetAddress0 may return NULL if there
> were no interfaces found.
> We should not return from getMulticastInterface() in this case.
>
> With kind regards,
> Ivan
>
>
> Thanks,
> Vyom
>
> On Wed, Mar 20, 2019 at 9:49 PM Ivan Gerasimov 
> wrote:
>
>> Hello!
>>
>> The function Java_java_net_NetworkInterface_getByInetAddress0 may throw,
>> so after calling it we need to check if an exception is pending.
>>
>> Would you please help review a one-line fix?
>>
>> BUGURL: https://bugs.openjdk.java.net/browse/JDK-8170494
>> WEBREV: http://cr.openjdk.java.net/~igerasim/8170494/00/webrev/
>>
>> Thanks in advance!
>>
>> --
>> With kind regards,
>> Ivan Gerasimov
>>
>>
>
> --
> Thanks,
> Vyom
>
>
> --
> With kind regards,
> Ivan Gerasimov
>
>

-- 
Thanks,
Vyom

Re: RFR: 8223457 java.net.ServerSocket protected constructor should throw NPE if impl null

[Vyom Tiwari]

Looks good to me.
Vyom

On Tue, May 7, 2019 at 7:20 PM Michael McMahon 
wrote:

> Could I get the following small change reviewed please?
> The recent change in the SocketImpl code unmasked the fact that
> the protected constructor of ServerSocket was not checking for a null
> impl parameter as required.
>
> http://cr.openjdk.java.net/~michaelm/8223457/webrev.1/
>
> Thanks
> Michael
>


-- 
Thanks,
Vyom

Re: RFR: 8223880: Update sun/net/ftp/FtpURL.java and sun/net/ftp/FtpURLConnectionLeak.java to work with IPv6 addresses

[Vyom Tiwari]

Hi Daniel,

Changes looks good to me, as you said code is copied from one test to
another, i found FtpGetContent.java where same FtpServer code is copied.
Are you planning to fix FtpGetContent.java as well  ?.

Thanks,
Vyom

On Wed, May 15, 2019 at 2:18 PM Daniel Fuchs 
wrote:

> Hi Arthue,
>
> On 14/05/2019 23:57, Arthur Eubanks wrote:
> > In test/jdk/sun/net/ftp/FtpURL.java, extendedEnabled is always true.
> > Same with portEnabled and pasvEnabled.
>
> Yes - I left them there as they were preexisting.
> The bits that handles EPSV is a copy paste from another
> test and that minimized the changes.
>
> > Is the assumption that all
> > current servers support these modes? If so, why is there a bool to say
> > those modes are not enabled?
>
> I have no idea. EPSV and EPRT have been introduced do
> support IPv6, and are replacements for PASV and PORT
> which only support IPv4. I would guess that servers
> should all support these modes by now.
>
> As to your question - my guess is that this test server
> got copy & pasted in many tests and some of them might
> want to disable some of these modes for testing purposes.
>
> WRT EPSV/PASV the jdk client tries EPSV ALL first, and
> if the server replies with `500 'command not understood'`
> it will fallback to PASV.
>
> Because this particular server in the test didn't support
> EPSV before - it was using PASV for IPv4 addresses.
> I made sure this would continue to happen - so that
> what's happening on the client side doesn't change
> when testing with IPv4.
>
> PASV and PORT will not work on a machine that only has
> IPv6.
>
> best regards,
>
> -- daniel
>
>
>

-- 
Thanks,
Vyom

Re: RFR: 8223880: Update sun/net/ftp/FtpURL.java and sun/net/ftp/FtpURLConnectionLeak.java to work with IPv6 addresses

[Vyom Tiwari]

Hi Daniel,

latest change looks good to me.

Even  i wanted to remove duplicate "FtpServer code" that we had copy and
pasted but somehow I did not got time do it.

Thanks,
Vyom

On Wed, May 15, 2019 at 7:08 PM Daniel Fuchs 
wrote:

> Hi Vyom,
>
> On 15/05/2019 11:25, Vyom Tiwari wrote:
> > Hi Daniel,
> >
> > Changes looks good to me,
>
> Thanks!
>
> > as you said code is copied from one test to
> > another, i found FtpGetContent.java where same FtpServer code is
> > copied.  Are you planning to fix FtpGetContent.java as well  ?.
>
> Well - I wasn't intending to but it looks like a low
> hanging fruit. All other tests in that directory either
> do not use PASV or already support EPSV so it seems like
> a low hanging fruit to bundle the same change FtpGetContent.java
> and then we'll be done with it...
>
> Here it goes...
> http://cr.openjdk.java.net/~dfuchs/webrev_8223880/webrev.01/
>
> best regards,
>
> -- daniel
>
>
> >
> > Thanks,
> > Vyom
>
>

-- 
Thanks,
Vyom

Re: [teststabilization] RFR: 8223798 : Replace wildcard address with loopback or local host in tests - part 7

[Vyom Tiwari]

Hi Aleks,
latest changes looks good to me .
Thanks,
Vyom

On Wed, May 15, 2019 at 11:12 PM Aleks Efimov 
wrote:

> Hi Daniel,
>
> Thanks for the review. I've modified Socket_getInputStream_[read|write]
> to follow your suggestion:
> -ServerSocket ss = new ServerSocket(0);
>   InetAddress lh = InetAddress.getLocalHost();
> +ServerSocket ss = new ServerSocket(0, 0, lh);
>
> Also I've broke the long lines in few places. Will push the changes
> shortly.
>
> With Best Regards,
> Aleksei
>
> On 15/05/2019 17:59, Daniel Fuchs wrote:
> > Hi Aleksei,
> >
> > On 15/05/2019 17:07, Aleks Efimov wrote:
> >> Hi,
> >>
> >> Another part of test fixes to address intermittent networking test
> >> failures can be viewed here:
> >> http://cr.openjdk.java.net/~aefimov/8223798/00/
> >
> > Socket_getInputStream_read.java:
> > Socket_getOutputStream_write.java:
> >
> > I think you could simply move the initialization of
> > lh above the line that creates the server socket
> > and bind the server socket to lh instead. That
> > would keep the changes minimal.
> >
> > The rest looks good - but there are a few long lines.
> > If you can break them before pushing that would be good.
> >
> > best regards,
> >
> > -- daniel
> >
> >>
> >> Could I please ask for the help to review it?
> >>
> >> JBS:
> >> https://bugs.openjdk.java.net/browse/JDK-8223798
> >>
> >>
> >> With Best Regards,
> >> Aleksei
> >
>
>

-- 
Thanks,
Vyom

Re: [ipv6]: 8224081: SOCKS v4 doesn't work with IPv6

[Vyom Tiwari]

Hi Arthur,
do we need "Integer.toString(4)" to convert int to string in
SocksProxyVersion ?
Thanks,
Vyom

On Fri, May 17, 2019 at 4:46 AM Arthur Eubanks  wrote:

> bug: https://bugs.openjdk.java.net/browse/JDK-8224081
> webrev: http://cr.openjdk.java.net/~aeubanks/8224081/webrev.00/index.html
>
> Tests that try to use SOCKS v4 will fail in an IPv6 only environment since
> SOCKS v4 does not support IPv6. SOCKS v5 does support IPv6.
>


-- 
Thanks,
Vyom

Re: [ipv6]: 8224081: SOCKS v4 doesn't work with IPv6

[Vyom Tiwari]

+1

On Fri, May 17, 2019 at 3:29 PM Daniel Fuchs 
wrote:

> Hi Arthur,
>
> On 17/05/2019 00:16, Arthur Eubanks wrote:
> > bug: https://bugs.openjdk.java.net/browse/JDK-8224081
> > webrev:
> http://cr.openjdk.java.net/~aeubanks/8224081/webrev.00/index.html
> >
> > Tests that try to use SOCKS v4 will fail in an IPv6 only environment
> > since SOCKS v4 does not support IPv6. SOCKS v5 does support IPv6.
>
> Your changes to java/net/Socks/SocksProxyVersion.java look good to me.
>
> However I'd like your changes to
> test/jdk/sun/security/x509/URICertStore/SocksProxy.java
> to be reviewed by the security-dev team which I have added
> in cc:, since I believe this falls into their area.
>
> best regards,
>
> -- daniel
>
>

-- 
Thanks,
Vyom

Re: [teststabilization] RFR: 8224603: Replace wildcard address with loopback or local host in tests - part 11

[Vyom Tiwari]

Hi Daniel,
Overall changes looks good to me, please update the copy write date that
you missed in couple of files. In "TestHttpServer.java"  we are printing
error message on System.err and throwing the RuntimeException both as
below, do you think we need both ?

 System.err.println ("Server could not start: " + e);+ throw new
RuntimeException("Server could not start: " + e, e);

Thanks,
Vyom

On Fri, May 24, 2019 at 6:33 PM Chris Hegarty 
wrote:

>
> On 22/05/2019 18:35, Daniel Fuchs wrote:
> > Hi,
> >
> > Please find below a patch for the next batch of tests
> > that have been observed failing intermittently.
> >
> > JBS: https://bugs.openjdk.java.net/browse/JDK-8224603
> >
> > webrev: http://cr.openjdk.java.net/~dfuchs/webrev_8224603/webrev.00/
>
> Looks ok to me Daniel.
>
> -Chris.
>


-- 
Thanks,
Vyom

Re: [teststabilization] RFR: 8224603: Replace wildcard address with loopback or local host in tests - part 11

[Vyom Tiwari]

Hi Daniel,

SocksProxyVersion.java is one of them my local repo it is showing
(Copyright (c) 2011, 2015, Oracle and/or its). I was talking do we need
both(printing message on System.err & throwing exception), as you already
explain throwing exception is required but do we need  same error
message ("Server
could not start:") on System.err as well ?
Thanks,
Vyom

On Fri, May 24, 2019 at 7:32 PM Daniel Fuchs 
wrote:

> Hi Vyom,
>
> On 24/05/2019 14:36, Vyom Tiwari wrote:
> > Hi Daniel,
> > Overall changes looks good to me, please update the copy write date that
> > you missed in couple of files.
>
> Which files did I miss?
>
> > In "TestHttpServer.java"  we are printing
> > error message on System.err and throwing the RuntimeException both as
> > below, do you think we need both ?
> >
> >   System.err.println ("Server could not start: " + e);
> > + throw new RuntimeException("Server could not start: " + e, e);
>
> Yes - if we don't throw the test will fail in timeout instead.
> I prefer to fail fast. The exception might be swallowed if the
> server is started in an executor thread though - so let's keep
> both.
>
> best regards.
>
> -- daniel
>
> >
> > Thanks,
> > Vyom
> >
> > On Fri, May 24, 2019 at 6:33 PM Chris Hegarty  > <mailto:chris.hega...@oracle.com>> wrote:
> >
> >
> > On 22/05/2019 18:35, Daniel Fuchs wrote:
> >  > Hi,
> >  >
> >  > Please find below a patch for the next batch of tests
> >  > that have been observed failing intermittently.
> >  >
> >  > JBS: https://bugs.openjdk.java.net/browse/JDK-8224603
> >  >
> >  > webrev:
> http://cr.openjdk.java.net/~dfuchs/webrev_8224603/webrev.00/
> >
> > Looks ok to me Daniel.
> >
> > -Chris.
> >
> >
> >
> > --
> > Thanks,
> > Vyom
>
>

-- 
Thanks,
Vyom

Re: RFR: 8223714: HTTPSetAuthenticatorTest could be made more resilient

[Vyom Tiwari]

Hi Jaikiran,

Code changes look OK to me , couple of minor comments
1-> you don't need "this" to access stop(this.stop)
2-> i think super.stop() should be the first line in the overridden stop
method.
3-> it is matter of personal preference but my preference is use the new
variable name 'running' to check if server is running in place of 'stop'
but again it is just a matter of personal  preference.

Thanks,
Vyom

On Sat, Aug 31, 2019 at 9:37 AM Jaikiran Pai 
wrote:

> Hello Daniel,
>
> On 30/08/19 9:14 PM, Daniel Fuchs wrote:
> > Hi Jaikiran,
> >
> > 1075 System.out.println("Server will stop
> > accepting connections due to an exception");
> > 1076 io.printStackTrace(System.out);
> > 1077 break;
> >
> > Two things here:
> >
> >  1.if ss.connect() throws, it may be because ss is closed, or not.
> >To be on the safe side, we should call ss.close() before break;
>
> You are right. I have now updated this part of the code.
>
>
> >  2.if the application calls stop() I expect that the call to ss.accept()
> >will throw.
> >
> >Therefore I think that the exception should be simply skipped
> >if stop == true (that's normal termination, we don't want to pollute
> >the log with stack traces of exceptions that are expected),
> >but we should probably always print it on System.err if stop == false
> >otherwise we will be reduced to guessing if we see the client side
> >failing with Connection Refused.
> >
> Agreed. This has now been updated too.
>
> The updated webrev is at
> http://cr.openjdk.java.net/~jpai/webrev/8223714/2/webrev/. I have also
> included you as the reviewer in the commit message.
>
>
> > Otherwise, I believe it looks good. Thanks for another good fix :-)
>
> Thank you :)
>
> -Jaikiran
>
>
>

-- 
Thanks,
Vyom

Re: [teststabilization] RFR: 8231631: sun/net/ftp/FtpURLConnectionLeak.java fails intermittently with NPE

[Vyom Tiwari]

Hi Daniel,

Code change looks good to me.

Thanks,
Vyom

On Wed, Oct 23, 2019 at 10:50 PM Daniel Fuchs 
wrote:

> Hi,
>
> Please find below a trivial test fix for:
>
> 8231631: sun/net/ftp/FtpURLConnectionLeak.java fails intermittently
>   with NPE
> https://bugs.openjdk.java.net/browse/JDK-8231631
>
> webrev:
> http://cr.openjdk.java.net/~dfuchs/webrev_8231631/webrev.00/
>
> readLine() returns null on EOF - which was mishandled.
>
> best regards,
>
> -- daniel
>


-- 
Thanks,
Vyom

Re: RFR[15] JDK-8183369 : RFC unconformity of HttpURLConnection with proxy

[Vyom Tiwari]

Hi Ravi/Daniel,

At my local env(REL 7) test is passing without fix as well. Although my
local repo contain some additional code changes but it is not related with
the current fix.

Test1 Passed with: Connect timed out
Test2 Passed with: Connect timed out
##

Please change copyright year(2020) as well.

Thanks,
Vyom

On Thu, Feb 13, 2020 at 6:27 PM Daniel Fuchs 
wrote:

> Hi,
>
> On 13/02/2020 10:52, Ravi Reddy wrote:
> > I notice that the `break` from the original code has not been
> >
> > reintroduced. I don't think that it is strictly needed, but did you give
> >
> > it any consideration?
> >
> > Chris , in original code since we were doing retry with direct
> > connection and not proceeding further with proxies by adding ‘break;’
> > , whereas now we are retrying with the proxy and if the connection fails
> > again , we have to make sure we do try with other proxies , hence I
> > haven’t reintroduced  ‘break’ .
> >
>
> The break is not needed because it.hasNext() is false - but IMO it
> would make the code more readable - I had to stare at the code for some
> time to convince myself that it was not needed.
> (where I assume we're talking about introducing a `break;` after
> line 1213).
>
> Also can you please confirm that the test fails on the jdk mainline
> without your fix?
>
> best regards,
>
> -- daniel
>
>

-- 
Thanks,
Vyom

RFR 8238579: HttpsURLConnection drops the timeout and hangs forever in read

[Vyom Tiwari]

Hi All,

Please find the below fix  which resolves the issue(
https://bugs.openjdk.java.net/browse/JDK-8238579).

"HttpURLConnection.writeRequests()" retry in case of any write failure,
during retry it creates new HttpsClient  without connectTimeout &
readTimeout. Below fix sets the connect & read timeout.

Thanks,
Vyom

diff -r 7e6165c9c606
src/java.base/share/classes/sun/net/www/protocol/https/AbstractDelegateHttpsURLConnection.java
---
a/src/java.base/share/classes/sun/net/www/protocol/https/AbstractDelegateHttpsURLConnection.java
Thu Feb 13 17:14:45 2020 -0800
+++
b/src/java.base/share/classes/sun/net/www/protocol/https/AbstractDelegateHttpsURLConnection.java
Fri Feb 14 10:11:06 2020 +0530
@@ -87,10 +87,15 @@
  */
 public void setNewClient (URL url, boolean useCache)
 throws IOException {
+int readTimeout = getReadTimeout();
 http = HttpsClient.New (getSSLSocketFactory(),
 url,
 getHostnameVerifier(),
-useCache, this);
+ null,
+ -1,
+useCache,
+ getConnectTimeout(), this);
+ http.setReadTimeout(readTimeout);
 ((HttpsClient)http).afterConnect();
 }

@@ -132,10 +137,14 @@
 boolean useCache) throws IOException {
 if (connected)
 return;
-http = HttpsClient.New (getSSLSocketFactory(),
-url,
-getHostnameVerifier(),
-proxyHost, proxyPort, useCache, this);
+int readTimeout = getReadTimeout();
+http = HttpsClient.New(getSSLSocketFactory(),
+url,
+getHostnameVerifier(),
+proxyHost, proxyPort,
+useCache,
+getConnectTimeout(), this);
+http.setReadTimeout(readTimeout);
 connected = true;
 }

Re: RFR 8237858: PlainSocketImpl.socketAccept() handles EINTR incorrectly

[Vyom Tiwari]

Hi Daniel/Michael,

Please find the latest webrev(
http://cr.openjdk.java.net/~vtewari/8237858/webrev0.2/index.html), where i
put a test case as well.
Thanks,
Vyom

On Wed, Mar 11, 2020 at 3:16 PM Bernd Eckenfels 
wrote:

> Hello,
>
> Depending on the test environment you don't need much native tools. You
> can use java.io.File to open /proc/thread-self to get the TID and use
> ProcessBuilder to execute the kill command.
>
> Gruss
> Bernd
> --
> http://bernd.eckenfels.net
> --
> *Von:* net-dev  im Auftrag von Alan
> Bateman 
> *Gesendet:* Wednesday, March 11, 2020 9:21:24 AM
> *An:* Vyom Tewari26 
> *Cc:* net-dev@openjdk.java.net 
> *Betreff:* Re: RFR 8237858: PlainSocketImpl.socketAccept() handles EINTR
> incorrectly
>
> On 11/03/2020 08:09, Vyom Tewari26 wrote:
>
> Hi Alan,
>
> Thanks for comment yes for  >=JDK13 we are using new socket impl which
> does not have this problem, i am not planning to wright a test,  to test
> this issue we need to get the native thread ID then we have to interrupt
> the thread to see if JDK code is behaving as expected.
>
> I tested this issue at my local Linux env, what i did is modify the
> java.lang.Thread.getId() to always return the *native thread id of main
> thread* and use this native thread id in the test program to send a
> signal(kill -SIGPIPE  threadid) and  i checked that JDK code is behaving as
> expected.
>
> We can try to write a test case(which requires some native code as well)
> which simulate the above but that will be lot of work.
>
> We can file a new issue to write a test which test how(read,write,accept)
> operations behave when they are interrupted by a signal.
>
> Alan, what do you think ?.
>
> Native tests used to be hard but there is supporting infrastructure in the
> build for some time that makes it a lot easier. In this case, I think it
> should be possible to invoke a JNI method that returns the native thread id
> and then it can be signalled when the thread is blocked in accept (or read
> or write). So it shouldn't be too hard, it comes to whether anyone has time.
>
> -Alan.
>


-- 
Thanks,
Vyom

Re: [14-dev] RFR : (dc) MulticastSendReceiveTests.java failing with ENOMEM when joining group (OS X 10.9)

[Vyom Tiwari]

Looks good to me.
Vyom

On Fri, Mar 20, 2020 at 12:49 PM Ivan Gerasimov 
wrote:

> Hello!
>
> An OOM error is intermittently observed on MacOS when joining a
> multicast group.
>
> A workaround was implemented as part of the fix for JDK-8236925.
>
> It is proposed to backport a portion of that fix, which helps to
> mitigate the issue on MacOS.
>
> BUGURL: https://bugs.openjdk.java.net/browse/JDK-8044365
> WEBREV: http://cr.openjdk.java.net/~igerasim/8044365/00/webrev/
>
> Mach5 control build in progress.
>
> --
> With kind regards,
> Ivan Gerasimov
>
>

-- 
Thanks,
Vyom

Re: RFR 15 8243099: Adding ADQ support to JDK

[Vyom Tiwari]

Hi Vladimir,

In LinuxSocketOptions.c we have lot's of duplicate code, can you please
reuse "socketOptionSupported" & "handleError" as follows, this we remove
lot's of duplicate code.
Thanks,
Vyom

diff -r b6b4506a7827 src/jdk.net/linux/native/libextnet/LinuxSocketOptions.c
--- a/src/jdk.net/linux/native/libextnet/LinuxSocketOptions.c Fri Apr 24
15:28:57 2020 +0800
+++ b/src/jdk.net/linux/native/libextnet/LinuxSocketOptions.c Fri Apr 24
13:37:36 2020 +0530
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2017, 2018, Oracle and/or its affiliates. All rights
reserved.
+ * Copyright (c) 2017, 2020, Oracle and/or its affiliates. All rights
reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -33,6 +33,10 @@
 #include "jni_util.h"
 #include "jdk_net_LinuxSocketOptions.h"

+#ifndef SO_INCOMING_NAPI_ID
+#define SO_INCOMING_NAPI_ID56
+#endif
+
 /*
  * Class: jdk_net_LinuxSocketOptions
  * Method:setQuickAck
@@ -44,15 +48,7 @@
 int rv;
 optval = (on ? 1 : 0);
 rv = setsockopt(fd, SOL_SOCKET, TCP_QUICKACK, &optval, sizeof
(optval));
-if (rv < 0) {
-if (errno == ENOPROTOOPT) {
-JNU_ThrowByName(env, "java/lang/UnsupportedOperationException",
-"unsupported socket option");
-} else {
-JNU_ThrowByNameWithLastError(env, "java/net/SocketException",
-"set option TCP_QUICKACK failed");
-}
-}
+handleError(env, rv, "set option TCP_QUICKACK failed");
 }

 /*
@@ -65,15 +61,7 @@
 int on;
 socklen_t sz = sizeof (on);
 int rv = getsockopt(fd, SOL_SOCKET, TCP_QUICKACK, &on, &sz);
-if (rv < 0) {
-if (errno == ENOPROTOOPT) {
-JNU_ThrowByName(env, "java/lang/UnsupportedOperationException",
-"unsupported socket option");
-} else {
-JNU_ThrowByNameWithLastError(env, "java/net/SocketException",
-"get option TCP_QUICKACK failed");
-}
-}
+handleError(env, rv, "get option TCP_QUICKACK failed");
 return on != 0;
 }

@@ -83,31 +71,18 @@
  * Signature: ()Z
  */
 JNIEXPORT jboolean JNICALL
Java_jdk_net_LinuxSocketOptions_quickAckSupported0
-(JNIEnv *env, jobject unused) {
-int one = 1;
-int rv, s;
-s = socket(PF_INET, SOCK_STREAM, 0);
-if (s < 0) {
-return JNI_FALSE;
-}
-rv = setsockopt(s, SOL_SOCKET, TCP_QUICKACK, (void *) &one, sizeof
(one));
-if (rv != 0 && errno == ENOPROTOOPT) {
-rv = JNI_FALSE;
-} else {
-rv = JNI_TRUE;
-}
-close(s);
-return rv;
+(JNIEnv *env, jobject unused) {
+return socketOptionSupported(SOL_SOCKET, TCP_QUICKACK);
 }

-static jint socketOptionSupported(jint sockopt) {
+static jint socketOptionSupported(jint level, jint optname) {
 jint one = 1;
 jint rv, s;
 s = socket(PF_INET, SOCK_STREAM, IPPROTO_TCP);
 if (s < 0) {
 return 0;
 }
-rv = setsockopt(s, SOL_TCP, sockopt, (void *) &one, sizeof (one));
+rv = setsockopt(s, level, optname, (void *) &one, sizeof (one));
 if (rv != 0 && errno == ENOPROTOOPT) {
 rv = 0;
 } else {
@@ -135,8 +110,8 @@
  */
 JNIEXPORT jboolean JNICALL
Java_jdk_net_LinuxSocketOptions_keepAliveOptionsSupported0
 (JNIEnv *env, jobject unused) {
-return socketOptionSupported(TCP_KEEPIDLE) &&
socketOptionSupported(TCP_KEEPCNT)
-&& socketOptionSupported(TCP_KEEPINTVL);
+return socketOptionSupported(SOL_TCP, TCP_KEEPIDLE) &&
socketOptionSupported(SOL_TCP, TCP_KEEPCNT)
+&& socketOptionSupported(SOL_TCP, TCP_KEEPINTVL);
 }

 /*
@@ -213,3 +188,27 @@
 handleError(env, rv, "get option TCP_KEEPINTVL failed");
 return optval;
 }
+
+/*
+ * Class: jdk_net_LinuxSocketOptions
+ * Method:IncomingNapiIdSupported0
+ * Signature: (I)I;
+ */
+JNIEXPORT jboolean JNICALL
Java_jdk_net_LinuxSocketOptions_IncomingNapiIdSupported0
+(JNIEnv *env, jobject unused) {
+return socketOptionSupported(SOL_SOCKET, SO_INCOMING_NAPI_ID);
+}
+
+/*
+ * Class: jdk_net_LinuxSocketOptions
+ * Method:getIncomingNapiId0
+ * Signature: (I)I;
+ */
+JNIEXPORT jint JNICALL Java_jdk_net_LinuxSocketOptions_getIncomingNapiId0
+(JNIEnv *env, jobject unused, jint fd) {
+jint optval, rv;
+socklen_t sz = sizeof (optval);
+rv = getsockopt(fd, SOL_SOCKET, SO_INCOMING_NAPI_ID, &optval, &sz);
+handleError(env, rv, "get option SO_INCOMING_NAPI_ID failed");
+return optval;
+}

On Fri, Apr 24, 2020 at 12:43 AM Ivanov, Vladimir A <
vladimir.a.iva...@intel.com> wrote:

> Thanks a lot to Chris and Alan for detailed comments.
> The updated version of patch available at
> http://cr.openjdk.java.net/~sviswanathan/Vladimir/8243099/webrev.01/
>
> Changes:
> 1. in native code the common pattern was used for the 'getsockopt' call.
> 2. condition to define SO_INCOMING_NAPI_ID was added.
> 3. t

Re: RFR 15 8243099: Adding ADQ support to JDK

[Vyom Tiwari]

Hi Vladimir,

changes looks much better now, i still see that
function(IncomingNapiIdSupported) & variable(IncomingNapiIdOptSupported)
does not follow Java naming convention.

   private static final boolean IncomingNapiIdOptSupported  =+
   platformSocketOptions.IncomingNapiIdSupported();

Thanks,
Vyom

On Tue, Apr 28, 2020 at 3:36 AM Ivanov, Vladimir A <
vladimir.a.iva...@intel.com> wrote:

> Thanks for the comments. The webrev was updated:
> http://cr.openjdk.java.net/~sviswanathan/Vladimir/8243099/webrev.02/
>
> Only the code was changed here. Changes in the documentation will be
> discussed separately.
>
> Changes:
> 1. The LinuxSocketOptions.c was updated according to Vyom comment.
> Note, the 'socketOptionSupported' implemented through the "setsockopt"
> call that always return error for the read only properties.
> It was updated to use 'getsockopt'. The tests set 'test/jdk/java/net
> test/jdk/java/nio/channels test/jdk/javax/net test/jdk/jdk/net
> test/jdk/sun/net'
> have same run results on RHEL7.7 as clean jdk/jdk workspace. Please,
> notify me if testing should be extended.
>
> 2. The 'IncomingNapiIdSupported0' was renamed to the
> 'incomingNapiIdSupported0.'
>
> 3. The test 'PrintSupportedOptions' was updated to use Set for
> read only options.
>
> 4. The test ExtOptionNAPITest.java was added. Now the test
> 'ExtOptionTest.java' do nothing with napi id.
>
>  Thanks, Vladimir
>
> -Original Message-
> From: Alan Bateman 
> Sent: Friday, April 24, 2020 12:09 AM
> To: Ivanov, Vladimir A ; OpenJDK Network Dev
> list 
> Subject: Re: RFR 15 8243099: Adding ADQ support to JDK
>
> On 23/04/2020 20:11, Ivanov, Vladimir A wrote:
> > Thanks a lot to Chris and Alan for detailed comments.
> > The updated version of patch available at
> > http://cr.openjdk.java.net/~sviswanathan/Vladimir/8243099/webrev.01/
> >
> > Changes:
> > 1. in native code the common pattern was used for the 'getsockopt' call.
> > 2. condition to define SO_INCOMING_NAPI_ID was added.
> > 3. the DatagarmSocket was added to the ExtOptionTest 4. testing on my
> > side was extended to the subset 'test/jdk/java/net
> test/jdk/java/nio/channels test/jdk/javax/net test/jdk/jdk/net
> test/jdk/sun/net'.
> > Results are same for the patched and non-patched builds on the RHEL7.7
> OS.
> > Tests test/jdk/java/net/SocketOption/AfterClose.java and
> > test/jdk/java/nio/channels/etc/PrintSupportedOptions.java were updated
> to support read only properties.
> > 5. description for the NAPI_ID was updated 6. the
> > UnsupportedOperationException was added to the 'setOption' call for the
> 'SO_INCOMING_NAPI_ID'.
> >
> (Dropping core-libs-dev as net-dev is the more appropriate list for this
> area).
>
> Thanks for the update.
>
> The updated javadoc looks better but will need a few iterations. It would
> be good if it could start by explaining what the value of the socket option
> is, e.g. "The value of this socket option is an Integer representing the
> network device queue ...". I think it needs to be clearer on the types of
> sockets that support this option, does it support both stream-oriented and
> datagram-oriented sockets? Does it return a value when invoked on a
> ServerSocketChannel? Instead of @throws, the text will need to that
> attempting to set the socket option will cause UOE to be thrown. Once the
> javadoc is agreed then the CSR can be submitted and the code review can
> continue in parallel.
>
> The implementation changes mostly look okay although
> IncomingNapiIdSupported0 should probably be renamed to start with lower
> case "i". Also someone might need to check that you can create an IPv4
> socket when a system is configured as an IPv6-only system.
>
> Tests. I think the the new tests in ExtOptionTest will need closer
> examination as I can't tell how reliable they are. It might be that it
> needs a completely new test. The update to the NIO PrintSupportedOptions
> test define READ_ONLY_OPTS as Set.
>
> -Alan
>


-- 
Thanks,
Vyom

Re: RFR 15 8243099: Adding ADQ support to JDK

[Vyom Tiwari]

Hi Vladimir,

Latest webrev(03) looks ok to me , one minor  bit, method signature(Signature:
(I)I;) is not  correct in incomingNapiIdSupported0.

Thanks,
Vyom

++/*+ * Class: jdk_net_LinuxSocketOptions+ * Method:
incomingNapiIdSupported0+ * Signature: (I)I;+ */+JNIEXPORT jboolean
JNICALL Java_jdk_net_LinuxSocketOptions_incomingNapiIdSupported0+(JNIEnv
*env, jobject unused) {+return socketOptionSupported(SOL_SOCKET,
SO_INCOMING_NAPI_ID);+}


On Wed, Apr 29, 2020 at 11:44 PM Ivanov, Vladimir A <
vladimir.a.iva...@intel.com> wrote:

> Updated version of the webrev available as
> http://cr.openjdk.java.net/~sviswanathan/Vladimir/8243099/webrev.03/
>
> The tests run results for the " test/jdk/java/net
> test/jdk/java/nio/channels test/jdk/javax/net test/jdk/jdk/net
> test/jdk/sun/net" same as for non-patched version.
>
> Changes:
> 1. comment was updated
> 2. names were adjusted with convention (IncomingNapiIdOptSupported  =>
> incomingNapiIdOptSupported, IncomingNapiIdSupported =>
> IncomingNapiIdSupported);
>
>  Thanks, Vladimir
>
> -Original Message-
> From: Alan Bateman 
> Sent: Wednesday, April 29, 2020 8:54 AM
> To: Ivanov, Vladimir A ; OpenJDK Network Dev
> list 
> Subject: Re: RFR 15 8243099: Adding ADQ support to JDK
>
> On 28/04/2020 01:33, Ivanov, Vladimir A wrote:
> > The changes for ADq doc update that includes explanation for this
> technology:
> > /**
> >   * ADQ (Application Device Queues) is an open technology to help
> > address network traffic challenges by improving application
> > * throughput and latency, and, most importantly, by enabling greater
> > predictability in application response times by creating
> > * application specific traffic queuing and steering.
> > *
> > * Modern Network Interface Card (NIC) devices have multiple queues or
> > channels to transmit and receive Network packets. ADQ
> > * lets each software application reserve subset of these device
> > queues, so that all the traffic, and only the traffic, belonging to
> > the
> > * application is directed to those set of queues avoiding sharing or
> > competing for network resources with other applications in the
> > * system.
> > *
> > * System Administrator allocates Network resources to an application
> > in a multi-application environment including set of device
> > * queues (Number of Tx/RX queue pairs), dedicated for application
> > traffic. In addition, QoS attributes can be applied to these
> > * Application Queues such as Network bandwidth limits & Traffic
> > priority. Device then assigns the incoming application connections
> > * to these set of queues by defined policies such as round robin. ADQ
> > provides hints to applications, by means of a new socket option
> > * SO_INCOMING_NAPI_ID, to indicate the device queue from the set of
> > assigned queues, to which an incoming socket connection and
> > * packets for that connection are directed to.
> > *
> > * For a multi-threaded application to take advantage of these
> > dedicated queues, application needs ability to query which queue a
> > * connection is assigned to, so all socket connections from a specific
> device queue can be serviced by a single application thread.
> > * Application then may utilize busy polling to receive and transmit
> > the network packets, minimizing system interrupts and context
> > * switches to better align overall system resources to applications.
> > This helps in improved predictability, reduce latency and improve
> > * throughput for ADQ enabled Application.
> > *
> > * The value of this SO_INCOMING_NAPI_ID socket option is an Integer,
> > uniquely representing the network device queue on which the
> > * last packet of the socket connection is received on. This option is
> > available for both stream-oriented and datagram-oriented sockets
> > * on both client and server sockets. The value returned for this
> > socket option will be zero until the socket is connected and has
> > received
> > * a network packet, at which point it will be a non-zero integer
> > value. This is a read only option. Attempting to set the socket option
> > will
> > * cause UOE to be thrown.
> > *
>
> What would you think about starting with something like"
>
> "Identifies the receive queue that the last incoming packet for the socket
> was received on.
>
> The value of this socket option is an Integer that identifies a receive
> queue that can application can use to split the incoming flows among
> threads based on the receive queue.
>
> The socket option is read-only and any attempt to set the socket option
> will throw UnsupportedOperationException.
> The socket option is supported by both stream and datagram oriented
> sockets. The value of the socket option is 0 when the socket is not bound
> or a packet has not been received."
>
> We could add an additional note to explain further how it might work on
> Linux that would use some of the text that you have in your mail.
>
> -Alan.
>
>

-- 
Thanks,
Vyom

Re: RFR[8242885]: 'PlainDatagramSocketImpl doesn’t allow for the sending of IPv6 datagrams on macOS with sizes between 65508-65527 bytes'

[Vyom Tiwari]

Looks good to me, minor comment can you please use some int constant  in
place of (65527, 65507) ?.
Vyom

On Fri, May 8, 2020 at 7:19 PM Patrick Concannon <
patrick.concan...@oracle.com> wrote:

> Hi,
>
> Could someone please review my fix for JDK-8242885
> 'PlainDatagramSocketImpl doesn’t allow for the sending of IPv6 datagrams
> on macOS with sizes between 65508-65527 bytes'?
>
> This fix changes the current max size for IPv6 datagrams on macOS from
> it's current size of 65507, which is the IPv4 limit, to65527, the actual
> limit for IPv6 on macOS. This will get around the issue of not being
> able to send datagrams with sizes between65508-65527 bytes.
>
> bug: https://bugs.openjdk.java.net/browse/JDK-8242885
> webrev: http://cr.openjdk.java.net/~pconcannon/8242885/webrevs/webrev.00/
>
>
> Kind regards,
>
> Patrick
>
>

-- 
Thanks,
Vyom

Re: RFR 15 8243099: SO_INCOMING_NAPI_ID support

[Vyom Tiwari]

Hi Vladimir,

Latest changes looks good to me, but i am not 100% sure if we need to
distinguish between "read only socket option" and "socket option not
supported" as below

if (!incomingNapiIdOptSupported)+throw new
UnsupportedOperationException("Attempt to set unsupported option " +
option);+else+throw new
SocketException("Attempt to set read only option " + option);

If developer wants to know the supported socket options he can use
"supportedOptions"  to get the list of supported socket options.

Thanks,

Vyom


On Tue, May 12, 2020 at 5:35 AM Ivanov, Vladimir A <
vladimir.a.iva...@intel.com> wrote:

> Thanks a lot Patrick. Your tests looks better then proposed ones.
> Updated webrev available as
> http://cr.openjdk.java.net/~sviswanathan/Vladimir/8243099/webrev.12
>
>  Thanks, Vladimir
>
> -Original Message-
> From: Patrick Concannon 
> Sent: Monday, May 11, 2020 11:11 AM
> To: Ivanov, Vladimir A ; Alan Bateman <
> alan.bate...@oracle.com>; OpenJDK Network Dev list <
> net-dev@openjdk.java.net>
> Subject: Re: RFR 15 8243099: SO_INCOMING_NAPI_ID support
>
> Hi Vladamir,
>
> Just a few observations with your test, ExtOptionNAPITest: I don't think
> the static class TestThread is needed for what you're trying to check and I
> think you can remove it. Also, I think using testNG assertions rather than
> throwing RunTimeExceptions might be better here, for example:
>
> -if (ssId != 0)
> -throw new RuntimeException("ServerSocket: incorrect value
> for SO_INCOMING_NAPI_ID: " + ssId);
> +assertEquals(ssID, 0, "Socket: Server");
>
> Finally, it might be a nice idea to split the test in two: one for
> DatagramSocket/DatagramChannel and the other for Sockets? -- for
> example, http://cr.openjdk.java.net/~pconcannon/8243099/webrevs/webrev.00/
>
>
> Kind regards,
>
> Patrick
>
> On 08/05/2020 20:02, Ivanov, Vladimir A wrote:
> > Thanks a lot. Updated webrev uploaded as
> http://cr.openjdk.java.net/~sviswanathan/Vladimir/8243099/webrev.10/
> > If no other comments the CSR will be crated on the next week.
> >
> >   Thanks, Vladimir
> >
> > -Original Message-
> > From: Alan Bateman 
> > Sent: Friday, May 8, 2020 12:10 AM
> > To: Ivanov, Vladimir A ; OpenJDK Network
> Dev list 
> > Subject: Re: RFR 15 8243099: SO_INCOMING_NAPI_ID support
> >
> > On 07/05/2020 19:51, Ivanov, Vladimir A wrote:
> >> In my case for 2 servers with RHEL8.1 the NapiId was non-zero for the
> DatagramSocket after the 'receive' call.
> >>
> > Thanks for checking. I tried the equivalent of RHEL7.6 and it
> consistently returns 0 for UDP sockets so they may be kernel differences
> that explain this.
> >
> > I took the liberty of tweaking the javadoc to allow for a bit more
> flexibility as to reasons why the socket option value may be 0. This allows
> us to drop the distinction between connecting and listing sockets. If you
> are okay with this text then let's give it a day or two to see if there are
> other comments before Sandhya submits the CSR.
> >
> > -Alan
> >
> >
> >   /**
> >* Identifies the receive queue that the last incoming packet for
> the socket
> >* was received on.
> >*
> >*  The value of this socket option is a positive {@code
> Integer} that
> >* identifies a receive queue that the application can use to
> split the
> >* incoming flows among threads based on the queue identifier. The
> value is
> >* {@code 0} when the socket is not bound, a packet has not been
> received,
> >* or more generally, when there is no receive queue to identify.
> > The socket
> >* option is supported by both stream-oriented and
> datagram-oriented
> >* sockets.
> >*
> >*  The socket option is read-only and an attempt to set the
> socket option
> >* will throw {@code SocketException}.
> >*
> >* @apiNote
> >* Network devices may have multiple queues or channels to
> transmit and receive
> >* network packets. The {@code SO_INCOMING_NAPI_ID} socket option
> provides a hint
> >* to the application to indicate the receive queue on which an
> incoming socket
> >* connection or packets for that connection are directed to. An
> application may
> >* take advantage of this by handling all socket connections
> assigned to a
> >* specific queue on one thread.
> >*
> >* @since 15
> >*/
>


-- 
Thanks,
Vyom

Re: RFR 15 8243099: SO_INCOMING_NAPI_ID support

[Vyom Tiwari]

Thanks for the clarification Alan. I think, i missed some part of
discussion.
Vyom

On Tue, May 12, 2020 at 1:20 PM Alan Bateman 
wrote:

> On 12/05/2020 08:34, Vyom Tiwari wrote:
>
> Hi Vladimir,
>
> Latest changes looks good to me, but i am not 100% sure if we need to
> distinguish between "read only socket option" and "socket option not
> supported" as below
>
> if (!incomingNapiIdOptSupported)+throw new 
> UnsupportedOperationException("Attempt to set unsupported option " + 
> option);+else+throw new 
> SocketException("Attempt to set read only option " + option);
>
>
> If developer wants to know the supported socket options he can use 
> "supportedOptions"  to get the list of supported socket options.
>
> You might need to go back through the full discussion thread on this
> point. The concern is that supportedOptions returns a set of socket
> options, no notion or distinction between read-only, write-only, or
> read-write options. Having setOption throw UOE when the socket option is
> supported could be confusing. We decided, in the discussion here, to align
> it with the other socket options that throw SocketException when they can
> not be set.
>
> -Alan
>


-- 
Thanks,
Vyom

Re: RFR: 8244958: preferIPv4Stack and preferIPv6Addresses do not affect addresses returned by HostsFileNameService

[Vyom Tiwari]

Hi Aleksei,

Latest webrev(01) looks good to me, one minor comment "PREFER_IPV4_STACK"
variable naming convention is not consistent  with other similar
variables(preferIPv6Address) in InetAddress.java.

Thanks,
Vyom

On Mon, May 25, 2020 at 4:18 PM Aleks Efimov 
wrote:

> Hi Alan, Daniel,
>
> Thank you for looking into this change. I've cleaned-up the fix and the
> test according to your comments.
> Modified fix can be viewed here:
> http://cr.openjdk.java.net/~aefimov/8244958/01
>
> Kind Regards,
> Aleksei
>
> On 24/05/2020 08:12, Alan Bateman wrote:
> > On 22/05/2020 16:45, Aleks Efimov wrote:
> >> Hi,
> >>
> >> The "java.net.preferIPv4Stack" and "java.net.preferIPv6Addresses"
> >> system properties do not affect the addresses and their order,
> >> returned by the HostsFileNameService provider that can be created by
> >> specifying "jdk.net.hosts.file" system property.
> >> The following fix analyses the system properties and
> >> re-orders/filters the returned IP addresses, if needed. Plus, small
> >> clean-up changes:
> >> http://cr.openjdk.java.net/~aefimov/8244958/00
> > The approach looks okay but I agree with Daniel on consistent naming
> > and cleanup.
> >
> > Another one to look is arrangeAddresses/appendAddresses as it's
> > surprising to mutate inet4Addresses or inet6Addresses with addresses
> > of the other type. It would be much cleaner to return a List
> > (inetAddress or a new List), then invoke toArray on the result and
> > this will avoid calling toArray in 3 places. If you using List in the
> > signatories then it will also be a lot cleaned as the caller should
> > not need to provide a specific type of List.
> >
> > Minor nit in the test but PREFER4_SP_VALUE and PREFER6_SP_VALUE can be
> > renamed to make the usages in the test easier to read. Also good to
> > add "_LIST" to IPV4_THEN_IPV6 to make it clearer at the use sites that
> > they are dealing with Lists. I think getExpectedAddressesArray would
> > be a bit clear if you assigned the result of the switch expression to
> > a variable, then return list.toArray(String::new).
> >
> > -Alan
>
>

-- 
Thanks,
Vyom

Re: RFR[8244582]: 'Remove terminally deprecated Solaris-specific SO_FLOW_SLA socket option'

[Vyom Tiwari]

Hi Patrick,

Changes looks good to me.

Thanks,
Vyom

On Wed, May 27, 2020 at 9:12 PM Patrick Concannon <
patrick.concan...@oracle.com> wrote:

> Hi,
>
> Could someone please review my webrev and CSR for JDK-8244582 'Remove
> terminally deprecated Solaris-specific SO_FLOW_SLA socket option'?
>
> This patch removes `ExtendedSocketOptions. SO_FLOW_SLA`, `SocketFlow`
> and `SocketFlow.Status` which were terminally deprecated in JDK 14 in
> preparation for the removal of the Solaris port. JEP 381 will remove
> this port in JDK 15 so the socket option and its supporting classes can
> now be removed.
>
>
> bug: https://bugs.openjdk.java.net/browse/JDK-8244582
> csr: https://bugs.openjdk.java.net/browse/JDK-8245984
> webrev: http://cr.openjdk.java.net/~pconcannon/8245828/webrevs/webrev.00/
>
>
> Kind regards,
>
> Patrick
>
>

-- 
Thanks,
Vyom

Re: RFR 8237858: PlainSocketImpl.socketAccept() handles EINTR incorrectly

[Vyom Tiwari]

Hi Alan,

Please find the  latest webrev(
http://cr.openjdk.java.net/~vtewari/8237858/webrev0.3/index.html). I tried
to incorporate the suggestion given by you.I created the NativeThread class
which  has  functions that send signals to threads.

Note: I am facing some compilation issue when i am trying to build my
native code using JDK builds scripts, i am hoping someone will help to
resolve this issue. I manually build the native library and copy in the
required folder before running my tests.

Thanks,
Vyom

On Tue, Mar 17, 2020 at 12:33 PM Alan Bateman 
wrote:

>
>
> On 17/03/2020 05:21, Vyom Tiwari wrote:
> > Hi Daniel/Michael,
> >
> > Please find the latest
> > webrev(http://cr.openjdk.java.net/~vtewari/8237858/webrev0.2/index.html),
>
> > where i put a test case as well.
> >
> Can you look at make/test/JtregNativeJdk.gmk? That should give you ideas
> on how to develop tests that use JNI code. You should just need two
> native methods, one to call pthread_self, the other call pthread_kill.
> It could potentially use sun.nio.ch.NativeThread but we'll see how far
> you get first. Once you have the primitives it should be easy to create
> a test that signals a thread blocked in read, write or accept.
>
> -Alan
>


-- 
Thanks,
Vyom

Re: RFR 8237858: PlainSocketImpl.socketAccept() handles EINTR incorrectly

[Vyom Tiwari]

Hi,
Please find the latest webrev(
http://cr.openjdk.java.net/~vtewari/8237858/webrev0.4/index.html) where i
fix the build issue as well.

Thanks,
Vyom

On Wed, Jun 17, 2020 at 3:34 PM Vyom Tiwari  wrote:

> Hi Alan,
>
> Please find the  latest webrev(
> http://cr.openjdk.java.net/~vtewari/8237858/webrev0.3/index.html). I
> tried to incorporate the suggestion given by you.I created the NativeThread
> class which  has  functions that send signals to threads.
>
> Note: I am facing some compilation issue when i am trying to build my
> native code using JDK builds scripts, i am hoping someone will help to
> resolve this issue. I manually build the native library and copy in the
> required folder before running my tests.
>
> Thanks,
> Vyom
>
> On Tue, Mar 17, 2020 at 12:33 PM Alan Bateman 
> wrote:
>
>>
>>
>> On 17/03/2020 05:21, Vyom Tiwari wrote:
>> > Hi Daniel/Michael,
>> >
>> > Please find the latest
>> > webrev(http://cr.openjdk.java.net/~vtewari/8237858/webrev0.2/index.html),
>>
>> > where i put a test case as well.
>> >
>> Can you look at make/test/JtregNativeJdk.gmk? That should give you ideas
>> on how to develop tests that use JNI code. You should just need two
>> native methods, one to call pthread_self, the other call pthread_kill.
>> It could potentially use sun.nio.ch.NativeThread but we'll see how far
>> you get first. Once you have the primitives it should be easy to create
>> a test that signals a thread blocked in read, write or accept.
>>
>> -Alan
>>
>
>
> --
> Thanks,
> Vyom
>


-- 
Thanks,
Vyom

Re: RFR 8237858: PlainSocketImpl.socketAccept() handles EINTR incorrectly

[Vyom Tiwari]

Hi Daniel/Alan,

Thanks for the review , I was not sure how JVM/JDK handles the signals
that's why I installed my own signal handler. As you both pointed out, I
removed the custom signal handler in NativeThread.

Please find the latest webrev(
http://cr.openjdk.java.net/~vtewari/8237858/webrev0.5/index.html) where i
have incorporated most of the review comments.

Thanks,
Vyom

On Tue, Jun 23, 2020 at 5:47 PM Daniel Fuchs 
wrote:

> Hi Vyom,
>
> Just to let you know that the new SocketAcceptInterruptTest test
> seems to have some stability issues. Patrick imported your test
> and ran it on our test system.
>
> We got one crash (SIGSEV) on macOS with the first @run that uses
> the old impl, and a timeout with the second @run that uses the new
> impl.
>
> I see that your test installs a signal handler that calls
> pthread_exit((void *)0); Is that really necessary?
> I am suspecting that this might be at the root cause of the crash.
>
>  From the test output:
>
> run main/othervm/native -Djdk.net.usePlainSocketImpl=true
> SocketAcceptInterruptTest
> --System.out:(20/1144)--
> Sending SIGPIPE to ServerSocket thread.
> Sending close Signal to server thread...
> #
> # A fatal error has been detected by the Java Runtime Environment:
> #
> #  SIGSEGV (0xb) at pc=0x00010399ac75, pid=30406, tid=12803
> #
> # JRE version: Java(TM) SE Runtime Environment (16.0) (build
> 16-internal+0-2020-06-22-1056440.patrick.concannon.openJDK)
> # Java VM: Java HotSpot(TM) 64-Bit Server VM
> (16-internal+0-2020-06-22-1056440.patrick.concannon.openJDK, mixed mode,
> sharing, tiered, compressed oops, g1 gc, bsd-amd64)
> # Problematic frame:
> # V  [libjvm.dylib+0x39ac75]
> _ZNK5frame28sender_for_interpreter_frameEP11RegisterMap+0x15
> #
>
> and in the hs_err file:
>
> ---  T H R E A D  ---
>
> Current thread (0x7fe72b523270):  GCTaskThread "GC Thread#0" [stack:
> 0x76bc8000,0x76cc8000] [id=12803]
>
> Stack: [0x76bc8000,0x76cc8000],  sp=0x76cc6a60,
> free space=1018k
> Native frames: (J=compiled Java code, A=aot compiled Java code,
> j=interpreted, Vv=VM code, C=native code)
> V  [libjvm.dylib+0x39ac75]
> _ZNK5frame28sender_for_interpreter_frameEP11RegisterMap+0x15
> V  [libjvm.dylib+0x39a99d]  _ZNK5frame6senderEP11RegisterMap+0x4d
> V  [libjvm.dylib+0x997f4e]
> _ZN10JavaThread11nmethods_doEP15CodeBlobClosure+0x9e
> V  [libjvm.dylib+0x99973c]
> _ZN7Threads28possibly_parallel_threads_doEbP13ThreadClosure+0x8c
> V  [libjvm.dylib+0x8b5c59]  _ZN21ParallelSPCleanupTask4workEj+0x29
> V  [libjvm.dylib+0xa3cc1d]  _ZN10GangWorker4loopEv+0x4d
> V  [libjvm.dylib+0x992ad1]  _ZN6Thread8call_runEv+0x71
> V  [libjvm.dylib+0x825177]  _ZL19thread_native_entryP6Thread+0x197
> C  [libsystem_pthread.dylib+0x3661]  _pthread_body+0x154
> C  [libsystem_pthread.dylib+0x350d]  _pthread_body+0x0
> C  [libsystem_pthread.dylib+0x2bf9]  thread_start+0xd
>
>
> siginfo: si_signo: 11 (SIGSEGV), si_code: 1 (SEGV_MAPERR), si_addr:
> 0x77dfa720
>
> ===
>
> For the second @run - the test failed in timeout:
>
> run main/othervm/native SocketAcceptInterruptTest
> --System.out:(3/107)--
> Sending SIGPIPE to ServerSocket thread.
> Sending close Signal to server thread...
> Timeout refired 480 times
>
> best regards,
>
> -- daniel
>
> On 18/06/2020 09:02, Vyom Tiwari wrote:
> > Hi,
> > Please find the latest
> > webrev(http://cr.openjdk.java.net/~vtewari/8237858/webrev0.4/index.html)
>
> > where i fix the build issue as well.
> >
> > Thanks,
> > Vyom
> >
>
>

-- 
Thanks,
Vyom

Re: RFR 8237858: PlainSocketImpl.socketAccept() handles EINTR incorrectly

[Vyom Tiwari]

Hi Alan,
thanks for the review, i think there is some problem with webrev, in my
local repo linux_close.c, bsd_close.c both are properly formatted.

I will do the suggested changes in the tests and  will send the updated
webrev soon.

Thanks,
Vyom

On Wed, Jun 24, 2020 at 11:49 AM Alan Bateman 
wrote:

> On 24/06/2020 05:53, Vyom Tiwari wrote:
> > Hi Daniel/Alan,
> >
> > Thanks for the review , I was not sure how JVM/JDK handles the signals
> > that's why I installed my own signal handler. As you both pointed out,
> > I removed the custom signal handler in NativeThread.
> >
> > Please find the latest
> > webrev(http://cr.openjdk.java.net/~vtewari/8237858/webrev0.5/index.html)
>
> > where i have incorporated most of the review comments.
> >
> This iteration is still missing the formatting fixes to linux_close.c
> and bsd_close.c. Also NativeSignal still has hardcoded values for the
> signals. NativeThread.close signals, it doesn't close, so needs to be
> renamed. I've skimmed through the updated SocketXXXInterruptTests but I
> don't think they are ready for review yet, e.g. I would expect
> SocketAcceptInterruptTest to test both timed and untimed accept. Also I
> would expect the test would establish a connection so that it verifies
> that a connection can be accept while until a barrage of signals. Also
> the cleanup is not clear, it looks like it seems the Server thread
> behind (this may be masked by running it with /othervm).
>
> -Alan
>


-- 
Thanks,
Vyom

Re: RFR 8237858: PlainSocketImpl.socketAccept() handles EINTR incorrectly

[Vyom Tiwari]

Hi Daniel,

Thanks for review, please find my answers below.

1-> What is the purpose of calling NativeThread.close?
To close the thread which is waiting in SocketAccept,  I wrote a test in
such a way that the server thread will wait infinitely in socket accept and
there will not be any incoming socket so accept will never return. As Alan
suggested we should have an incoming connection to test if accept works
after SIGPIPE.  I will change the test as per his suggestion and I think we
don't need the "close" method.

2-> When does the test succeed?
test succeeds if no SocketTimeoutException is thrown when you send a
SIGPIPE to the server thread. If you run the same test without a fix, the
test will throw a SocketTimeoutException.
#
Sending SIGPIPE to ServerSocket thread.
Exception in thread "Thread-0" java.lang.RuntimeException:
java.net.SocketTimeoutException: Accept timed out
at SocketAcceptTest$Server.run(SocketAcceptTest.java:89)
at java.base/java.lang.Thread.run(Thread.java:832)
Caused by: java.net.SocketTimeoutException: Accept timed out
at java.base/java.net.PlainSocketImpl.socketAccept(Native Method)


3-> Looking at SocketAcceptInterruptTest it looks like the test
will succeed if it manages to send the signal - and that's it.
Is it really a measure of success?
No, I explained in  question 2.

Use of  CountdownLatch  will be good, let me rewrite the test I will try to
use if it is possible.

Thanks,
Vyom


On Wed, Jun 24, 2020 at 4:11 PM Daniel Fuchs 
wrote:

> Hi Vyom,
>
> What is the purpose of calling NativeThread.close?
> Is that just to clean up and make the server thread terminate?
> If so there should be better ways to do that.
>
> When does the test succeed?
>
> Looking at SocketAcceptInterruptTest it looks like the test
> will succeed if it manages to send the signal - and that's it.
> Is it really a measure of success?
>
> Also it seems like better synchronization primitive could
> be used (for instance CountDownLatch) than relying on arbitrary
> timeout. In particular `isSet` could be a CountDownLatch.
>
> best regards,
>
> -- daniel
>
> On 24/06/2020 05:53, Vyom Tiwari wrote:
> > Hi Daniel/Alan,
> >
> > Thanks for the review , I was not sure how JVM/JDK handles the signals
> > that's why I installed my own signal handler. As you both pointed out, I
> > removed the custom signal handler in NativeThread.
> >
> > Please find the latest
> > webrev(http://cr.openjdk.java.net/~vtewari/8237858/webrev0.5/index.html)
>
> > where i have incorporated most of the review comments.
> >
> > Thanks,
> > Vyom
>


-- 
Thanks,
Vyom

Re: RFR 8237858: PlainSocketImpl.socketAccept() handles EINTR incorrectly

[Vyom Tiwari]

let me refactor the test, i will try to incorporate your suggestion.
Vyom


On Wed, Jun 24, 2020 at 5:21 PM Daniel Fuchs 
wrote:

> On 24/06/2020 12:14, Vyom Tiwari wrote:
> > 3-> Looking at SocketAcceptInterruptTest it looks like the test
> > will succeed if it manages to send the signal - and that's it.
> > Is it really a measure of success?
> > No, I explained in  question 2.
> >
>
> So basically you rely on the test infrastructure having
> registered an UncaughtExceptionHandler to catch the exception
> thrown by your server thread.
>
> I would prederably not rely on this, and instead check whether
> the exception has been thrown by the server thread from
> the main thread.
>
> best regards,
>
> -- daniel
>


-- 
Thanks,
Vyom

Re: RFR 8237858: PlainSocketImpl.socketAccept() handles EINTR incorrectly

[Vyom Tiwari]

Hi Daniel/Alan,
Please find the latest webrev(
http://cr.openjdk.java.net/~vtewari/8237858/webrev0.6/index.html).
Thanks,
Vyom

On Wed, Jun 24, 2020 at 5:32 PM Vyom Tiwari  wrote:

> let me refactor the test, i will try to incorporate your suggestion.
> Vyom
>
>
> On Wed, Jun 24, 2020 at 5:21 PM Daniel Fuchs 
> wrote:
>
>> On 24/06/2020 12:14, Vyom Tiwari wrote:
>> > 3-> Looking at SocketAcceptInterruptTest it looks like the test
>> > will succeed if it manages to send the signal - and that's it.
>> > Is it really a measure of success?
>> > No, I explained in  question 2.
>> >
>>
>> So basically you rely on the test infrastructure having
>> registered an UncaughtExceptionHandler to catch the exception
>> thrown by your server thread.
>>
>> I would prederably not rely on this, and instead check whether
>> the exception has been thrown by the server thread from
>> the main thread.
>>
>> best regards,
>>
>> -- daniel
>>
>
>
> --
> Thanks,
> Vyom
>


-- 
Thanks,
Vyom

Re: [8u] RFR(m): 8194298: Add support for per Socket configuration of TCP keepalive

[Vyom Tiwari]

Hi Bernd,

When we added these socket options to JDK, that time these options were not
available on Windows.

Thanks,
Vyom


On Fri, Jun 26, 2020 at 7:12 AM Bernd Eckenfels 
wrote:

> Hello,
>
> This would be a great addition. I do not understand why it does not
> support the options available for Windows. Especially given the fact that
> it actually implements 6 native methods to print "Unsupported".
>
> But I guess that's less a question to the backport and more to the general
> implementation.
>
> Gruss
> Bernd
>
>
> --
> http://bernd.eckenfels.net
> --
> *Von:* net-dev  im Auftrag von Severin
> Gehwolf 
> *Gesendet:* Thursday, June 25, 2020 3:39:38 PM
> *An:* jdk8u-dev 
> *Cc:* net-dev 
> *Betreff:* [8u] RFR(m): 8194298: Add support for per Socket configuration
> of TCP keepalive
>
> Hi,
>
> Please review this OpenJDK 8u backport of JDK-8194298 which adds socket
> configuration options for TCP keepalive. It's one of the Oracle JDK
> parity patches.
>
> As with the OpenJDK 11u change, this includes Linux and Mac only
> changes. This backport is pretty much a rewrite as the JDK 11u codebase
> is rather different in this area.
>
> Bug:https://bugs.openjdk.java.net/browse/JDK-8194298
> 8u CSR: https://bugs.openjdk.java.net/browse/JDK-8236187
> webrev:
> http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8194298/jdk8/04/webrev/
>
> Testing:
>  * Build/test on Linux x86_64 and Mac OS x (thanks to Simon Tooke).
>Build on Windows x86_64 (also thanks to Simon Tooke)
>  * tier1 tests, test/java/nio test/java/net/ test/jdk/net/ on Linux
>x86_64. No regressions noted.
>
> Thoughts?
>
> Thanks,
> Severin
>
>
>

-- 
Thanks,
Vyom

Re: [8u] RFR(m): 8194298: Add support for per Socket configuration of TCP keepalive

[Vyom Tiwari]

Hi Severin,

Overall code changes looks ok to me, I build & tested at my local REL it
worked fine for me.

Below are few minor comments.

1-> Net.java
1.1-> I think you don't need to explicitly convert value to integer and
then pass it. You can avoid the local int variable creation as follows.
  ExtendedOptionsImpl.setTcpKeepAliveIntvl(fd, (Integer)value);
   1.2-> In getSocketOption you don't need to explicitly cast it to Integer.
 return ExtendedOptionsImpl.getTcpKeepAliveIntvl(fd);
2-> PlainSocketImpl.java
1.1 -> In setOption(SocketOption name, T value) you can avoid the
local int variable.
3-> Any specific reason for two set of functions "setTcpKeepAliveTime0
and setTcpKeepAliveTime"
for all three socket options ?

Thanks,
Vyom

On Fri, Jun 26, 2020 at 1:08 PM Severin Gehwolf  wrote:

> Hi,
>
> On Thu, 2020-06-25 at 23:55 +, Bernd Eckenfels wrote:
> > This would be a great addition.
>
> Thanks.
>
> > I do not understand why it does not support the options available for
> > Windows. Especially given the fact that it actually implements 6
> > native methods to print "Unsupported".
> >
> > But I guess that's less a question to the backport and more to the
> > general implementation.
>
> Yes, indeed. This should be brought up for discussion in JDK head first
> and implemented there before we can consider a backport.
>
> Thanks,
> Severin
>
>
>

-- 
Thanks,
Vyom

Re: RFR 8237858: PlainSocketImpl.socketAccept() handles EINTR incorrectly

[Vyom Tiwari]

Hi Alan,

Thanks for the review comment, we can send multiple signals for sure. I
will create a  thread which will send multiple signals.

Creating a new native method  that returns a signal is a bit too much as
SIGPIPE is a standard signal, but I will add a native method as you
suggested.

I think webrev is ignoring the spaces that is why you are observing the
formatting issues in (linux/bsd)_close.c files. In my local repo code looks
well formatted.

Thanks,
Vyom

On Sat, Jun 27, 2020 at 11:46 AM Alan Bateman 
wrote:

> On 26/06/2020 05:17, Vyom Tiwari wrote:
> > Hi Daniel/Alan,
> > Please find the latest
> > webrev(http://cr.openjdk.java.net/~vtewari/8237858/webrev0.6/index.html
> ).
> >
> I think we can do better than one signal after 200ms. Have you given any
> thought to have a thread that signals many times so that you have a
> better chance of tickling bugs?  One other comment at this time is that
> the value of SIGPIPE is still hardcoded - I think you'll need to add
> another native method to return its value.
>
> BTW:  The formatting issues with linux_close.c and bsd_close.c are the
> same as the previous iterations. Did you track down that issue, I think
> you thought it was webrev or an issue with your patch queue.
>
> -Alan.
>
>

-- 
Thanks,
Vyom

Re: RFR 8237858: PlainSocketImpl.socketAccept() handles EINTR incorrectly

[Vyom Tiwari]

Hi Alan,Daniel,

Please find the latest webrev(
http://cr.openjdk.java.net/~vtewari/8237858/webrev0.7/index.html). I have
added a new native method and removed the hardcoding(SIGPIPE).

I  gave a thought on creating a separate thread and sending a signal but it
will further increase the complexity of the test. If tests send signals in
separate threads  then  tests have to make sure that server thread is alive
and running.

Thanks,
Vyom

On Sat, Jun 27, 2020 at 12:56 PM Vyom Tiwari  wrote:

> Hi Alan,
>
> Thanks for the review comment, we can send multiple signals for sure. I
> will create a  thread which will send multiple signals.
>
> Creating a new native method  that returns a signal is a bit too much as
> SIGPIPE is a standard signal, but I will add a native method as you
> suggested.
>
> I think webrev is ignoring the spaces that is why you are observing the
> formatting issues in (linux/bsd)_close.c files. In my local repo code looks
> well formatted.
>
> Thanks,
> Vyom
>
> On Sat, Jun 27, 2020 at 11:46 AM Alan Bateman 
> wrote:
>
>> On 26/06/2020 05:17, Vyom Tiwari wrote:
>> > Hi Daniel/Alan,
>> > Please find the latest
>> > webrev(http://cr.openjdk.java.net/~vtewari/8237858/webrev0.6/index.html
>> ).
>> >
>> I think we can do better than one signal after 200ms. Have you given any
>> thought to have a thread that signals many times so that you have a
>> better chance of tickling bugs?  One other comment at this time is that
>> the value of SIGPIPE is still hardcoded - I think you'll need to add
>> another native method to return its value.
>>
>> BTW:  The formatting issues with linux_close.c and bsd_close.c are the
>> same as the previous iterations. Did you track down that issue, I think
>> you thought it was webrev or an issue with your patch queue.
>>
>> -Alan.
>>
>>
>
> --
> Thanks,
> Vyom
>


-- 
Thanks,
Vyom

Re: [8u] RFR(m): 8194298: Add support for per Socket configuration of TCP keepalive

[Vyom Tiwari]

Hi Severin,

Latest code looks good, I think in
src/solaris/native/java/net/ExtendedOptionsImpl.c, you don't need #ifdef
blocks. In case of "flowOption" it was required because flow is specific to
Solaris.

In case of KEEPALIVE we are using the POSIX api(setsockopt/getsockopt)
which exists on all POSIX OS's. If a  OS does not support KEEPALIVE then

Java_sun_net_ExtendedOptionsImpl_keepAliveOptionsSupported will return
false and #else will never execute.

For Windows we have different files and for all other platforms same
code will work without explicit #ifdef.

Please do let me know if i am missing something.

Thanks,

Vyom



On Mon, Jun 29, 2020 at 2:25 PM Severin Gehwolf  wrote:

> Hi Vyom!
>
> On Fri, 2020-06-26 at 15:55 +0530, Vyom Tiwari wrote:
> > Hi Severin,
> >
> > Overall code changes looks ok to me, I build & tested at my local REL
> > it worked fine for me.
>
> Thanks for the review!
>
> > Below are few minor comments.
> >
> > 1-> Net.java
> > 1.1-> I think you don't need to explicitly convert value to integer
> and then pass it. You can avoid the local int variable creation as follows.
> >   ExtendedOptionsImpl.setTcpKeepAliveIntvl(fd, (Integer)value);
> >1.2-> In getSocketOption you don't need to explicitly cast it to
> Integer.
> >  return ExtendedOptionsImpl.getTcpKeepAliveIntvl(fd);
> > 2-> PlainSocketImpl.java
> > 1.1 -> In setOption(SocketOption name, T value) you can avoid the
> local int variable.
>
> Should all be fixed.
>
> > 3-> Any specific reason for two set of functions "setTcpKeepAliveTime0
> and setTcpKeepAliveTime" for all three socket options ?
>
> Not really, other than keeping the backport aligned to the JDK 11u
> patch. I've updated it in the latest webrev:
> http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8194298/jdk8/05/webrev/
>
> Thanks,
> Severin
>
>
> > Thanks,
> > Vyom
> >
> > On Fri, Jun 26, 2020 at 1:08 PM Severin Gehwolf 
> wrote:
> > > Hi,
> > >
> > > On Thu, 2020-06-25 at 23:55 +, Bernd Eckenfels wrote:
> > > > This would be a great addition.
> > >
> > > Thanks.
> > >
> > > > I do not understand why it does not support the options available for
> > > > Windows. Especially given the fact that it actually implements 6
> > > > native methods to print "Unsupported".
> > > >
> > > > But I guess that's less a question to the backport and more to the
> > > > general implementation.
> > >
> > > Yes, indeed. This should be brought up for discussion in JDK head first
> > > and implemented there before we can consider a backport.
> > >
> > > Thanks,
> > > Severin
> > >
> > >
> >
> >
>
>

-- 
Thanks,
Vyom

Re: [8u] RFR(m): 8194298: Add support for per Socket configuration of TCP keepalive

[Vyom Tiwari]

leDesc);
+if (fd < 0) {
+NET_ERROR(env, JNU_JAVANETPKG "SocketException", "socket
closed");
+return;
+} else {
+jint rv = setsockopt(fd, SOCK_OPT_LEVEL, TCP_KEEPINTVL,
&optval, sizeof (optval));
+handleError(env, rv, "set option TCP_KEEPINTVL failed");
+}
+#else
+JNU_ThrowByName(env, "java/lang/UnsupportedOperationException",
"unsupported socket option");
+#endif
+}
+
+/*
+ * Class: sun_net_ExtendedOptionsImpl
+ * Method:getTcpKeepAliveProbes
+ * Signature: (Ljava/io/FileDescriptor;)I
+ */
+JNIEXPORT jint JNICALL
Java_sun_net_ExtendedOptionsImpl_getTcpKeepAliveProbes
+(JNIEnv *env, jobject unused, jobject fileDesc) {
+#if defined(__linux__) || defined(MACOSX)
+int fd = getFD(env, fileDesc);
+if (fd < 0) {
+NET_ERROR(env, JNU_JAVANETPKG "SocketException", "socket
closed");
+return -1;
+} else {
+jint optval, rv;
+socklen_t sz = sizeof (optval);
+rv = getsockopt(fd, SOCK_OPT_LEVEL, TCP_KEEPCNT, &optval, &sz);
+handleError(env, rv, "get option TCP_KEEPCNT failed");
+return optval;
+}
+#else
+JNU_ThrowByName(env, "java/lang/UnsupportedOperationException",
"unsupported socket option");
+#endif
+}
+
+/*
+ * Class: sun_net_ExtendedOptionsImpl
+ * Method:getTcpKeepAliveTime
+ * Signature: (Ljava/io/FileDescriptor;)I
+ */
+JNIEXPORT jint JNICALL Java_sun_net_ExtendedOptionsImpl_getTcpKeepAliveTime
+(JNIEnv *env, jobject unused, jobject fileDesc) {
+#if defined(__linux__) || defined(MACOSX)
+int fd = getFD(env, fileDesc);
+if (fd < 0) {
+NET_ERROR(env, JNU_JAVANETPKG "SocketException", "socket
closed");
+return -1;
+} else {
+jint optval, rv;
+socklen_t sz = sizeof (optval);
+rv = getsockopt(fd, SOCK_OPT_LEVEL, SOCK_OPT_NAME_KEEPIDLE,
&optval, &sz);
+handleError(env, rv, "get option " SOCK_OPT_NAME_KEEPIDLE_STR
" failed");
+return optval;
+}
+#else
+JNU_ThrowByName(env, "java/lang/UnsupportedOperationException",
"unsupported socket option");
+#endif
+}
+
+/*
+ * Class: sun_net_ExtendedOptionsImpl
+ * Method:getTcpKeepAliveIntvl
+ * Signature: (Ljava/io/FileDescriptor;)I
+ */
+JNIEXPORT jint JNICALL
Java_sun_net_ExtendedOptionsImpl_getTcpKeepAliveIntvl
+(JNIEnv *env, jobject unused, jobject fileDesc) {
+#if defined(__linux__) || defined(MACOSX)
+int fd = getFD(env, fileDesc);
+if (fd < 0) {
+NET_ERROR(env, JNU_JAVANETPKG "SocketException", "socket
closed");
+return -1;
+} else {
+jint optval, rv;
+socklen_t sz = sizeof (optval);
+rv = getsockopt(fd, SOCK_OPT_LEVEL, TCP_KEEPINTVL, &optval,
&sz);
+handleError(env, rv, "get option TCP_KEEPINTVL failed");
+return optval;
+}
+#else
+JNU_ThrowByName(env, "java/lang/UnsupportedOperationException",
"unsupported socket option");
+#endif
+}

On Mon, Jun 29, 2020 at 5:27 PM Severin Gehwolf  wrote:

> Hi Vyom,
>
> On Mon, 2020-06-29 at 17:11 +0530, Vyom Tiwari wrote:
> > Hi Severin,
> >
> > Latest code looks good
>
> Thanks!
>
> > I think in src/solaris/native/java/net/ExtendedOptionsImpl.c, you
> > don't need #ifdef blocks. In case of "flowOption" it was required
> > because flow is specific to Solaris.
> >
> > In case of KEEPALIVE we are using the POSIX
> > api(setsockopt/getsockopt) which exists on all POSIX OS's. If a  OS
> > does not support KEEPALIVE then
> > Java_sun_net_ExtendedOptionsImpl_keepAliveOptionsSupported will
> > return false and #else will never execute.
> > For Windows we have different files and for all other platforms same
> > code will work without explicit #ifdef.
>
> Not quite.
>
> For example, on MACOSX some macros have a diferent name than on Linux.
> Thus, the ifdef magic to work around that. I don't have any insight as
> to what they're called on, say, solaris or aix, or, if they're
> different at all. Anyway, I'd rely on somebody else doing the testing.
> Without that, I don't think we can add this to other platforms and
> potentially break them. Support for them could get added later if
> somebody with the relevant systems steps up to do it.
>
> > Please do let me know if i am missing something.
>
> FWIW, those options are only enabled on Linux/Mac for JDK 11u and
> jdk/jdk. Those changes would have to be d

Re: RFR 8237858: PlainSocketImpl.socketAccept() handles EINTR incorrectly

[Vyom Tiwari]

Hi Daniel,

Thanks for review please find the latest webrev(
http://cr.openjdk.java.net/~vtewari/8237858/webrev0.8/index.html). I fixed
the Windows build issue.

Thanks,
Vyom

On Mon, Jun 29, 2020 at 10:23 PM Daniel Fuchs 
wrote:

> Hi Vyom,
>
>
> On 29/06/2020 08:03, Vyom Tiwari wrote:
> > Hi Alan,Daniel,
> >
> > Please find the latest
> > webrev(http://cr.openjdk.java.net/~vtewari/8237858/webrev0.7/index.html).
> I
> > have added a new native method and removed the hardcoding(SIGPIPE).
>
> That fails to build on windows:
>
> [2020-06-29T15:26:16,899Z] ERROR: Build failed for target 'default
> (product-bundles test-bundles static-libs-bundles)' in configuration
> 'windows-x64' (exit code 2)
> [2020-06-29T15:26:17,587Z] * For target
> support_test_jdk_jtreg_native_support_libNativeThread_libNativeThread.obj:
> [2020-06-29T15:26:17,649Z] libNativeThread.c
> [2020-06-29T15:26:17,649Z]
> ./open/test/jdk/java/net/Socket/libNativeThread.c(26): fatal error
> C1083: Cannot open include file: 'pthread.h': No such file or directory
> [2020-06-29T15:26:17,743Z]... (rest of output omitted)
>
> I'll suggest to put the content of the native file under:
>
> #ifndef _WIN32
>
> or maybe
>
> #ifndef _WINDOWS
>
> You can probably use the `submit` repo to verify that there is no
> build break.
>
> > I gave a thought on creating a separate thread and sending a signal but
> > it will further increase the complexity of the test. If tests send
> > signals in separate threads  then  tests have to make sure that server
> > thread is alive and running.
>
> You can probably send from the main thread - just bail out if the
> server has closed ?
>
> while (!ss.isClosed() && i < 20) {
> ... send SIGPIPE and sleep  10ms ...
> }
>
> best regards,
>
> -- daniel
>
> >
> > Thanks,
> > Vyom
>
>

-- 
Thanks,
Vyom

Re: RFR 8237858: PlainSocketImpl.socketAccept() handles EINTR incorrectly

[Vyom Tiwari]

Hi Alan,

Thanks for the review, I  passed ss to sendSignal because in  failing cases
ss will throw an exception  on the first SIGPIPE signal so  i need to put a
check if the server socket is not closed.

Same reason for L64,in failing case server socket will be closed after
SIGPIPE. I will  address all your review comments.

Thanks,
Vyom

On Sat, Jul 4, 2020 at 11:41 AM Alan Bateman 
wrote:

> On 30/06/2020 06:39, Vyom Tiwari wrote:
> >
> >
> > Thanks for review please find the latest
> > webrev(http://cr.openjdk.java.net/~vtewari/8237858/webrev0.8/index.html).
>
> > I fixed the Windows build issue.
> >
> Minor nit but I assume build-dev will want to keep the indentation in
> TestFilesCompilation.gmk consistent. Also the patch still has the
> formatting issues in linux_close.c and bsd_close.c. You thought it might
> be webrev but looks to me that the issues are in the patch file too.
>
> The updated tests are looking better. A few comments on
> SocketAcceptInterruptTest, most apply to SocketReadInterruptTest too:
> - "service" isn't a great name for the Executor. Also you can make use
> of try-finally, e.g.
> ExecutorService executor = Executors.newFixedThreadPool(1);
> try { ... } finally { executor.shutdown(); }
> - you can also use try-with-resources for the ServerSocket and it will
> avoid checking if ss is nul
> - sendSignal - not clear why this needs the "ss" parameter, maybe to
> make the testing of the buggy implementation more robust?
> - I assume thet isClosed check at L64 can be removed too.
> - is Server.isSet needed - the getID method just needs to spin until
> threadId != 0.
>
> -Alan.
>


-- 
Thanks,
Vyom

Re: RFR 8237858: PlainSocketImpl.socketAccept() handles EINTR incorrectly

[Vyom Tiwari]

Hi Martin
Thanks for the review, I will try to address your review comment.

I wanted to write a simple test case for this issue but it is getting more
complex.

Thanks,
Vyom

On Sat, Jul 4, 2020 at 8:14 PM Martin Buchholz  wrote:

> On Fri, Jul 3, 2020 at 11:12 PM Alan Bateman 
> wrote:
>
> > - "service" isn't a great name for the Executor. Also you can make use
> > of try-finally, e.g.
> > ExecutorService executor = Executors.newFixedThreadPool(1);
> > try { ... } finally { executor.shutdown(); }
>
> If you want to do this structured-concurrency-style, you should wait
> for all the threads you started to complete (they might block!).
> shutdown is not enough - you also want awaitTermination.
>
> We have a PoolCleaner utility in test/jdk/java/util/concurrent/tck/ to
> help do this.
>
>  try (PoolCleaner cleaner = cleaner(p)) {
>


-- 
Thanks,
Vyom

Re: RFR 8237858: PlainSocketImpl.socketAccept() handles EINTR incorrectly

[Vyom Tiwari]

Hi All,

Please find the updated webrev(
http://cr.openjdk.java.net/~vtewari/8237858/webrev0.9/index.html). I leave
the idea of using the PoolCleaner.

Thanks,
Vyom

On Sat, Jul 4, 2020 at 9:08 PM Martin Buchholz  wrote:

> Right.  It would be a project to create a jtreg test utility inspired
> by PoolCleaner and use it in many tests.
>
> On Sat, Jul 4, 2020 at 8:24 AM Vyom Tiwari  wrote:
> >
> > Hi Martin
> > Thanks for the review, I will try to address your review comment.
> >
> > I wanted to write a simple test case for this issue but it is getting
> more complex.
> >
> > Thanks,
> > Vyom
> >
> > On Sat, Jul 4, 2020 at 8:14 PM Martin Buchholz 
> wrote:
> >>
> >> On Fri, Jul 3, 2020 at 11:12 PM Alan Bateman 
> wrote:
> >>
> >> > - "service" isn't a great name for the Executor. Also you can make use
> >> > of try-finally, e.g.
> >> > ExecutorService executor = Executors.newFixedThreadPool(1);
> >> > try { ... } finally { executor.shutdown(); }
> >>
> >> If you want to do this structured-concurrency-style, you should wait
> >> for all the threads you started to complete (they might block!).
> >> shutdown is not enough - you also want awaitTermination.
> >>
> >> We have a PoolCleaner utility in test/jdk/java/util/concurrent/tck/ to
> >> help do this.
> >>
> >>  try (PoolCleaner cleaner = cleaner(p)) {
> >
> >
> >
> > --
> > Thanks,
> > Vyom
>


-- 
Thanks,
Vyom

Re: RFR 8237858: PlainSocketImpl.socketAccept() handles EINTR incorrectly

[Vyom Tiwari]

Hi Patrick,
Thanks for testing, please find the latest webrev(
http://cr.openjdk.java.net/~vtewari/8237858/webrev1.0/index.html). I fixed
the windows build issue.
Thanks,
Vyom

On Tue, Jul 7, 2020 at 11:49 PM Patrick Concannon <
patrick.concan...@oracle.com> wrote:

> Hi Vyom,
>
> I imported your latest patch and ran it on our test system, and I noticed
> the following error on Windows:
>
> [2020-07-07T11:09:20,621Z]
> T:\workspace\open\test\jdk\java\net\Socket\libNativeThread.c(54) : error
> C2220: the following warning is treated as an error
> [2020-07-07T11:09:20,621Z]
> T:\workspace\open\test\jdk\java\net\Socket\libNativeThread.c(54) : warning
> C4716: 'Java_NativeThread_signal': must return a value
>
> Kind regards,
> Patrick
>
> On 7 Jul 2020, at 04:14, Vyom Tiwari  wrote:
>
> Hi All,
>
> Please find the updated webrev(
> http://cr.openjdk.java.net/~vtewari/8237858/webrev0.9/index.html). I
> leave the idea of using the PoolCleaner.
>
> Thanks,
> Vyom
>
> On Sat, Jul 4, 2020 at 9:08 PM Martin Buchholz 
> wrote:
>
>> Right.  It would be a project to create a jtreg test utility inspired
>> by PoolCleaner and use it in many tests.
>>
>> On Sat, Jul 4, 2020 at 8:24 AM Vyom Tiwari  wrote:
>> >
>> > Hi Martin
>> > Thanks for the review, I will try to address your review comment.
>> >
>> > I wanted to write a simple test case for this issue but it is getting
>> more complex.
>> >
>> > Thanks,
>> > Vyom
>> >
>> > On Sat, Jul 4, 2020 at 8:14 PM Martin Buchholz 
>> wrote:
>> >>
>> >> On Fri, Jul 3, 2020 at 11:12 PM Alan Bateman 
>> wrote:
>> >>
>> >> > - "service" isn't a great name for the Executor. Also you can make
>> use
>> >> > of try-finally, e.g.
>> >> > ExecutorService executor = Executors.newFixedThreadPool(1);
>> >> > try { ... } finally { executor.shutdown(); }
>> >>
>> >> If you want to do this structured-concurrency-style, you should wait
>> >> for all the threads you started to complete (they might block!).
>> >> shutdown is not enough - you also want awaitTermination.
>> >>
>> >> We have a PoolCleaner utility in test/jdk/java/util/concurrent/tck/ to
>> >> help do this.
>> >>
>> >>  try (PoolCleaner cleaner = cleaner(p)) {
>> >
>> >
>> >
>> > --
>> > Thanks,
>> > Vyom
>>
>
>
> --
> Thanks,
> Vyom
>
>
>

-- 
Thanks,
Vyom

Re: RFR 8237858: PlainSocketImpl.socketAccept() handles EINTR incorrectly

[Vyom Tiwari]

Hi Patrick,

Thanks for testing, Alan, Daniel can i get the final  review comment from
you both ?.

Thanks,
Vyom

On Wed, Jul 8, 2020 at 8:17 PM Patrick Concannon <
patrick.concan...@oracle.com> wrote:

> Hi,
>
> No problem.
>
> I ran our tests against your latest patch, and everything passed.
>
> Thanks Vyom.
>
> Kind regards,
> Patrick
>
> On 8 Jul 2020, at 06:49, Vyom Tiwari  wrote:
>
> Hi Patrick,
> Thanks for testing, please find the latest webrev(
> http://cr.openjdk.java.net/~vtewari/8237858/webrev1.0/index.html). I
> fixed the windows build issue.
> Thanks,
> Vyom
>
> On Tue, Jul 7, 2020 at 11:49 PM Patrick Concannon <
> patrick.concan...@oracle.com> wrote:
>
>> Hi Vyom,
>>
>> I imported your latest patch and ran it on our test system, and I noticed
>> the following error on Windows:
>>
>> [2020-07-07T11:09:20,621Z]
>> T:\workspace\open\test\jdk\java\net\Socket\libNativeThread.c(54) : error
>> C2220: the following warning is treated as an error
>> [2020-07-07T11:09:20,621Z]
>> T:\workspace\open\test\jdk\java\net\Socket\libNativeThread.c(54) : warning
>> C4716: 'Java_NativeThread_signal': must return a value
>>
>> Kind regards,
>> Patrick
>>
>> On 7 Jul 2020, at 04:14, Vyom Tiwari  wrote:
>>
>> Hi All,
>>
>> Please find the updated webrev(
>> http://cr.openjdk.java.net/~vtewari/8237858/webrev0.9/index.html). I
>> leave the idea of using the PoolCleaner.
>>
>> Thanks,
>> Vyom
>>
>> On Sat, Jul 4, 2020 at 9:08 PM Martin Buchholz 
>> wrote:
>>
>>> Right.  It would be a project to create a jtreg test utility inspired
>>> by PoolCleaner and use it in many tests.
>>>
>>> On Sat, Jul 4, 2020 at 8:24 AM Vyom Tiwari  wrote:
>>> >
>>> > Hi Martin
>>> > Thanks for the review, I will try to address your review comment.
>>> >
>>> > I wanted to write a simple test case for this issue but it is getting
>>> more complex.
>>> >
>>> > Thanks,
>>> > Vyom
>>> >
>>> > On Sat, Jul 4, 2020 at 8:14 PM Martin Buchholz 
>>> wrote:
>>> >>
>>> >> On Fri, Jul 3, 2020 at 11:12 PM Alan Bateman 
>>> wrote:
>>> >>
>>> >> > - "service" isn't a great name for the Executor. Also you can make
>>> use
>>> >> > of try-finally, e.g.
>>> >> > ExecutorService executor = Executors.newFixedThreadPool(1);
>>> >> > try { ... } finally { executor.shutdown(); }
>>> >>
>>> >> If you want to do this structured-concurrency-style, you should wait
>>> >> for all the threads you started to complete (they might block!).
>>> >> shutdown is not enough - you also want awaitTermination.
>>> >>
>>> >> We have a PoolCleaner utility in test/jdk/java/util/concurrent/tck/ to
>>> >> help do this.
>>> >>
>>> >>  try (PoolCleaner cleaner = cleaner(p)) {
>>> >
>>> >
>>> >
>>> > --
>>> > Thanks,
>>> > Vyom
>>>
>>
>>
>> --
>> Thanks,
>> Vyom
>>
>>
>>
>
> --
> Thanks,
> Vyom
>
>
>

-- 
Thanks,
Vyom

Re: RFR 8237858: PlainSocketImpl.socketAccept() handles EINTR incorrectly

[Vyom Tiwari]

Hi Alan,

thanks for the review, I will definitely fix any formatting issue before
pushing the patch. My local repo code is properly formatted and i was
suspecting that  webrev is ignoring the space  while generating the patch
file that's why you are seeing the formatting issue.

My local code is properly formatted  and i am assuming that while pushing
there will not be any formatting issue.

Please let me know if I am missing something.

thanks,
Vyom

On Wed, Jul 15, 2020 at 12:44 AM Alan Bateman 
wrote:

>
>
> On 14/07/2020 20:09, Daniel Fuchs wrote:
> > On 12/07/2020 07:36, Vyom Tiwari wrote:
> >> Hi Patrick,
> >>
> >> Thanks for testing, Alan, Daniel can i get the final  review comment
> >> from you both ?.
> >>
> >
> > Hi Vyom,
> >
> > http://cr.openjdk.java.net/~vtewari/8237858/webrev1.0/index.html
> Vyom - will you fix the formatting issues in linux_close.c and
> bsd_close.c before you push this? You mentioned that it's a webrev issue
> but I don't think so because they are in the patch file too.
>
> -Alan
>


-- 
Thanks,
Vyom

Re: RFR 8237858: PlainSocketImpl.socketAccept() handles EINTR incorrectly

[Vyom Tiwari]

Hi Daniel,

you are right, Alan is talking the same , in webrev that i have posted code
is not properly formatted.

In my local repo code is properly formatted.

Thanks,
Vyom

On Wed, Jul 15, 2020 at 2:55 PM Daniel Fuchs 
wrote:

> Hi Vyom,
>
> I don't know if that's what Alan is referring to but I see
> this:
>
> bsd_close.c:
>
> missing space just after `if` extra space just before `timeout`:
>   475 if( timeout > 0) {
>
> linux_close.c:
>
> missing space just after `if`:
>   440 if(timeout > 0) {
>
> missing space just after else:
>   447     }else {
>
> best regards,
>
> -- daniel
>
> On 15/07/2020 09:14, Vyom Tiwari wrote:
> > Hi Alan,
> >
> > thanks for the review, I will definitely fix any formatting issue before
> > pushing the patch. My local repo code is properly formatted and i was
> > suspecting that  webrev is ignoring the space  while generating the
> > patch file that's why you are seeing the formatting issue.
> >
> > My local code is properly formatted  and i am assuming that while
> > pushing there will not be any formatting issue.
> >
> > Please let me know if I am missing something.
> >
> > thanks,
> > Vyom
> >
> > On Wed, Jul 15, 2020 at 12:44 AM Alan Bateman  > <mailto:alan.bate...@oracle.com>> wrote:
> >
> >
> >
> > On 14/07/2020 20:09, Daniel Fuchs wrote:
> >  > On 12/07/2020 07:36, Vyom Tiwari wrote:
> >  >> Hi Patrick,
> >  >>
> >  >> Thanks for testing, Alan, Daniel can i get the final  review
> > comment
> >  >> from you both ?.
> >  >>
> >  >
> >  > Hi Vyom,
> >  >
> >  > http://cr.openjdk.java.net/~vtewari/8237858/webrev1.0/index.html
> > Vyom - will you fix the formatting issues in linux_close.c and
> > bsd_close.c before you push this? You mentioned that it's a webrev
> > issue
> > but I don't think so because they are in the patch file too.
> >
> > -Alan
> >
> >
> >
> > --
> > Thanks,
> > Vyom
>
>

-- 
Thanks,
Vyom

Re: JDK 16 RFR of JDK-8250244: Address reliance on default constructors in java.net

[Vyom Tiwari]

looks ok to me.
Vyom

On Fri, Jul 24, 2020 at 6:05 AM Joe Darcy  wrote:

> Hello,
>
> Please review the replacement of default constructors in various
> abstract classes in java.net with explicit constructors:
>
>  webrev: http://cr.openjdk.java.net/~darcy/8250244.0/
>  CSR: https://bugs.openjdk.java.net/browse/JDK-8250245
>
> (This is part of a larger effort to remove default constructor usage
> from the JDK in preparation for enabling a new lint warning.)
>
> Thanks,
>
> -Joe
>
>

-- 
Thanks,
Vyom

Re: RFR[8246164]: ‘SendDatagramToBadAddress.java and ChangingAddress.java should be changed to explicitly require the new DatagramSocket implementation’

[Vyom Tiwari]

Hi Patrick,
Changes looks ok to me.
Thanks,
Vyom

On Wed, Jul 29, 2020 at 9:45 PM Patrick Concannon <
patrick.concan...@oracle.com> wrote:

> Hi,
>
> Could someone please review my fix for JDK-8246164 —
> ‘SendDatagramToBadAddress.java and ChangingAddress.java should be changed
> to explicitly require the new DatagramSocket implementation’.
>
> The issues addressed by the tests
> `java/net/DatagramSocket/SendDatagramToBadAddress.java` and
> `java/nio/channels/DatagramChannel/ChangingAddress.java` have been fixed by
> the new implementation of DatagramSocket. They are still known to fail on
> macOS with the old implementation, however.
>
> This fix updates these tests to explicitly run with
> `-Djdk.net.usePlainDatagramSocketImpl=false` to avoid false failures when
> running all tests with a global jtreg -Djdk.net.usePlainDatagramSocketImpl
> switch.
>
> issue: https://bugs.openjdk.java.net/browse/JDK-8246164
> webrev: cr.openjdk.java.net/~pconcannon/8246164/webrevs/webrev.00/
>
> Kind regards,
> Patrick



-- 
Thanks,
Vyom

Re: RFR 8252248: __SIGRTMAX is not declared in musl libc

[Vyom Tiwari]

+1
Vyom

On Fri, Aug 28, 2020 at 8:24 PM Alexander Scherbatiy <
alexander.scherba...@bell-sw.com> wrote:

> On 28.08.2020 17:40, Alan Bateman wrote:
>
> > On 28/08/2020 15:32, Alexander Scherbatiy wrote:
> >>
> >>   I run the java/nio/channels tests with the fix. There are five
> >> failed tests that fail with and without the fix:
> >> java/nio/channels/DatagramChannel/AdaptorMulticasting.java
> >> java/nio/channels/DatagramChannel/MulticastSendReceiveTests.java
> >> java/nio/channels/DatagramChannel/PromiscuousIPv6.java
> >> java/nio/channels/DatagramChannel/Loopback.java
> >> java/nio/channels/FileChannel/FileExtensionAndMap.java
> >>
> >> The FileExtensionAndMap.java has clear fail message: "Error. Test
> >> ignored: This test has huge disk space requirements".
> > The DatagramChannel failures are probably because of firewall or
> > iptables issues too.  The FileExtensionAndMap test has @ignore so it
> > will be skipped, maybe you are running jtreg directly and aren't use
> > -ignore:quiet?
>Yes, I run the jtreg tests directly without the -ignore:quiet option.
>
>Thanks,
>Alexander.
> >
> > In any case, I think the changes look okay.
> >
> > -Alan
>


-- 
Thanks,
Vyom

jdk11u build failure on Windows

[Vyom Tiwari]

Hi,
I am facing the build issue with  OpenJDK11(jdk11u). I am trying to build
jdk11u on Windows and I am  getting the below error.


./src/java.base/windows/native/libnet/net_util_md.c(792): error C2065:
'TCP_INITIAL_RTO_NO_SYN_RETRANSMISSIONS': undeclared identifier
make[3]: *** [Lib-java.base.gmk:44:
/cygdrive/d/source/mirrors_github_jdk11u/build/windows-x86_64-normal-server-release/support/native/java.base/libnet/net_util_md.obj]
Error 1
make[3]: *** Waiting for unfinished jobs
make[2]: *** [make/Main.gmk:215: java.base-libs] Error 2

ERROR: Build failed for target 'images' in configuration
'windows-x86_64-normal-server-release' (exit code 2)
Stopping sjavac server
###

I found that the back-port of  JDK-8250521
  is causing this failure.

My  environment details are as follows.
OS: Windows Server 2016 Standard, Visual Studio professional 2017 version
15.2(26430.14)

When I updated my Visual Studio to 15.9.42(which contains Windows 10
SDK(10.0.17134.0)) the problem went away.

My question is do we have a minimum supported Visual Studio(15.9.42) to
build jdk11u(11.0.14) ?. If this is not the case then I would like to fix
this build failure.

Thanks,
Vyom