RE: [11u] RFR: 8183369: RFC unconformity of HttpURLConnection with proxy

2020-03-24 Thread Langer, Christoph
Hi Alex,

looks good now.

Cheers
Christoph

> -Original Message-
> From: Alex Kashchenko 
> Sent: Dienstag, 24. März 2020 19:09
> To: Langer, Christoph ; jdk-updates-
> d...@openjdk.java.net
> Cc: net-dev@openjdk.java.net
> Subject: Re: [11u] RFR: 8183369: RFC unconformity of HttpURLConnection
> with proxy
> 
> Hi Christoph,
> 
> On 03/24/2020 12:36 PM, Langer, Christoph wrote:
> > Hi Alex,
> >
> > I think this looks good overall.
> 
> Thanks for the review!
> 
> >
> > There's one line in
> test/jdk/java/net/HttpURLConnection/HttpURLConWithProxy.java, line 85,
> where you got the indentation wrong. Please check and fix that.
> 
> Sorry for tabs on that line, not sure how they got there, updated webrev
> after :retab :
> 
> http://cr.openjdk.java.net/~akasko/jdk11u/8183369/webrev.01/
> 
> >
> > Best regards
> > Christoph
> >
> >> -Original Message-
> >> From: jdk-updates-dev 
> On
> >> Behalf Of Alex Kashchenko
> >> Sent: Dienstag, 24. März 2020 12:29
> >> To: jdk-updates-...@openjdk.java.net
> >> Cc: net-dev@openjdk.java.net
> >> Subject: [11u] RFR: 8183369: RFC unconformity of HttpURLConnection
> with
> >> proxy
> >>
> >> Hi,
> >>
> >> Please review the backport of JDK-8183369 to 11u:
> >>
> >> Bug: https://bugs.openjdk.java.net/browse/JDK-8183369
> >>
> >> Original change: https://hg.openjdk.java.net/jdk/jdk/rev/0ba758a2b6f0
> >>
> >> 11u webrev:
> >> http://cr.openjdk.java.net/~akasko/jdk11u/8183369/webrev.00/
> >>
> >> Patch to HttpURLConnection applies cleanly, patch to
> HttpURLConWithProxy
> >> test was adjusted because mainline changes depend on JDK-8227539 that
> >> was added in 14.
> >>
> >> --
> >> -Alex
> >
> 
> 
> --
> -Alex



RE: [11u] RFR: 8183369: RFC unconformity of HttpURLConnection with proxy

2020-03-24 Thread Langer, Christoph
Hi Alex,

I think this looks good overall.

There's one line in 
test/jdk/java/net/HttpURLConnection/HttpURLConWithProxy.java, line 85, where 
you got the indentation wrong. Please check and fix that.

Best regards
Christoph

> -Original Message-
> From: jdk-updates-dev  On
> Behalf Of Alex Kashchenko
> Sent: Dienstag, 24. März 2020 12:29
> To: jdk-updates-...@openjdk.java.net
> Cc: net-dev@openjdk.java.net
> Subject: [11u] RFR: 8183369: RFC unconformity of HttpURLConnection with
> proxy
> 
> Hi,
> 
> Please review the backport of JDK-8183369 to 11u:
> 
> Bug: https://bugs.openjdk.java.net/browse/JDK-8183369
> 
> Original change: https://hg.openjdk.java.net/jdk/jdk/rev/0ba758a2b6f0
> 
> 11u webrev:
> http://cr.openjdk.java.net/~akasko/jdk11u/8183369/webrev.00/
> 
> Patch to HttpURLConnection applies cleanly, patch to HttpURLConWithProxy
> test was adjusted because mainline changes depend on JDK-8227539 that
> was added in 14.
> 
> --
> -Alex



RE: RFR 8129841 Update comment for Java_java_net_Inet6AddressImpl_getHostByAddr

2020-03-04 Thread Langer, Christoph
Hi Vipin,

I'm forwarding this to net-dev where it should be reviewed.

Best regards
Christoph

> -Original Message-
> From: core-libs-dev  On Behalf
> Of Vipin Mv1
> Sent: Dienstag, 3. März 2020 11:54
> To: core-libs-...@openjdk.java.net
> Subject: RFR 8129841 Update comment for
> Java_java_net_Inet6AddressImpl_getHostByAddr
> 
> Hi All,
> 
> Please find the below changes for the issue
> https://bugs.openjdk.java.net/browse/JDK-8129841. Apart from the
> requested changes, I have made additional changes to the Signature where
> ever I found it incorrect.
> 
> 
> Thanks
> Vipin M V
> 
> 
> diff -r 387369ff21a4 src/java.base/unix/native/libnet/Inet4AddressImpl.c
> --- a/src/java.base/unix/native/libnet/Inet4AddressImpl.c Wed Feb 05
> 12:20:36 2020 -0300
> +++ b/src/java.base/unix/native/libnet/Inet4AddressImpl.c Tue Mar 03
> 15:32:21 2020 +0530
> @@ -218,7 +218,7 @@
>  /*
>   * Class: java_net_Inet4AddressImpl
>   * Method:getHostByAddr
> - * Signature: (I)Ljava/lang/String;
> + * Signature: ([B)Ljava/lang/String;
>   *
>   * Theoretically the UnknownHostException could be enriched with gai error
>   * information. But as it is silently ignored anyway, there's no need for 
> this.
> diff -r 387369ff21a4 src/java.base/unix/native/libnet/Inet6AddressImpl.c
> --- a/src/java.base/unix/native/libnet/Inet6AddressImpl.c Wed Feb 05
> 12:20:36 2020 -0300
> +++ b/src/java.base/unix/native/libnet/Inet6AddressImpl.c Tue Mar 03
> 15:32:21 2020 +0530
> @@ -413,7 +413,7 @@
>  /*
>   * Class: java_net_Inet6AddressImpl
>   * Method:getHostByAddr
> - * Signature: (I)Ljava/lang/String;
> + * Signature: ([B)Ljava/lang/String;
>   *
>   * Theoretically the UnknownHostException could be enriched with gai error
>   * information. But as it is silently ignored anyway, there's no need for 
> this.
> @@ -668,7 +668,7 @@
>  /*
>   * Class: java_net_Inet6AddressImpl
>   * Method:isReachable0
> - * Signature: ([bII[bI)Z
> + * Signature: ([BII[BII)Z
>   */
>  JNIEXPORT jboolean JNICALL
>  Java_java_net_Inet6AddressImpl_isReachable0(JNIEnv *env, jobject this,
> diff -r 387369ff21a4
> src/java.base/windows/native/libnet/Inet4AddressImpl.c
> --- a/src/java.base/windows/native/libnet/Inet4AddressImpl.c  Wed
> Feb 05 12:20:36 2020 -0300
> +++ b/src/java.base/windows/native/libnet/Inet4AddressImpl.c  Tue
> Mar 03 15:32:21 2020 +0530
> @@ -171,7 +171,7 @@
>  /*
>   * Class: java_net_Inet4AddressImpl
>   * Method:getHostByAddr
> - * Signature: (I)Ljava/lang/String;
> + * Signature: ([B)Ljava/lang/String;
>   *
>   * Theoretically the UnknownHostException could be enriched with gai error
>   * information. But as it is silently ignored anyway, there's no need for 
> this.
> diff -r 387369ff21a4
> src/java.base/windows/native/libnet/Inet6AddressImpl.c
> --- a/src/java.base/windows/native/libnet/Inet6AddressImpl.c  Wed
> Feb 05 12:20:36 2020 -0300
> +++ b/src/java.base/windows/native/libnet/Inet6AddressImpl.c  Tue
> Mar 03 15:32:21 2020 +0530
> @@ -240,7 +240,7 @@
>  /*
>   * Class: java_net_Inet6AddressImpl
>   * Method:getHostByAddr
> - * Signature: (I)Ljava/lang/String;
> + * Signature: ([B)Ljava/lang/String;
>   *
>   * Theoretically the UnknownHostException could be enriched with gai error
>   * information. But as it is silently ignored anyway, there's no need for 
> this.
> @@ -435,7 +435,7 @@
>  /*
>   * Class: java_net_Inet6AddressImpl
>   * Method:isReachable0
> - * Signature: ([bII[bI)Z
> + * Signature: ([BII[BII)Z
>   */
>  JNIEXPORT jboolean JNICALL
>  Java_java_net_Inet6AddressImpl_isReachable0(JNIEnv *env, jobject this,
> 
> 



RE: RFR: 8228482: fix xlc16/xlclang comparison of distinct pointer types and string literal conversion warnings

2019-09-17 Thread Langer, Christoph
Hi Matthias,

this seems fine to me. The change in NetworkInterface.c is correct, too.

Best regards
Christoph

> -Original Message-
> From: core-libs-dev  On Behalf
> Of Baesken, Matthias
> Sent: Donnerstag, 25. Juli 2019 15:45
> To: Doerr, Martin ; 'hotspot-
> d...@openjdk.java.net' ; core-libs-
> d...@openjdk.java.net; net-dev@openjdk.java.net
> Cc: 'ppc-aix-port-...@openjdk.java.net'  d...@openjdk.java.net>
> Subject: [CAUTION] RE: RFR: 8228482: fix xlc16/xlclang comparison of distinct
> pointer types and string literal conversion warnings
> 
> Thanks Martin .
> May I get a second review ?
> 
> Best regards, Matthias
> 
> 
> From: Doerr, Martin
> Sent: Mittwoch, 24. Juli 2019 12:14
> To: Baesken, Matthias ; 'hotspot-
> d...@openjdk.java.net' ; core-libs-
> d...@openjdk.java.net; net-dev@openjdk.java.net
> Cc: 'ppc-aix-port-...@openjdk.java.net'  d...@openjdk.java.net>
> Subject: RE: RFR: 8228482: fix xlc16/xlclang comparison of distinct pointer
> types and string literal conversion warnings
> 
> Hi Matthias,
> 
> I wouldn’t say „looks good”, but I think it’s the right thing to do 😊
> The type casts look correct to fit to the AIX headers.
> 
> libodm_aix:
> Good. Maybe we should open a new issue for freeing what’s returned by
> odm_set_path and we could also remove the AIX 5 support.
> 
> NetworkInterface.c:
> Strange, but ok. Should be reviewed by somebody else in addition.
> 
> Other files:
> No comments.
> 
> Best regards,
> Martin
> 
> 
> From: ppc-aix-port-dev  boun...@openjdk.java.net boun...@openjdk.java.net>> On Behalf Of Baesken, Matthias
> Sent: Dienstag, 23. Juli 2019 17:15
> To: 'hotspot-...@openjdk.java.net'  d...@openjdk.java.net>; core-libs-
> d...@openjdk.java.net; net-
> d...@openjdk.java.net
> Cc: 'ppc-aix-port-...@openjdk.java.net'  d...@openjdk.java.net>
> Subject: RFR: 8228482: fix xlc16/xlclang comparison of distinct pointer types
> and string literal conversion warnings
> 
> Hello please review this patch .
> 
> It fixes a couple of  xlc16/xlclang warnings  , especially  comparison of 
> distinct
> pointer types and string literal conversion warnings  .
> 
> When building with xlc16/xlclang, we still have a couple of warnings that have
> to be fixed :
> warning: ISO C++11 does not allow conversion from string literal to 'char *' 
> [-
> Wwritable-strings]
> for example :
> /nightly/jdk/src/hotspot/os/aix/libodm_aix.cpp:81:18: warning: ISO C++11
> does not allow conversion from string literal to 'char *' [-Wwritable-strings]
>   odmWrapper odm("product", "/usr/lib/objrepos"); // could also use "lpp"
>  ^
> /nightly/jdk/src/hotspot/os/aix/libodm_aix.cpp:81:29: warning: ISO C++11
> does not allow conversion from string literal to 'char *' [-Wwritable-strings]
>   odmWrapper odm("product", "/usr/lib/objrepos"); // could also use "lpp"
> ^
> 
> warning: comparison of distinct pointer types, for example :
> 
> /nightly/jdk/src/java.desktop/aix/native/libawt/porting_aix.c:50:14:
> warning: comparison of distinct pointer types ('void *' and 'char *') [-
> Wcompare-distinct-pointer-types]
> addr < (((char*)p->ldinfo_textorg) + p->ldinfo_textsize)) {
>  ^ ~
> 
> 
> 
> Bug/webrev :
> 
> 
> https://bugs.openjdk.java.net/browse/JDK-8228482
> 
> http://cr.openjdk.java.net/~mbaesken/webrevs/8228482.1/
> 
> 
> Thanks, Matthias
> 



RE: RFR [XS] : 8229706: java/net/MulticastSocket/NoLoopbackPackets.java fails on some AIX machines

2019-08-26 Thread Langer, Christoph
Hi Matthias,

looks good to me, too. And +1 for dropping the comment.

Best regards
Christoph

> -Original Message-
> From: net-dev  On Behalf Of Chris
> Hegarty
> Sent: Freitag, 16. August 2019 13:44
> To: Baesken, Matthias 
> Cc: net-dev@openjdk.java.net
> Subject: Re: RFR [XS] : 8229706:
> java/net/MulticastSocket/NoLoopbackPackets.java fails on some AIX
> machines
> 
> 
> > On 16 Aug 2019, at 10:51, Baesken, Matthias 
> wrote:
> >
> > Thanks alot !
> >
> > I guess I can push  this as  trivial (but a second reviewer would be good 
> > too
> ).
> 
> The change looks fine.
> 
> You could probably drop the comment, I don’t think that
> it’s really needed.
> 
> > Best regards, Matthias
> >
> >> -Original Message-
> >> From: Daniel Fuchs 
> >> ...
> >>
> >> I haven't observed any failures over 50 runs on all
> >> platforms tested. macosx is problem listed for this
> >> test so it wasn't run there - but all others platforms
> >> passed with your changes.
> 
> The set of checks in the NetworkConfiguration for
> has_testableipv6address could probably be expanded to
> include macOS. The maybe the test could be removed
> from the ProblemList ( for mac ). But that is a separate
> issue, for a follow-on, verification, testing, etc.
> 
> -Chris.


[11u] RFR: 8216562: UnknownBodyLength sometimes fails due to "Connection reset by peer"

2019-08-06 Thread Langer, Christoph
Hi,

please review the jdk11u backport of another fix for test 
java/net/httpclient/UnknownBodyLengthTest.java. The patch did apply mostly 
clean but needed some help in the import directives. Now the change looks 
effectively the same as the original.

Bug: https://bugs.openjdk.java.net/browse/JDK-8216562
Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8216562.11u-dev.0/

Thanks
Christoph



[11u] RFR: 8210130: java/net/httpclient/UnknownBodyLengthTest.java failed

2019-08-06 Thread Langer, Christoph
Hi,

please review the jdk11u backport of a fix for test 
java/net/httpclient/UnknownBodyLengthTest.java. The test is instable in jdk11u, 
too, so it should be backported. The patch did apply mostly but the changes to 
the @run main/othervm directives needed some manual intervention.
Bug: https://bugs.openjdk.java.net/browse/JDK-8210130
Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8210130.11u-dev.0/

Thanks
Christoph



RE: RFR : 8227737: avoid implicit-function-declaration on AIX

2019-07-17 Thread Langer, Christoph
Hi Matthias,



looks good, thanks for doing this.



How far are we then from enabling "warnings as errors" on AIX? :P



Best regards

Christoph



From: awt-dev  On Behalf Of Baesken, Matthias
Sent: Dienstag, 16. Juli 2019 17:04
To: Java Core Libs ; nio-...@openjdk.java.net; 
net-dev@openjdk.java.net; awt-...@openjdk.java.net
Cc: 'ppc-aix-port-...@openjdk.java.net' 
Subject: [CAUTION]  RFR : 8227737: avoid implicit-function-declaration 
on AIX



Hello, please review the following  AIX related change .

It fixes a number of  missing inclusions  leading to  
implicit-function-declaration  warnings when compiling  with the recent xlc16 
/xlclang .





At various places in the native C coding in jdk, we miss header inclusions on 
AIX.
This leads to warnings like :

/nightly/jdk/src/java.base/unix/native/libjava/childproc.c:99:5: warning: 
implicitly declaring library function 'snprintf'
with type 'int (char *, unsigned long, const char *, ...)' 
[-Wimplicit-function-declaration]
snprintf(aix_fd_dir, 32, "/proc/%d/fd", getpid());
^
/nightly/jdk/src/java.base/unix/native/libjava/childproc.c:99:5: note: include 
the header  or explicitly provide a declaration for 'snprintf'

or

/nightly/jdk/src/java.base/aix/native/libjli/java_md_aix.c:38:5: warning: 
implicitly declaring library function 'memset'
with type 'void *(void *, int, unsigned long)' [-Wimplicit-function-declaration]
memset((void *)info, 0, sizeof(Dl_info));

/nightly/jdk/src/java.base/aix/native/libjli/java_md_aix.c:38:5: note: include 
the header  or explicitly provide a declaration for 'memset'







Bug/webrev :



https://bugs.openjdk.java.net/browse/JDK-8227737



http://cr.openjdk.java.net/~mbaesken/webrevs/8227737.0/





Thanks, Matthias



RE: [8u] RFR: 8226928: [TESTBUG] test/java/net/NetworkInterface/IPv4Only.java fails intermittently on AIX

2019-06-28 Thread Langer, Christoph
Hi Severin,

looks good, ship it 😊

Thanks
Christoph

> -Original Message-
> From: net-dev  On Behalf Of Severin
> Gehwolf
> Sent: Freitag, 28. Juni 2019 10:41
> To: jdk8u-dev 
> Cc: net-dev 
> Subject: [8u] RFR: 8226928: [TESTBUG]
> test/java/net/NetworkInterface/IPv4Only.java fails intermittently on AIX
> 
> Hi,
> 
> Please review this trivial test fix for JDK 8u. JDK 11u's version of
> the same test sets the property on the command line which doesn't have
> the issue on AIX. This patch makes JDK 11u and 8u aligned when it comes
> to IPv4Only.java test. I didn't backport the 11 change, as that's been
> part of a huge "Modular Run-Time Images" change, not suitable for JDK
> 8u. Thoughts?
> 
> Bug: https://bugs.openjdk.java.net/browse/JDK-8226928
> webrev: http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-
> 8226928/01/webrev/
> 
> Testing: Passes before/after this change on Linux x86_64 (unsurprisingly).
> 
> Note that I've not reproduce the issue on AIX, I'm just the person
> reporting the issue and thought that I might as well fix it right away.
> Shouldn't really be controversal.
> 
> Thanks,
> Severin



RE: [BUG] Inet6Address.isIPv4CompatibleAddress uses wrong prefix

2019-06-24 Thread Langer, Christoph
Hi Rob,

sending this over to net-dev, where it should be discussed...

/Christoph

> -Original Message-
> From: core-libs-dev  On Behalf
> Of Rob Spoor
> Sent: Montag, 24. Juni 2019 22:58
> To: core-libs-...@openjdk.java.net
> Subject: [BUG] Inet6Address.isIPv4CompatibleAddress uses wrong prefix
> 
> I found a bug in Inet6Adress.isIPv4CompatibleAddress(). While parsing
> correctly uses the ::: format, isIPv4CompatibleAddress()
> checks for :: instead. An example:
> 
>  Inet6Address address = (Inet6Address)
> InetAddress.getByName("::192.168.1.13");
>  System.out.printf("%s: %b%n", address,
> address.isIPv4CompatibleAddress());
> 
> This should print false, but instead it prints true.
> 
> The error is in the Inet6Address.Inet6AddressHolder class:
> 
>  boolean isIPv4CompatibleAddress() {
>  if ((ipaddress[0] == 0x00) && (ipaddress[1] == 0x00) &&
>  (ipaddress[2] == 0x00) && (ipaddress[3] == 0x00) &&
>  (ipaddress[4] == 0x00) && (ipaddress[5] == 0x00) &&
>  (ipaddress[6] == 0x00) && (ipaddress[7] == 0x00) &&
>  (ipaddress[8] == 0x00) && (ipaddress[9] == 0x00) &&
>  (ipaddress[10] == 0x00) && (ipaddress[11] == 0x00))  {
>  return true;
>  }
>  return false;
>  }
> 
> I think that bytes 10 and 11 should both be (byte) 0xFF instead of 0x00.
> This is what's being used in IPAddressUtil, which is used for parsing:
> 
>  private static boolean isIPv4MappedAddress(byte[] addr) {
>  if (addr.length < INADDR16SZ) {
>  return false;
>  }
>  if ((addr[0] == 0x00) && (addr[1] == 0x00) &&
>  (addr[2] == 0x00) && (addr[3] == 0x00) &&
>  (addr[4] == 0x00) && (addr[5] == 0x00) &&
>  (addr[6] == 0x00) && (addr[7] == 0x00) &&
>  (addr[8] == 0x00) && (addr[9] == 0x00) &&
>  (addr[10] == (byte)0xff) &&
>  (addr[11] == (byte)0xff))  {
>  return true;
>  }
>  return false;
>  }
> 
> Maybe it's an idea to let Inet6Address.Inet6AddressHolder delegate to
> this latter method?
> 
> 
> Rob


RE: [RFR] 8194231: java/net/DatagramSocket/ReuseAddressTest.java failed with java.net.BindException: Address already in use: Cannot bind

2019-05-31 Thread Langer, Christoph
Thanks, Chris. I pushed it then on behalf of Arno.

> -Original Message-
> From: net-dev  On Behalf Of Chris
> Hegarty
> Sent: Freitag, 31. Mai 2019 16:06
> To: Zeller, Arno 
> Cc: net-dev@openjdk.java.net
> Subject: Re: [RFR] 8194231:
> java/net/DatagramSocket/ReuseAddressTest.java failed with
> java.net.BindException: Address already in use: Cannot bind
> 
> 
> > On 28 May 2019, at 16:20, Chris Hegarty 
> wrote:
> >
> > ..
> >
> > I would like to run it through our internal build and test infra, after 
> > which
> > I’ll post the results here.
> 
> All clear ( this test passes successfully in our build and test environment ).
> 
> -Chris.


RE: [RFR] 8194231: java/net/DatagramSocket/ReuseAddressTest.java failed with java.net.BindException: Address already in use: Cannot bind

2019-05-29 Thread Langer, Christoph
Hi,

Thanks, Arno. This looks good to me, too.

@Chris: We were running with this patch in our nightly test system for a while 
now and we don't see regressions. But looking forward to hear from your results.

Best regards
Christoph

> -Original Message-
> From: net-dev  On Behalf Of Zeller,
> Arno
> Sent: Dienstag, 28. Mai 2019 20:52
> To: Chris Hegarty 
> Cc: net-dev@openjdk.java.net
> Subject: [CAUTION] RE: [RFR] 8194231:
> java/net/DatagramSocket/ReuseAddressTest.java failed with
> java.net.BindException: Address already in use: Cannot bind
> 
> Hi Chris,
> 
> thanks for having a look and running tests in your infrastructure.
> 
> I updated the webrev in place with the missing whitespace in the copyright
> header. I hope this is ok!
> 
> Best regards,
> Arno
> 
> -Original Message-
> From: Chris Hegarty 
> Sent: Dienstag, 28. Mai 2019 17:21
> To: Zeller, Arno 
> Cc: net-dev@openjdk.java.net
> Subject: Re: [RFR] 8194231:
> java/net/DatagramSocket/ReuseAddressTest.java failed with
> java.net.BindException: Address already in use: Cannot bind
> 
> Arno,
> 
> > On 28 May 2019, at 16:10, Zeller, Arno  wrote:
> >
> > Hi,
> >
> > I took a look at JDK-8194231 and would like to propose the following fix for
> the issue.
> >
> > The problem seems that the test uses fixed port 5050 for testing that is 
> > also
> used by newer Windows versions (Windows 10/Window Server 2016 and
> newer).
> > Therefore the test will always fail when run on a newer Windows machine.
> To avoid such problems in the future I changed the three sub tests that use a
> fixed port to use a generic port instead.
> >
> > Bug:
> > 8194231: java/net/DatagramSocket/ReuseAddressTest.java failed with
> java.net.BindException: Address already in use: Cannot bind
> > https://bugs.openjdk.java.net/browse/JDK-8194231
> >
> > Webrev:
> > http://cr.openjdk.java.net/~azeller/webrevs/8194231/
> 
> I think this is good.
> 
> I would like to run it through our internal build and test infra, after which
> I’ll post the results here.
> 
> Trivially, a space is needed after the coma in the copyright header. “2016,
> 2019, "
> 
> +/* Copyright (c) 2016,2019, Oracle and/or its affiliates. All rights 
> reserved.
> 
> -Chris.


RE: RFR: 8223553: Fix code constructs that do not compile with the Eclipse Java Compiler

2019-05-23 Thread Langer, Christoph
> Overall, introducing a local variable is more-or-less reasonable even if it's
> used only once. One point is that somebody might come along and "clean
> up" the
> code and inline local variables and reintroduce the problem. Another point is
> that, hypothetically, if in the future Eclipse is changed to match javac's
> behavior, these changes should be reverted.
>
> The bug database is a good place to capture actions that need to take place in
> the future. Unfortunately, it's pretty far from these locations, so the
> existence of such a bug wouldn't prevent the accidental cleanup from
> happening.
> That would indicate having a comment in the code at these locations.
>
> On the other hand, if Eclipse is "fixed" and these locations don't get cleaned
> up for a long time, it doesn't seem like that big a deal.
>
> I'd suggest to put a comment at these 3 locations, something like:
>
>  // local variable required here; see JDK-8223553
>
> and not bother with filing another bug report to track this.

Ok, good idea. I'll add the comment before I push.

> I'll defer to Martin as to how he wants to handle the
> ConcurrentSkipListMap.java
> change.

As Martin has taken this to the jsr166 integration 2019-06, I'll push the 
change without ConcurrentSkipListMap.java tomorrow.

Thanks to all involved in this review!
/Christoph



RE: RFR: 8223553: Fix code constructs that do not compile with the Eclipse Java Compiler

2019-05-23 Thread Langer, Christoph
Hi Stuart,

big thanks for your great updates here. This all looks a lot cleaner: 
http://cr.openjdk.java.net/~clanger/webrevs/8223553.3/

So, for ConcurrentSkipListMap.java, the new coding is a real improvement, 
removing a @SuppressWarnings("unchecked") and a cast which are both unnecessary 
then.
@Martin Buchholz: What do I have to do to get this into the jsr166 integration 
2019-06? Shall I open a separate bug? Or shall it be committed with the fix to 
JDK-8223553? Please guide me.

In the other 3 locations, we're able to eliminate the EC4J issues by 
introducing a local variable with the right type declaration. Sounds like a 
viable workaround.

At Eclipse we have the following bug: 
https://bugs.eclipse.org/bugs/show_bug.cgi?id=530236. It refers to the OpenJDK 
bug https://bugs.openjdk.java.net/browse/JDK-8016207. 

I'm wondering whether this should be submitted and I should at the same time 
submit another bug to evaluate these code places at a time when the final 
resolution for JDK-8016207 was provided?

Thank you
Christoph


> -Original Message-
> From: Stuart Marks 
> Sent: Samstag, 18. Mai 2019 00:56
> To: Langer, Christoph 
> Cc: David Holmes ; Daniel Fuchs
> ; core-libs-dev  d...@openjdk.java.net>; net-dev ; compiler-
> d...@openjdk.java.net
> Subject: Re: RFR: 8223553: Fix code constructs that do not compile with the
> Eclipse Java Compiler
> 
> Hi Christoph,
> 
> I'm still not entirely sure why this is so, but the introduction of a local
> variable in MethodHandles.java seems to make things work for Eclipse.
> Addition
> of a local variable seems to be minimally invasive, so it makes sense to see 
> if
> this technique can be applied to other cases.
> 
> (In ConcurrentSkipListMap the issue seems to be solved by addition of
> wildcards,
> as I had suggested previously, and it cleans up the unchecked cast that was
> there in the first place. So I think that one is ok already.)
> 
> In ManagementFactory.java, an unchecked cast and warnings suppression is
> introduced, and in ExchangeImpl.java a helper method was introduced. I've
> found
> ways to introduce local variables that make Eclipse happy for these cases.
> (Well, on localized test cases; I haven't built the whole JDK with Eclipse.) 
> See
> diffs below.
> 
> The type of the local variable in ExchangeImpl.java is a mouthful.
> Interestingly
> I had to change Function.identity() to the lambda x -> x. I think this is
> because Function.identity() returns a function that doesn't allow its return
> type to vary from its argument, whereas x -> x allows a widening conversion.
> (This might provide a clue as to the differences between javac and Eclipse
> here,
> but a full understanding eludes me.)
> 
> s'marks
> 
> 
> 
> diff -r 006dadb903ab
> src/java.base/share/classes/java/util/concurrent/ConcurrentSkipListMap.jav
> a
> ---
> a/src/java.base/share/classes/java/util/concurrent/ConcurrentSkipListMap.j
> ava
> Mon May 13 17:15:56 2019 -0700
> +++
> b/src/java.base/share/classes/java/util/concurrent/ConcurrentSkipListMap.j
> ava
> Fri May 17 15:46:14 2019 -0700
> @@ -1712,9 +1712,7 @@
>   Map m = (Map) o;
>   try {
>   Comparator cmp = comparator;
> -@SuppressWarnings("unchecked")
> -Iterator> it =
> -(Iterator>)m.entrySet().iterator();
> +Iterator> it = m.entrySet().iterator();
>   if (m instanceof SortedMap &&
>   ((SortedMap)m).comparator() == cmp) {
>   Node b, n;
> diff -r 006dadb903ab
> src/java.management/share/classes/java/lang/management/ManagementF
> actory.java
> ---
> a/src/java.management/share/classes/java/lang/management/Managemen
> tFactory.java
> Mon May 13 17:15:56 2019 -0700
> +++
> b/src/java.management/share/classes/java/lang/management/Managemen
> tFactory.java
> Fri May 17 15:46:14 2019 -0700
> @@ -872,12 +872,12 @@
>   public static Set>
>  getPlatformManagementInterfaces()
>   {
> -return platformComponents()
> +Stream> pmos =
> platformComponents()
>   .stream()
>   .flatMap(pc -> pc.mbeanInterfaces().stream())
>   .filter(clazz ->
> PlatformManagedObject.class.isAssignableFrom(clazz))
> -.map(clazz -> clazz.asSubclass(PlatformManagedObject.class))
> -.collect(Collectors.toSet());
> +.map(clazz -> clazz.asSubclass(PlatformManagedObject.class));
> + return pmos.collect(Collectors.toSet());
>   }
> 
>   private static final String NOTIF_EMITTER =
>

RE: RFR: 8223553: Fix code constructs that do not compile with the Eclipse Java Compiler

2019-05-15 Thread Langer, Christoph
Hi David, Martin,

thanks for looking into this.

Generally I share your view on this. It's not nice at all.

However, it's the only way I can see currently to get rid of the Errors in the 
Eclipse IDE. Maybe an idea would be to get this in but at the same time open a 
ticket to evaluate this code again from a compiler/spec perspective and make 
the according modifications?

Thanks
Christoph

> -Original Message-
> From: David Holmes 
> Sent: Mittwoch, 15. Mai 2019 00:05
> To: Langer, Christoph ; Daniel Fuchs
> ; core-libs-dev  d...@openjdk.java.net>; net-dev 
> Cc: compiler-...@openjdk.java.net; Martin Buchholz
> 
> Subject: Re: RFR: 8223553: Fix code constructs that do not compile with the
> Eclipse Java Compiler
> 
> Hi Christoph,
> 
> I'm very reluctant to see changes like this that the compiler folk have
> not determined are actually incorrect. That said ...
> 
> On 15/05/2019 7:03 am, Langer, Christoph wrote:
> > Thanks Daniel.
> >
> > Can anybody help reviewing the changes to:
> > src/java.base/share/classes/java/lang/invoke/MethodHandles.java
> 
> The introduction of the intermediate local variable seems harmless
> (though why it should be necessary is another matter).
> 
> >
> src/java.base/share/classes/java/util/concurrent/ConcurrentSkipListMap.jav
> a
> 
> As you note this should be ok'd by jsr166 folk so I've cc'd Martin
> Buccholz. I dislike seeing a raw type introduced here though.
> 
> > and
> >
> src/java.management/share/classes/java/lang/management/ManagementF
> actory.java ?
> 
> Introducing an unchecked cast seems very crude. I'd want the core-libs
> stream experts to comment on this.
> 
> Cheers,
> David
> 
> 
> > Thanks
> > Christoph
> >
> >> -Original Message-
> >> From: Daniel Fuchs 
> >> Sent: Dienstag, 14. Mai 2019 18:04
> >> To: Langer, Christoph ; core-libs-dev  libs-
> >> d...@openjdk.java.net>; net-dev 
> >> Cc: compiler-...@openjdk.java.net
> >> Subject: Re: RFR: 8223553: Fix code constructs that do not compile with
> the
> >> Eclipse Java Compiler
> >>
> >> Hi Christoph,
> >>
> >> That looks much better, thanks!
> >> (but still not commenting on the other changes ;-))
> >>
> >> best regards,
> >>
> >> -- daniel
> >>
> >> On 14/05/2019 13:57, Langer, Christoph wrote:
> >>> Hi Daniel,
> >>>
> >>>>> unfortunately, your proposed solution does not work with javac. I get
> >> this
> >>>> in the build:
> >>>>
> >>>> Oh darn. I should have double checked.
> >>>> Can we at least reduce the scope of the @SuppressedWarnings by
> >>>> introducing a private method that just has the return call?
> >>>
> >>> Sure, what about this one:
> >> http://cr.openjdk.java.net/~clanger/webrevs/8223553.2/ ?
> >>>
> >>> Thanks
> >>> Christoph
> >>>
> >


RE: RFR: 8223553: Fix code constructs that do not compile with the Eclipse Java Compiler

2019-05-14 Thread Langer, Christoph
Thanks Daniel.

Can anybody help reviewing the changes to:
src/java.base/share/classes/java/lang/invoke/MethodHandles.java
src/java.base/share/classes/java/util/concurrent/ConcurrentSkipListMap.java
and
src/java.management/share/classes/java/lang/management/ManagementFactory.java ?

Thanks
Christoph

> -Original Message-
> From: Daniel Fuchs 
> Sent: Dienstag, 14. Mai 2019 18:04
> To: Langer, Christoph ; core-libs-dev  d...@openjdk.java.net>; net-dev 
> Cc: compiler-...@openjdk.java.net
> Subject: Re: RFR: 8223553: Fix code constructs that do not compile with the
> Eclipse Java Compiler
>
> Hi Christoph,
>
> That looks much better, thanks!
> (but still not commenting on the other changes ;-))
>
> best regards,
>
> -- daniel
>
> On 14/05/2019 13:57, Langer, Christoph wrote:
> > Hi Daniel,
> >
> >>> unfortunately, your proposed solution does not work with javac. I get
> this
> >> in the build:
> >>
> >> Oh darn. I should have double checked.
> >> Can we at least reduce the scope of the @SuppressedWarnings by
> >> introducing a private method that just has the return call?
> >
> > Sure, what about this one:
> http://cr.openjdk.java.net/~clanger/webrevs/8223553.2/ ?
> >
> > Thanks
> > Christoph
> >



RE: RFR: 8223553: Fix code constructs that do not compile with the Eclipse Java Compiler

2019-05-14 Thread Langer, Christoph
Hi Daniel,

> > unfortunately, your proposed solution does not work with javac. I get this
> in the build:
> 
> Oh darn. I should have double checked.
> Can we at least reduce the scope of the @SuppressedWarnings by
> introducing a private method that just has the return call?

Sure, what about this one: 
http://cr.openjdk.java.net/~clanger/webrevs/8223553.2/ ?

Thanks
Christoph



RE: RFR: 8223553: Fix code constructs that do not compile with the Eclipse Java Compiler

2019-05-13 Thread Langer, Christoph
Hi Daniel,

unfortunately, your proposed solution does not work with javac. I get this in 
the build:

...\mercurial\jdk\src\java.net.http\share\classes\jdk\internal\net\http\ExchangeImpl.java:103:
 error: method thenCompose in class CompletableFuture cannot be applied to 
given types;
return c2f.handle(factory).thenCompose(identity);
  ^
  required: Function>,? 
extends CompletionStage>
  found:Function>,CompletableFuture>>
  reason: cannot infer type-variable(s) U#2
(argument mismatch; Function>,CompletableFuture>> cannot be 
converted to Function>,? 
extends CompletionStage>)
  where U#1,U#2,T are type-variables:
U#1 extends Object declared in method get(Exchange,HttpConnection)
U#2 extends Object declared in method thenCompose(Function>)
T extends Object declared in class CompletableFuture
1 error

So I think we need to go back to my initial proposal which works for both, IDE 
and javac: http://cr.openjdk.java.net/~clanger/webrevs/8223553.0/ 

Thanks
Christoph


> -Original Message-
> From: Langer, Christoph
> Sent: Freitag, 10. Mai 2019 11:17
> To: Daniel Fuchs ; core-libs-dev  d...@openjdk.java.net>; net-dev 
> Cc: compiler-...@openjdk.java.net
> Subject: RE: RFR: 8223553: Fix code constructs that do not compile with the
> Eclipse Java Compiler
> 
> Hi Daniel,
> 
> I fully agree, @SuppressWarnings should be avoided wherever possible.
> 
> So, thanks for coming up with this alternative suggestion. It works and so I
> updated my webrev:
> http://cr.openjdk.java.net/~clanger/webrevs/8223553.1/
> 
> Any reviews for the other 3 files?
> 
> Thanks
> Christoph
> 
> > -----Original Message-
> > From: Daniel Fuchs 
> > Sent: Donnerstag, 9. Mai 2019 17:24
> > To: Langer, Christoph ; core-libs-dev  libs-
> > d...@openjdk.java.net>; net-dev 
> > Cc: compiler-...@openjdk.java.net
> > Subject: Re: RFR: 8223553: Fix code constructs that do not compile with the
> > Eclipse Java Compiler
> >
> > Hi Christoph,
> >
> > I'm only commenting on the HttpClient changes, I'll let
> > others comment on the other changes...
> >
> >
> http://cr.openjdk.java.net/~clanger/webrevs/8223553.0/src/java.net.http/s
> > hare/classes/jdk/internal/net/http/ExchangeImpl.java.udiff.html
> >
> > I have a profound dislike for using @SuppressedWarnings, unless
> > it's the only alternative. My preference would be to introduce
> > local variables, if that can make the compiler happy, until such time
> > where the issue is actually resolved...
> >
> > For instance, it seems that the following would work:
> >
> >  // Use local variable assignments to keep other java compilers
> >  // happy and avoid unchecked casts warnings
> >  BiFunction > ExchangeImpl>>
> >      factory = (h2c, t) -> createExchangeImpl(h2c, t, exchange,
> > connection);
> >  Function>,
> > CompletableFuture>>
> >  identity = (cf) -> cf;
> >  return c2f.handle(factory).thenCompose(identity);
> >
> >
> > best regards,
> >
> > -- daniel
> >
> > On 08/05/2019 09:02, Langer, Christoph wrote:
> > > Hi,
> > >
> > > please review a small change that I'd like to see in OpenJDK to get rid
> > > of compilation errors in the Eclipse IDE.
> > >
> > > It seems the root cause for the compilation errors is that javac would
> > > sometimes widen capture variables and is hence able to compile the
> > > places that I touch here. The EC4J compiler claims that it's following
> > > the spec more strictly and bails out at these places. I had posted about
> > > this on compiler-dev but got no reaction [0].
> > >
> > > So, as this seems to be some field of open work for the compiler/spec
> > > folks, I'd like to get EC4J quiesced by introducing some slight
> > > modifications to the original places which makes the code appeal a bit
> > > less elegant and fluent but will get rid of the compile errors.
> > >
> > > Please review:
> > >
> > > Bug: https://bugs.openjdk.java.net/browse/JDK-8223553
> > >
> > > Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8223553.0/
> > >
> > > The modification to
> > >
> >
> src/java.base/share/classes/java/util/concurrent/ConcurrentSkipListMap.jav
> > a
> > > belongs to JSR-166, so I don't know if it needs some special handling.
> > >
> > > Thanks & Best regards
> > >
> > > Christoph
> > >
> > > [0]
> > > https://mail.openjdk.java.net/pipermail/compiler-dev/2019-
> > March/013054.html
> > >



RE: RFR: 8223553: Fix code constructs that do not compile with the Eclipse Java Compiler

2019-05-10 Thread Langer, Christoph
Hi Daniel,

I fully agree, @SuppressWarnings should be avoided wherever possible.

So, thanks for coming up with this alternative suggestion. It works and so I 
updated my webrev: http://cr.openjdk.java.net/~clanger/webrevs/8223553.1/  

Any reviews for the other 3 files?

Thanks
Christoph

> -Original Message-
> From: Daniel Fuchs 
> Sent: Donnerstag, 9. Mai 2019 17:24
> To: Langer, Christoph ; core-libs-dev  d...@openjdk.java.net>; net-dev 
> Cc: compiler-...@openjdk.java.net
> Subject: Re: RFR: 8223553: Fix code constructs that do not compile with the
> Eclipse Java Compiler
> 
> Hi Christoph,
> 
> I'm only commenting on the HttpClient changes, I'll let
> others comment on the other changes...
> 
> http://cr.openjdk.java.net/~clanger/webrevs/8223553.0/src/java.net.http/s
> hare/classes/jdk/internal/net/http/ExchangeImpl.java.udiff.html
> 
> I have a profound dislike for using @SuppressedWarnings, unless
> it's the only alternative. My preference would be to introduce
> local variables, if that can make the compiler happy, until such time
> where the issue is actually resolved...
> 
> For instance, it seems that the following would work:
> 
>  // Use local variable assignments to keep other java compilers
>  // happy and avoid unchecked casts warnings
>  BiFunction ExchangeImpl>>
>  factory = (h2c, t) -> createExchangeImpl(h2c, t, exchange,
> connection);
>  Function>,
> CompletableFuture>>
>  identity = (cf) -> cf;
>  return c2f.handle(factory).thenCompose(identity);
> 
> 
> best regards,
> 
> -- daniel
> 
> On 08/05/2019 09:02, Langer, Christoph wrote:
> > Hi,
> >
> > please review a small change that I'd like to see in OpenJDK to get rid
> > of compilation errors in the Eclipse IDE.
> >
> > It seems the root cause for the compilation errors is that javac would
> > sometimes widen capture variables and is hence able to compile the
> > places that I touch here. The EC4J compiler claims that it's following
> > the spec more strictly and bails out at these places. I had posted about
> > this on compiler-dev but got no reaction [0].
> >
> > So, as this seems to be some field of open work for the compiler/spec
> > folks, I'd like to get EC4J quiesced by introducing some slight
> > modifications to the original places which makes the code appeal a bit
> > less elegant and fluent but will get rid of the compile errors.
> >
> > Please review:
> >
> > Bug: https://bugs.openjdk.java.net/browse/JDK-8223553
> >
> > Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8223553.0/
> >
> > The modification to
> >
> src/java.base/share/classes/java/util/concurrent/ConcurrentSkipListMap.jav
> a
> > belongs to JSR-166, so I don't know if it needs some special handling.
> >
> > Thanks & Best regards
> >
> > Christoph
> >
> > [0]
> > https://mail.openjdk.java.net/pipermail/compiler-dev/2019-
> March/013054.html
> >



RFR: 8223553: Fix code constructs that do not compile with the Eclipse Java Compiler

2019-05-08 Thread Langer, Christoph
Hi,

please review a small change that I'd like to see in OpenJDK to get rid of 
compilation errors in the Eclipse IDE.

It seems the root cause for the compilation errors is that javac would 
sometimes widen capture variables and is hence able to compile the places that 
I touch here. The EC4J compiler claims that it's following the spec more 
strictly and bails out at these places. I had posted about this on compiler-dev 
but got no reaction [0].

So, as this seems to be some field of open work for the compiler/spec folks, 
I'd like to get EC4J quiesced by introducing some slight modifications to the 
original places which makes the code appeal a bit less elegant and fluent but 
will get rid of the compile errors.

Please review:
Bug: https://bugs.openjdk.java.net/browse/JDK-8223553
Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8223553.0/

The modification to 
src/java.base/share/classes/java/util/concurrent/ConcurrentSkipListMap.java 
belongs to JSR-166, so I don't know if it needs some special handling.

Thanks & Best regards
Christoph

[0] https://mail.openjdk.java.net/pipermail/compiler-dev/2019-March/013054.html



RE: RFR(xxxs): 8221406: Windows 32bit build error in NetworkInterface_winXP.c

2019-03-26 Thread Langer, Christoph
Hi Thomas,

looks good 😊

Christoph

From: net-dev  On Behalf Of Thomas Stüfe
Sent: Montag, 25. März 2019 14:15
To: net-dev 
Subject: RFR(xxxs): 8221406: Windows 32bit build error in 
NetworkInterface_winXP.c

Hi all,

please review this tiny fix to a windows 32 build warning:

Issue:  https://bugs.openjdk.java.net/browse/JDK-8221406
cr: 
http://cr.openjdk.java.net/~stuefe/webrevs/8221406-windows32-buildfixes-networkinterface_winxp.c/webrev.00/webrev/

Thanks, Thomas




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

2019-03-20 Thread Langer, Christoph
Hi Ivan,

looks good to me.

Best regards
Christoph

> -Original Message-
> From: net-dev  On Behalf Of Ivan
> Gerasimov
> Sent: Mittwoch, 20. März 2019 17:18
> To: net-dev 
> Subject: RFR 8170494 : JNI exception pending in PlainDatagramSocketImpl.c
> 
> 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



RE: RFR 8179549: Typo in network properties documentation

2019-03-14 Thread Langer, Christoph
Looks good, +1

/Christoph

From: net-dev  On Behalf Of Chris Hegarty
Sent: Donnerstag, 14. März 2019 17:47
To: net-dev 
Subject: RFR 8179549: Typo in network properties documentation

Trivial typo fix.

diff --git a/src/java.base/share/classes/java/net/doc-files/net-properties.html 
b/src/java.base/share/classes/java/net/doc-files/net-properties.html
--- a/src/java.base/share/classes/java/net/doc-files/net-properties.html
+++ b/src/java.base/share/classes/java/net/doc-files/net-properties.html
@@ -1,6 +1,6 @@
 
 

RE: [JDK 13] RFR 8219802: Problem list java/net/MulticastSocket/SetGetNetworkInterfaceTest.java

2019-02-27 Thread Langer, Christoph
Hi Amy,

seems reasonable to me. +1

Best regards
Christoph

> -Original Message-
> From: core-libs-dev  On Behalf
> Of Amy Lu
> Sent: Mittwoch, 27. Februar 2019 02:41
> To: OpenJDK Dev list ; Core-Libs-Dev  libs-...@openjdk.java.net>
> Subject: [JDK 13] RFR 8219802: Problem list
> java/net/MulticastSocket/SetGetNetworkInterfaceTest.java
> 
> java/net/MulticastSocket/SetGetNetworkInterfaceTest.java
> 
> Test fails due to JDK-8219083 (failures observed on windows-x64) and
> should be problem listed before mentioned issue fixed.
> 
> Please review the patch.
> 
> bug: https://bugs.openjdk.java.net/browse/JDK-8219802
> 
> Thanks,
> Amy
> 
> --- old/test/jdk/ProblemList.txt    2019-02-27 09:37:33.0 +0800
> +++ new/test/jdk/ProblemList.txt    2019-02-27 09:37:32.0 +0800
> @@ -556,6 +556,8 @@
> 
>   java/net/MulticastSocket/Test.java 7145658 macosx-all
> 
> +java/net/MulticastSocket/SetGetNetworkInterfaceTest.java 8219083
> windows-all
> +
>   java/net/DatagramSocket/SendDatagramToBadAddress.java 7143960
> macosx-all
> 
>   java/net/ServerSocket/AcceptInheritHandle.java 8211854 aix-ppc64



RE: RFR 8217657: Move the test for default value of jdk.includeInExceptions into own test

2019-01-24 Thread Langer, Christoph
Hi Sean,

thanks for looking at this. I agree. Will remove othervm...

Best regards
Christoph

> -Original Message-
> From: Sean Mullan 
> Sent: Donnerstag, 24. Januar 2019 17:43
> To: Langer, Christoph ; OpenJDK Dev list
> ; OpenJDK Network Dev list  d...@openjdk.java.net>
> Subject: Re: RFR 8217657: Move the test for default value of
> jdk.includeInExceptions into own test
> 
> I don't think you really need to run the test with the othervm flag,
> unless you are concerned other tests may be setting this property and
> (incorrectly) not running in a separate VM, which would be a bug in my
> opinion. Well, then maybe you should run it in othervm just in case.
> Otherwise, looks ok to me.
> 
> --Sean
> 
> On 1/23/19 11:05 AM, Langer, Christoph wrote:
> > Hi,
> >
> > please review a small test update.
> >
> > Bug: https://bugs.openjdk.java.net/browse/JDK-8217657
> >
> > Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8217657.0/
> >
> > I'd like to move the test for the correct default value of security
> > property "jdk.includeInExceptions" into an own testcase in the
> > jdk.security area. This seems a bit more natural than to hide this check
> > in a java/net specific test and will help finding/maintaining tests when
> > the (default-)value for that property changes. For instance new values
> > get added or other OpenJDK builds have different defaults (e.g.
> SAPMachine).
> >
> > Thanks
> >
> > Christoph
> >


RE: RFR 8217657: Move the test for default value of jdk.includeInExceptions into own test

2019-01-24 Thread Langer, Christoph
Hi Goetz,

thanks for the review.

I've added the comment as you suggested: 
http://cr.openjdk.java.net/~clanger/webrevs/8217657.1/

Will run it through our nightly tests before submitting...

/Christoph


From: Lindenmaier, Goetz
Sent: Donnerstag, 24. Januar 2019 08:48
To: Langer, Christoph ; OpenJDK Dev list 
; OpenJDK Network Dev list 

Subject: RE: RFR 8217657: Move the test for default value of 
jdk.includeInExceptions into own test

Hi Christoph,

it is a good idea to keep testing these two matters separately.

Could you please document in the new test that in OpenJDK
it is decided to keep this property empty?
Something like:
@comment In OpenJDK, this property is empty by default and on purpose. This 
test assures the default is not changed.

Otherwise, looks good.

Best regards,
  Goetz.

From: net-dev 
mailto:net-dev-boun...@openjdk.java.net>> On 
Behalf Of Langer, Christoph
Sent: Mittwoch, 23. Januar 2019 17:06
To: OpenJDK Dev list 
mailto:security-...@openjdk.java.net>>; OpenJDK 
Network Dev list mailto:net-dev@openjdk.java.net>>
Subject: [CAUTION] RFR 8217657: Move the test for default value of 
jdk.includeInExceptions into own test

Hi,

please review a small test update.

Bug: https://bugs.openjdk.java.net/browse/JDK-8217657
Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8217657.0/

I'd like to move the test for the correct default value of security property 
"jdk.includeInExceptions" into an own testcase in the jdk.security area. This 
seems a bit more natural than to hide this check in a java/net specific test 
and will help finding/maintaining tests when the (default-)value for that 
property changes. For instance new values get added or other OpenJDK builds 
have different defaults (e.g. SAPMachine).

Thanks
Christoph



RFR 8217657: Move the test for default value of jdk.includeInExceptions into own test

2019-01-23 Thread Langer, Christoph
Hi,

please review a small test update.

Bug: https://bugs.openjdk.java.net/browse/JDK-8217657
Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8217657.0/

I'd like to move the test for the correct default value of security property 
"jdk.includeInExceptions" into an own testcase in the jdk.security area. This 
seems a bit more natural than to hide this check in a java/net specific test 
and will help finding/maintaining tests when the (default-)value for that 
property changes. For instance new values get added or other OpenJDK builds 
have different defaults (e.g. SAPMachine).

Thanks
Christoph



RE: 8217461: (ch) Add Net.available to return the number of bytes in the socket input buffer

2019-01-22 Thread Langer, Christoph
Hi Alan,

the change looks good to me.

In src/java.base/unix/native/libnio/ch/Net.c and 
src/java.base/windows/native/libnio/ch/Net.c you could change the code from

...
int n = NET_SocketAvailable(fdval(env, fdo), &count);
if (n != 0) {
...

to one line
if (NET_SocketAvailable(fdval(env, fdo), &count) != 0) {

as it is done in src/java.base/unix/native/libnet/PlainSocketImpl.c

The variable n is not used except for the returncode check.

By the way, looking at the 2 implementations of Net.c for unix resp. windows, 
it seems they have quite some code in common. Isn't that something where the 
shared parts could be merged into one file?

Best regards
Christoph


> -Original Message-
> From: nio-dev  On Behalf Of Alan
> Bateman
> Sent: Montag, 21. Januar 2019 20:10
> To: nio-...@openjdk.java.net; OpenJDK Network Dev list  d...@openjdk.java.net>
> Subject: 8217461: (ch) Add Net.available to return the number of bytes in the
> socket input buffer
> 
> This is a small change to add a method to sun.nio.ch.Net to get the
> number of bytes in the socket input buffer. The motive for adding this
> to make it possible for the socket adaptors to implement
> InputStream::available and also to support an alternative SocketImpl
> based on NIO. I've used the opportunity to clean up NET_SocketAvailable
> in libnet so that it returns 0 when it succeeds.
> 
> The proposed changes are here:
>    http://cr.openjdk.java.net/~alanb/8217461/webrev/index.html
> 
> -Alan


RE: RFR: 8207404: MulticastSocket tests failing on AIX

2019-01-20 Thread Langer, Christoph
Thanks, Steve and Chris for reviewing. Pushed to jdk12.

> -Original Message-
> From: Chris Hegarty 
> Sent: Freitag, 18. Januar 2019 15:15
> To: Langer, Christoph ; Steve Groeger
> 
> Cc: net-dev@openjdk.java.net; ppc-aix-port-...@openjdk.java.net
> Subject: Re: RFR: 8207404: MulticastSocket tests failing on AIX
> 
> 
> On 18/01/2019 08:54, Langer, Christoph wrote:
> > Hi,
> >
> > here is the updated webrev with modifications to the problem list and
> > reverting the static imports:
> > _http://cr.openjdk.java.net/~clanger/webrevs/8207404.1/__
> I think this is ok.
> 
> -Chris.


RE: RFR: 8207404: MulticastSocket tests failing on AIX

2019-01-18 Thread Langer, Christoph
Hi,

here is the updated webrev with modifications to the problem list and reverting 
the static imports: http://cr.openjdk.java.net/~clanger/webrevs/8207404.1/

It went fine through our test system so I think it's good now.

Please review.

Thanks
Christoph

From: Chris Hegarty 
Sent: Donnerstag, 17. Januar 2019 12:27
To: Langer, Christoph 
Cc: net-dev@openjdk.java.net; ppc-aix-port-...@openjdk.java.net
Subject: Re: RFR: 8207404: MulticastSocket tests failing on AIX




On 17 Jan 2019, at 10:55, Langer, Christoph 
mailto:christoph.lan...@sap.com>> wrote:

Hi,

here is a fix for the MulticastSocket tests failing on AIX (in certain 
sceanrios):

Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8207404.0/

Why remove the static imports from NetworkConfiguration? It makes
the code more verbose ( and IMO harder to read ).

-Chris.



RE: RFR: 8207404: MulticastSocket tests failing on AIX

2019-01-17 Thread Langer, Christoph
Hi Steve,

thanks for the review and the additional testing.

I’ll post an update tomorrow after I have results from our test system which 
will test all platforms that we have an eye on. I’ll roll back the change to 
the import statement and will also add an update to the problem list in order 
to make the tests run by default on AIX.

You could do me a favor though. There’s not only the issue with the 
IPV6_MULTICAST_IF setsockopt operation but there’s also the other where node 
local IPv6 multicast does not work when only the loopback interface is 
configured for IPv6 (as show in Test.java). Could you check this also with the 
AIX team whether they consider this a bug or working as expected?

Thanks & Best regards
Christoph

From: Steve Groeger 
Sent: Donnerstag, 17. Januar 2019 14:32
To: Langer, Christoph 
Cc: Chris Hegarty ; net-dev@openjdk.java.net; net-dev 
; ppc-aix-port-...@openjdk.java.net
Subject: RE: RFR: 8207404: MulticastSocket tests failing on AIX

Hi Christoph,

Have reviewed you changes and they look OK.

I have no preference with regards the static imports, but my view is keep the 
changes as simple as possible and to not make changes just for the sake of it.

Have tested your patches on a couple of AIX systems, both with IPv6 enabled but 
one didn't have an IPv6 address configured and the other did.
All the jdk/java/net/MulticastSocket tests passed. So the change also looks 
functionally OK.

Thanks
Steve Groeger
IBM Runtime Technologies
Hursley, Winchester
Tel: (44) 1962 816911  Mobex: 279990  Mobile: 07718 517 129
Fax (44) 1962 816800
Lotus Notes: Steve Groeger/UK/IBM
Internet: groe...@uk.ibm.com<mailto:groe...@uk.ibm.com>

Unless stated otherwise above:
IBM United Kingdom Limited - Registered in England and Wales with number 741598.
Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU



From:    "Langer, Christoph" 
mailto:christoph.lan...@sap.com>>
To:Chris Hegarty 
mailto:chris.hega...@oracle.com>>
Cc:
"ppc-aix-port-...@openjdk.java.net<mailto:ppc-aix-port-...@openjdk.java.net>" 
mailto:ppc-aix-port-...@openjdk.java.net>>, 
"net-dev@openjdk.java.net<mailto:net-dev@openjdk.java.net>" 
mailto:net-dev@openjdk.java.net>>
Date:17/01/2019 11:34
Subject:RE: RFR: 8207404: MulticastSocket tests failing on AIX
Sent by:"net-dev" 
mailto:net-dev-boun...@openjdk.java.net>>




Hi Chris,

for me it was the other way round… I did not see immediately where the methods 
came from and had to do some navigation. But I’m happy to revert this.

As I had only worked with AIX so far, I’ll put the test update into our test 
system to see whether there’ll be issues on other platforms. I’ll send an 
updated webrev tomorrow.

Best regards
Christoph

From: Chris Hegarty mailto:chris.hega...@oracle.com>>
Sent: Donnerstag, 17. Januar 2019 12:27
To: Langer, Christoph 
mailto:christoph.lan...@sap.com>>
Cc: net-dev@openjdk.java.net<mailto:net-dev@openjdk.java.net>; 
ppc-aix-port-...@openjdk.java.net<mailto:ppc-aix-port-...@openjdk.java.net>
Subject: Re: RFR: 8207404: MulticastSocket tests failing on AIX




On 17 Jan 2019, at 10:55, Langer, Christoph 
mailto:christoph.lan...@sap.com>> wrote:

Hi,

here is a fix for the MulticastSocket tests failing on AIX (in certain 
sceanrios):

Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8207404.0/

Why remove the static imports from NetworkConfiguration? It makes
the code more verbose ( and IMO harder to read ).

-Chris.



Unless stated otherwise above:
IBM United Kingdom Limited - Registered in England and Wales with number 741598.
Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU


RE: RFR: 8207404: MulticastSocket tests failing on AIX

2019-01-17 Thread Langer, Christoph
Hi Chris,

for me it was the other way round... I did not see immediately where the 
methods came from and had to do some navigation. But I'm happy to revert this.

As I had only worked with AIX so far, I'll put the test update into our test 
system to see whether there'll be issues on other platforms. I'll send an 
updated webrev tomorrow.

Best regards
Christoph

From: Chris Hegarty 
Sent: Donnerstag, 17. Januar 2019 12:27
To: Langer, Christoph 
Cc: net-dev@openjdk.java.net; ppc-aix-port-...@openjdk.java.net
Subject: Re: RFR: 8207404: MulticastSocket tests failing on AIX




On 17 Jan 2019, at 10:55, Langer, Christoph 
mailto:christoph.lan...@sap.com>> wrote:

Hi,

here is a fix for the MulticastSocket tests failing on AIX (in certain 
sceanrios):

Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8207404.0/

Why remove the static imports from NetworkConfiguration? It makes
the code more verbose ( and IMO harder to read ).

-Chris.



RE: RFR(XS): 8217311: Improve Exception thrown when MulticastSocket.setInterface fails on AIX(Unix)

2019-01-17 Thread Langer, Christoph
Thanks Alan and Steve for the reviews.

From: net-dev  On Behalf Of Alan Bateman
Sent: Donnerstag, 17. Januar 2019 11:45
To: net-dev@openjdk.java.net
Subject: Re: RFR(XS): 8217311: Improve Exception thrown when 
MulticastSocket.setInterface fails on AIX(Unix)

Looks okay to me too.  I think we should encourage new multicast applications 
to move to DatagramChannel as it a defines factory methods to specify the 
protocol family at creation time. Also setOption(IP_MULTICAST_IF, ...) and the 
join methods are also specified for such cases. Going forward I also need we 
need to replace PlainDatagramSocketImpl but that's a topic for another thread.

-Alan
On 17/01/2019 10:18, Steve Groeger wrote:
Hi Chris,

Looks OK to me.

That is one of the changes I had made locally while investigating  
https://bugs.openjdk.java.net/browse/JDK-8207404, so happy to accept it.

Thanks
Steve Groeger
IBM Runtime Technologies
Hursley, Winchester
Tel: (44) 1962 816911  Mobex: 279990  Mobile: 07718 517 129
Fax (44) 1962 816800
Lotus Notes: Steve Groeger/UK/IBM
Internet: groe...@uk.ibm.com<mailto:groe...@uk.ibm.com>

Unless stated otherwise above:
IBM United Kingdom Limited - Registered in England and Wales with number 741598.
Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU



From:    "Langer, Christoph" 
<mailto:christoph.lan...@sap.com>
To:"net-dev@openjdk.java.net"<mailto:net-dev@openjdk.java.net> 
<mailto:net-dev@openjdk.java.net>
Date:17/01/2019 09:55
Subject:RFR(XS): 8217311: Improve Exception thrown when 
MulticastSocket.setInterface fails on AIX(Unix)
Sent by:"net-dev" 
<mailto:net-dev-boun...@openjdk.java.net>




Hi,

please help to review a tiny fix.

While working on the issue with the AIX multicast tests 
(https://bugs.openjdk.java.net/browse/JDK-8207404), I found a place where a 
SocketException thrown in a specific error case could be improved. There 
already exists code to throw a SocketException with the text "IPV6_MULTICAST_IF 
failed (interface has IPv4 address only?) " when setsockopt(IPV6_MULTICAST_IF) 
fails with EINVAL. On AIX, when you have IPv4 addresses only on an interface, 
you’ll run into when setting the option on an AF_INET6 socket. So that specific 
SocketException text would match very well then. I guess the addition is fine 
to be platform independent. At best on other platforms EADDRNOTAVAIL is never 
seen at that place. Or if it was, it would certainly have to do with an IP 
address problem on the interface...

Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8217311.0/
Bug: https://bugs.openjdk.java.net/browse/JDK-8217311

Thanks and best regards
Christoph



Unless stated otherwise above:
IBM United Kingdom Limited - Registered in England and Wales with number 741598.
Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU



RFR: 8207404: MulticastSocket tests failing on AIX

2019-01-17 Thread Langer, Christoph
Hi,

here is a fix for the MulticastSocket tests failing on AIX (in certain 
sceanrios):

Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8207404.0/
Bug: https://bugs.openjdk.java.net/browse/JDK-8207404

First of all, I "modernized" the testcases 
test/jdk/java/net/MulticastSocket/SetGetNetworkInterfaceTest.java and 
test/jdk/java/net/MulticastSocket/Test.java to make use of 
test/lib/jdk/test/lib/NetworkConfiguration.java which probes for the network 
configuration and returns lists of local interfaces or addresses for certain 
use cases. In NetworkConfiguration.java I made some cleanups and enhancements 
which will hopefully find some appreciation. These 3 tests will now also be 
executed with and without -Djava.net.preferIPv4Stack=true.

The actual fixes are:
test/jdk/java/net/MulticastSocket/JoinLeave.java:
test/jdk/java/net/MulticastSocket/SetGetNetworkInterfaceTest.java:
-> NetworkConfiguration.ip4MulticastInterfaces() will now, on AIX, in a 
scenario where IPv6 is available, filter out interfaces that only have IPv4 
addresses (line 121ff in test/lib/jdk/test/lib/NetworkConfiguration.java)

test/jdk/java/net/MulticastSocket/Test.java:
-> The test for node local multicasting is only executed, when 
NetworkConfiguration::hasTestableIPv6Address is true. For AIX it'll be the case 
only if a "real" IPv6 address exists (on some interface) which is not loopback 
or wildcard. This is how it has already been for Solaris. For other operating 
systems, any IPv6 address on any interface is sufficient for running the test. 
The coding for the AIX/Solaris check can be found in line 66ff of 
test/lib/jdk/test/lib/NetworkConfiguration.java.

As it is a test only fix, I request to do it in JDK12.

Thanks & Best regards
Christoph



RFR(XS): 8217311: Improve Exception thrown when MulticastSocket.setInterface fails on AIX(Unix)

2019-01-17 Thread Langer, Christoph
Hi,

please help to review a tiny fix.

While working on the issue with the AIX multicast tests 
(https://bugs.openjdk.java.net/browse/JDK-8207404), I found a place where a 
SocketException thrown in a specific error case could be improved. There 
already exists code to throw a SocketException with the text "IPV6_MULTICAST_IF 
failed (interface has IPv4 address only?) " when setsockopt(IPV6_MULTICAST_IF) 
fails with EINVAL. On AIX, when you have IPv4 addresses only on an interface, 
you'll run into when setting the option on an AF_INET6 socket. So that specific 
SocketException text would match very well then. I guess the addition is fine 
to be platform independent. At best on other platforms EADDRNOTAVAIL is never 
seen at that place. Or if it was, it would certainly have to do with an IP 
address problem on the interface...

Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8217311.0/
Bug: https://bugs.openjdk.java.net/browse/JDK-8217311

Thanks and best regards
Christoph



RE: RFR 8007606 : Handle realloc() failure in unix/native/libnet/net_util_md.c correctly

2019-01-15 Thread Langer, Christoph
Hi Ivan,

yes, sure, push it 😊

Best regards
Christoph

> -Original Message-
> From: Ivan Gerasimov 
> Sent: Dienstag, 15. Januar 2019 20:53
> To: Baesken, Matthias ; net-
> d...@openjdk.java.net; Langer, Christoph 
> Subject: Re: RFR 8007606 : Handle realloc() failure in
> unix/native/libnet/net_util_md.c correctly
> 
> Hello!
> 
> Do you think it is good to go now?
> 
> With kind regards,
> 
> Ivan
> 
> 
> On 1/11/19 11:30 AM, Ivan Gerasimov wrote:
> > Good catch, thank you!
> >
> > Indeed, if we don't reset localifsSize then we could end up accessing
> > already freed memory, which is worse than just a memory leak.
> >
> > Here's the updated webrev:
> >
> > http://cr.openjdk.java.net/~igerasim/8007606/01/webrev/
> >
> > With kind regards,
> > Ivan
> >
> > On 1/11/19 4:43 AM, Baesken, Matthias wrote:
> >> Hi Ivan,
> >>
> >> Shouldn't you resetlocalifsSize to 0   in case of  the early
> >> return ?  The comment says  localifsSize is the size of the array so
> >> the size of the array is 0 again after freeing.
> >>
> >>
> >> 637 static struct localinterface *localifs = 0;
> >>   638 static int localifsSize = 0;/* size of array */
> >>   639 static int nifs = 0;/* number of entries used in
> >> array */
> >>
> >> ...
> >>
> >> 679 if (localifsTemp == 0) {
> >>   680 free(localifs);
> >>   681 localifs = 0;
> >>   682 nifs = 0;
> >>   683 fclose(f);
> >>   684 return;
> >>   685 }
> >>
> >>
> >>
> >>
> >> Best regards, Matthias
> >>
> >>
> >>
> >>> Date: Thu, 10 Jan 2019 20:29:08 -0800
> >>> From: Ivan Gerasimov 
> >>> To: "net-dev@openjdk.java.net" 
> >>> Subject: RFR 8007606 : Handle realloc() failure in
> >>> unix/native/libnet/net_util_md.c correctly
> >>> Message-ID: <3dc3c26b-fea7-2538-2c7a-bfa623f2f...@oracle.com>
> >>> Content-Type: text/plain; charset=utf-8; format=flowed
> >>>
> >>> Hello!
> >>>
> >>> This seems to be the last use of realloc() without proper handling of a
> >>> failure.
> >>>
> >>> Would you please help review a trivial fix?
> >>>
> >>> BUGURL: https://bugs.openjdk.java.net/browse/JDK-8007606
> >>> WEBREV: http://cr.openjdk.java.net/~igerasim/8007606/00/webrev/
> >>>
> >>> Thanks in advance!
> >>>
> >>> --
> >>> With kind regards,
> >>> Ivan Gerasimov
> >>>
> >>>
> >>
> >
> 
> --
> With kind regards,
> Ivan Gerasimov



RE: 8207404: MulticastSocket tests failing on Aix

2019-01-15 Thread Langer, Christoph
Hi Steve,

I have looked into this topic. I started some work in that area a few years ago 
and never finished it. I think the root cause of the issue got already pointed 
out in this discussion.

The JVM (correctly) detects that the system is capable of IPv6 and hence opens 
new sockets with address family AF_INET6. This type of socket shall allow for 
communication in both domains, IPv6 and IPv4. To ensure the dual stack socket, 
we also set socket option IPV6_V6ONLY to '0' which should make sure it's not an 
IPv6 only socket. This is the way, the standard TCP/UDP socket communication 
works in the JDK, also on AIX.

Now, for multicasting, the VM wants to set the multicast interface. At that 
point, the socket is not yet bound to a local address or has joined a multicast 
group which might restrict the address type. So, we need either 
setsockopt(IPV6_MULTICAST_IF) or setsockopt(IP_MULTICAST_IF) to work. The 
documentation cited by Volker 
(https://www.ibm.com/support/knowledgecenter/ssw_ibm_i_71/apis/ssocko.htm) is 
quite explicit in that IP_MULTICAST_IF is for AF_INET sockets and 
IPV6_MULTICAST_IF is for AF_INET6 type sockets. So I would expect, that 
IPV6_MULTICAST_IF works, even for an interface without configured IPv6 address.

So, while this is nothing for the compiler folks, it is something for the AIX 
runtime/network people. Can you address that within IBM? Maybe the AIX experts 
have a suggestion if there's already a way to code around this. And if it's not 
possible to get this to work with the existing APIs, would it be possible for 
you to open a PMR with IBM or find any other means to get that fixed in the OS?

Anyway, I think even if IBM can fix this in AIX, it'll be in future versions 
only. So I'll go ahead and create a patch for modifying the failing tests to be 
more tolerant to such type of system configurations on AIX. I will send an RFR 
mail soon.

Best regards
Christoph


From: net-dev  On Behalf Of Steve Groeger
Sent: Montag, 14. Januar 2019 15:31
To: Volker Simonis 
Cc: ppc-aix-port-...@openjdk.java.net; net-dev 
Subject: Re: 8207404: MulticastSocket tests failing on Aix

Hi Volker,

The reason the mcast_set_if_by_if_v4(env, this, fd, value); probably fails
is that the socket that we are trying to set the IPPROTO_IP, IP_MULTICAST_IF 
options
on has been created with the AF_INET6 family, and these options may not be 
valid.

The socket is setup with the AF_INET6 family because ipv6_available() returns 
true,
as the AIX system does support INET6/IPv6. It is just that the interface we are 
using
doesnt have an INET6 address associated with it.

I am checking with the AIX/C compiler team in IBM to see if this a bug or not.

Thanks
Steve Groeger
IBM Runtime Technologies
Hursley, Winchester
Tel: (44) 1962 816911  Mobex: 279990  Mobile: 07718 517 129
Fax (44) 1962 816800
Lotus Notes: Steve Groeger/UK/IBM
Internet: groe...@uk.ibm.com

Unless stated otherwise above:
IBM United Kingdom Limited - Registered in England and Wales with number 741598.
Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU



From:Volker Simonis 
mailto:volker.simo...@gmail.com>>
To:Steve Groeger mailto:groe...@uk.ibm.com>>
Cc:Chris Hegarty 
mailto:chris.hega...@oracle.com>>, net-dev 
mailto:net-dev@openjdk.java.net>>, 
ppc-aix-port-...@openjdk.java.net
Date:14/01/2019 14:11
Subject:Re: 8207404: MulticastSocket tests failing on Aix




On Mon, Jan 14, 2019 at 2:19 PM Steve Groeger 
mailto:groe...@uk.ibm.com>> wrote:
>
> Hi Chris / Volker,
>
> I had already tried doing the same as is done for Linux, ie calling this first
>
> mcast_set_if_by_if_v4(env, this, fd, value);
>
> This still fails for some reason with EADDRNOTAVAIL.

Finding this out is probably the main challenge in order to solve this
issue and one of the reason why we asked IBM for assistance :)

I suppose there must be a possibility to set a socket option for an
IPv4 socket on AIX even if IPv6 is enabled. Otherwise this is probably
an AIX problem/bug which should be fixed there (where, again, we need
assistance from IBM).

The man-page 
(https://www.ibm.com/support/knowledgecenter/ssw_ibm_i_71/apis/ssocko.htm)
mentions the following:

IP_MULTICAST_IF: et interface over which outgoing multicast datagrams
should be sent. This option is only supported for sockets with an
address family of AF_INET and type of SOCK_DGRAM or SOCK_RAW.

Maybe the interface is erroneously in the AF_INET6 family? The
man-page only mentions the following for EADDRNOTAVAIL:

EADDRNOTAVAIL: Address not available. For the IP_ADD_MEMBERSHIP or
IP_DROP_MEMBERSHIP operations, this error code indicates that an
incorrect address was specified for either the imr_multiaddr or
imr_interface parameter value.

> This error gets cleared by the
>
> if ((*env)->ExceptionCheck(env)){
>(*env)->ExceptionClear(env);
> }
>
> as ipv6_

RE: RFR 8007606 : Handle realloc() failure in unix/native/libnet/net_util_md.c correctly

2019-01-11 Thread Langer, Christoph
Hi,

that's right, good catch. Either set localifs to 0 or maybe even keep the old 
pointer with the old value of localifs. I guess the case is a bit theoretical 
but it should be done right.

In line 695 fclose (f); the formatting can be fixed (also remove space 
between fclose and the bracket).

Thanks
Christoph


> -Original Message-
> From: net-dev  On Behalf Of
> Baesken, Matthias
> Sent: Freitag, 11. Januar 2019 13:43
> To: net-dev@openjdk.java.net
> Subject: [CAUTION] Re: RFR 8007606 : Handle realloc() failure in
> unix/native/libnet/net_util_md.c correctly
> 
> Hi Ivan,
> 
> Shouldn't you resetlocalifsSize to 0   in case of  the early return ?  The
> comment says  localifsSize is the size of the array so the size of the array 
> is 0
> again after freeing.
> 
> 
> 637 static struct localinterface *localifs = 0;
>  638 static int localifsSize = 0;/* size of array */
>  639 static int nifs = 0;/* number of entries used in array */
> 
>...
> 
> 679 if (localifsTemp == 0) {
>  680 free(localifs);
>  681 localifs = 0;
>  682 nifs = 0;
>  683 fclose(f);
>  684 return;
>  685 }
> 
> 
> 
> 
> Best regards, Matthias
> 
> 
> 
> > Date: Thu, 10 Jan 2019 20:29:08 -0800
> > From: Ivan Gerasimov 
> > To: "net-dev@openjdk.java.net" 
> > Subject: RFR 8007606 : Handle realloc() failure in
> > unix/native/libnet/net_util_md.c correctly
> > Message-ID: <3dc3c26b-fea7-2538-2c7a-bfa623f2f...@oracle.com>
> > Content-Type: text/plain; charset=utf-8; format=flowed
> >
> > Hello!
> >
> > This seems to be the last use of realloc() without proper handling of a
> > failure.
> >
> > Would you please help review a trivial fix?
> >
> > BUGURL: https://bugs.openjdk.java.net/browse/JDK-8007606
> > WEBREV: http://cr.openjdk.java.net/~igerasim/8007606/00/webrev/
> >
> > Thanks in advance!
> >
> > --
> > With kind regards,
> > Ivan Gerasimov
> >
> >



RE: RFR 8007606 : Handle realloc() failure in unix/native/libnet/net_util_md.c correctly

2019-01-10 Thread Langer, Christoph
Hi Ivan,

looks good.

Best regards
Christoph

> -Original Message-
> From: net-dev  On Behalf Of Ivan
> Gerasimov
> Sent: Freitag, 11. Januar 2019 05:29
> To: net-dev@openjdk.java.net
> Subject: RFR 8007606 : Handle realloc() failure in
> unix/native/libnet/net_util_md.c correctly
> 
> Hello!
> 
> This seems to be the last use of realloc() without proper handling of a
> failure.
> 
> Would you please help review a trivial fix?
> 
> BUGURL: https://bugs.openjdk.java.net/browse/JDK-8007606
> WEBREV: http://cr.openjdk.java.net/~igerasim/8007606/00/webrev/
> 
> Thanks in advance!
> 
> --
> With kind regards,
> Ivan Gerasimov



RE: [CAUTION] RE: RFR 8216355: missing NULL checks in libnet in interface iteration and potential resource leak in getMacAddress

2019-01-09 Thread Langer, Christoph
Hi Matthias,

looks good to me.

In src/java.base/unix/native/libnet/NetworkInterface.c, lines 1019 and 1034, 
I'd prefer if the style was:
if (addr == NULL) {
return 0;
}

Thanks
Christoph


From: net-dev  On Behalf Of Baesken, Matthias
Sent: Mittwoch, 9. Januar 2019 10:02
To: net-dev 
Cc: Robin Westberg 
Subject: [CAUTION] RE: RFR 8216355: missing NULL checks in libnet in interface 
iteration and potential resource leak in getMacAddress

New webrev :

http://cr.openjdk.java.net/~mbaesken/webrevs/8216355.2/


( I noticed that I accidentally  deleted a line in the AIX coding so I brought 
it back,  and I updated the Copyright  year  info to 2019 ).

Best regards, Matthias


From: Baesken, Matthias
Sent: Mittwoch, 9. Januar 2019 09:36
To: net-dev mailto:net-dev@openjdk.java.net>>
Cc: 'Robin Westberg' 
mailto:robin.westb...@oracle.com>>
Subject: RFR 8216355: missing NULL checks in libnet in interface iteration and 
potential resource leak in getMacAddress

Hello, please review the following fix .

In NetworkInterface.c and Inet6AddressImpl.c we have some coding that omits 
checking for ifa_addr == NULL when iterating on the result of the getifaddrs 
call.
This is similar to what has been fixed in hotspot with

https://bugs.openjdk.java.net/browse/JDK-8208676

The issues are in the bsd/macOS coding. However bsd/macOS also document that 
ifa_addr can be NULL in special cases (not sure how likely it is to see it "in 
the wild").
See


https://www.freebsd.org/cgi/man.cgi?getifaddrs

https://developer.apple.com/library/archive/documentation/System/Conceptual/ManPages_iPhoneOS/man3/getifaddrs.3.html


Additionally  a small resource  leak  in NetworkInterface.c   is fixed.

Bug/webrev :

https://bugs.openjdk.java.net/browse/JDK-8216355


http://cr.openjdk.java.net/~mbaesken/webrevs/8216355.1/


Best regards, Matthias


RE: [CAUTION] RFR : 8211146 : fix problematic elif-tests after recent gcc warning changes Werror=undef

2018-09-26 Thread Langer, Christoph
Hi Matthias,

looks good (and trivial). Ccing serviceability-dev because of change in libjdwp.

Best regards
Christoph

From: nio-dev  On Behalf Of Baesken, Matthias
Sent: Mittwoch, 26. September 2018 11:25
To: 'build-...@openjdk.java.net' ; net-dev 
; nio-...@openjdk.java.net
Cc: Lindenmaier, Goetz ; Schmidt, Lutz 

Subject: [CAUTION] RFR : 8211146 : fix problematic elif-tests after recent gcc 
warning changes Werror=undef

Hello, please review this small build fix .
After the recent  changes  to  the gcc compile flags   with  8211029,  most 
of our  Linux builds stopped working .

Error :

=== Output from failing command(s) repeated here ===
* For target support_native_java.base_libnet_DatagramPacket.o:
In file included from 
/OpenJDK/8210319/jdk/src/java.base/share/native/libnet/net_util.h:31:0,
from 
/OpenJDK/8210319/jdk/src/java.base/share/native/libnet/DatagramPacket.c:27:
/OpenJDK/8210319/jdk/src/java.base/unix/native/libnet/net_util_md.h:50:7: 
error: "__solaris__" is not defined [-Werror=undef]
#elif __solaris__
   ^

After looking into it,  it seems  that  the   
jdk/src/java.base/unix/native/libnet/net_util_md.hcompile error is only 
triggered on older Linux OS  (e.g. SLES11).
Probably that's why it was not seen at Oracle  after  puhsing  after 8211029   .

Some greps  showed me a number of similar problematic  #elif-checks for OS, I 
adjusted them too .


Bug / webrev :

https://bugs.openjdk.java.net/browse/JDK-8211146


http://cr.openjdk.java.net/~mbaesken/webrevs/8211146.0/


Thanks, Matthias


RE: RFR of JDK-8210802,temp files left by tests in jdk/java/net/httpclient

2018-09-17 Thread Langer, Christoph
Hi Hamlin,

no, I don't insist. You should probably wait for another review from some of 
the folks that are more experienced with the http client and its testing, 
though.

Best regards
Christoph

> -Original Message-
> From: Hamlin Li 
> Sent: Montag, 17. September 2018 11:02
> To: Langer, Christoph 
> Cc: OpenJDK Network Dev list 
> Subject: Re: RFR of JDK-8210802,temp files left by tests in
> jdk/java/net/httpclient
> 
> Hi Christoph,
> 
> Thank you for review.
> 
> Normally I prefer to use "finally" too.
> 
> But for this case, it's simpler to use deleteOnExit(), and for
> TestUtil.java and Driver.java it can not use "finally", because the temp
> files will be used outside of methods.
> 
> If you insist on "finally" I can modify them except of TestUtil.java and
> Driver.java.
> 
> Thank you
> 
> -Hamlin
> 
> 
> On 2018/9/17 4:25 PM, Langer, Christoph wrote:
> > Hi Hamlin,
> >
> > wouldn't it be better/cleaner to move the deletion of files into finally
> blocks? But I guess one can do it with deleteOnExit() as well...
> >
> > Best regards
> > Christoph
> >
> >> -Original Message-
> >> From: net-dev  On Behalf Of
> Hamlin Li
> >> Sent: Montag, 17. September 2018 07:55
> >> To: OpenJDK Network Dev list 
> >> Subject: RFR of JDK-8210802,temp files left by tests in
> jdk/java/net/httpclient
> >>
> >> Would you please review the following patch?
> >>
> >> bug: https://bugs.openjdk.java.net/browse/JDK-8210802
> >> webrev: http://cr.openjdk.java.net/~mli/8210802/webrev.00/
> >>
> >> Thank you
> >> -Hamlin



RE: RFR of JDK-8210802,temp files left by tests in jdk/java/net/httpclient

2018-09-17 Thread Langer, Christoph
Hi Hamlin,

wouldn't it be better/cleaner to move the deletion of files into finally 
blocks? But I guess one can do it with deleteOnExit() as well...

Best regards
Christoph

> -Original Message-
> From: net-dev  On Behalf Of Hamlin Li
> Sent: Montag, 17. September 2018 07:55
> To: OpenJDK Network Dev list 
> Subject: RFR of JDK-8210802,temp files left by tests in 
> jdk/java/net/httpclient
> 
> Would you please review the following patch?
> 
> bug: https://bugs.openjdk.java.net/browse/JDK-8210802
> webrev: http://cr.openjdk.java.net/~mli/8210802/webrev.00/
> 
> Thank you
> -Hamlin


RE: RFR [XS]: 8210147: adjust some WSAGetLastError usages in windows network coding - was : RE: WSAGetLastError usage in jdk/src/java.base/windows/native/libnet/SocketInputStream.c

2018-08-30 Thread Langer, Christoph
Hi Matthias,

> Can I have a second review please ?
+1

Best regards
Christoph



RE: RFR : 8205959 : Do not restart close if errno is EINTR

2018-07-02 Thread Langer, Christoph
Hi Matthias,

forwarding to serviceability-dev, because debugging is usually discussed there.

Yes, I would think this coding should be fixed, too. Can you open a bug and 
prepare a change?

Thanks
Christoph

> -Original Message-
> From: net-dev [mailto:net-dev-boun...@openjdk.java.net] On Behalf Of
> Norman Maurer
> Sent: Montag, 2. Juli 2018 10:23
> To: Baesken, Matthias 
> Cc: Stuefe, Thomas ; net-dev@openjdk.java.net
> Subject: Re: RFR : 8205959 : Do not restart close if errno is EINTR
> 
> +1 retry a close on EINTR has most likely not the outcome you expect and
> may even close a wrong FD if the same FD is reused already (as even if EINTR
> is returned it may have closed the FD)
> 
> > Am 02.07.2018 um 10:17 schrieb Baesken, Matthias
> :
> >
> > Hello  ,  there is a similar pattern (attempt to restart close in case of 
> > EINTR)
> in the coding as well   in  socket_md.c   :
> >
> > src/jdk.jdwp.agent/unix/native/libdt_socket/socket_md.c-147-int rv;
> > src/jdk.jdwp.agent/unix/native/libdt_socket/socket_md.c-148-do {
> > src/jdk.jdwp.agent/unix/native/libdt_socket/socket_md.c-149-rv =
> close(fd);
> > src/jdk.jdwp.agent/unix/native/libdt_socket/socket_md.c:150:} while (rv
> == -1 && errno == EINTR);
> > src/jdk.jdwp.agent/unix/native/libdt_socket/socket_md.c-151-
> > src/jdk.jdwp.agent/unix/native/libdt_socket/socket_md.c-152-return rv;
> > src/jdk.jdwp.agent/unix/native/libdt_socket/socket_md.c-153-}
> >
> > Do you think this needs adjustment   (on LINUX)  as well ?
> >
> > Best regards, Matthias
> >
> >
> >> Message: 2
> >> Date: Thu, 28 Jun 2018 18:19:46 +0100
> >> From: Alan Bateman 
> >> To: David Lloyd , ivan.gerasi...@oracle.com
> >> Cc: OpenJDK Network Dev list 
> >> Subject: Re: RFR : 8205959 : Do not restart close if errno is EINTR
> >> Message-ID: <3fd1496f-ab83-a2d5-0699-13c8b735d...@oracle.com>
> >> Content-Type: text/plain; charset=utf-8; format=flowed
> >>
> >>> On 28/06/2018 17:35, David Lloyd wrote:
> >>> :
> >>> Do you (or Alan) think that this might have accounted for real-world
> >>> connection problems?
> >>>
> >> In the file I/O area, with NFS I think, we had an issue a long time ago
> >> where close was retried after EIO. That issue was fixed a long time ago
> >> but it's one that comes to mind in this general area.
> >>
> >> -Alan
> >>
> >


RE: RFR:8194298 Add support for per Socket configuration of TCP keepalive

2018-05-15 Thread Langer, Christoph
Hi,

As I had written previously, I’m still unclear whether the naming of the MacOS 
implementation files is the best choice.

Currently it is:
src/jdk.net/macosx/classes/jdk/net/UnixSocketOptions.java
src/jdk.net/macosx/native/libextnet/UnixSocketOptions.c

Maybe it  should rather be:
src/jdk.net/macosx/classes/jdk/net/MacOSXSocketOptions.java
src/jdk.net/macosx/native/libextnet/MacOSXSocketOptions.c

Since for Linux we have:
src/jdk.net/linux/classes/jdk/net/LinuxSocketOptions.java
src/jdk.net/linux/native/libextnet/LinuxSocketOptions.c

I’m asking because I’m planning to add some AIX options and will have to choose 
a name for this implementation eventually.

@Alan: What do you think?

Best regards
Christoph


From: net-dev [mailto:net-dev-boun...@openjdk.java.net] On Behalf Of vyom tewari
Sent: Montag, 14. Mai 2018 17:31
To: Alan Bateman ; OpenJDK Network Dev list 

Subject: Re: RFR:8194298 Add support for per Socket configuration of TCP 
keepalive


Hi All,

Please find the latest 
webrev(http://cr.openjdk.java.net/~vtewari/8194298/webrev0.5/index.html). Only 
change with the previous wrev(04) is i removed "socket type" as Alan suggested 
and used the default  constructor (Set> options = new 
HashSet<>();) in ExtendedSocketOptions.java

Thanks,

Vyom

On Sunday 13 May 2018 12:37 PM, Alan Bateman wrote:
On 12/05/2018 10:21, vyom tewari wrote:

:

Thanks for review, please find the updated 
webrev(http://cr.openjdk.java.net/~vtewari/8194298/webrev0.4/index.html).
I've skimmed through this webrev.

The spec for the new options mostly look good but all three include "The exact 
semantics of this socket option are socket type and system dependent". I assume 
"socket type" should be dropped from this as these are TCP options. Maybe you 
can borrow text from the TCP_NODELAY option where it has the statement "The 
socket option is specific to stream-oriented sockets using the TCP/IP protocol".

The changes to the NetworkChannel implementations look okay. The changes to the 
NIO tests looks okay too but would be good if you could keep the formatting 
consistent with the existing code if you can.

Ivan had good comments so I didn't spent as much time on that code and it seems 
very reviewed.

-Alan.




RE: RFR(XS): 8202181: Correctly specify size of hostname buffer in Unix Inet*AddressImpl_getLocalHostName implementations

2018-04-30 Thread Langer, Christoph
Hi Chris,

thanks for looking at this. I just received positive confirmation from 
jdk-submit. Our internal tests did not show regressions as well. So I'll wait 
for word on your tests before submitting.

Thanks
Christoph

> -Original Message-
> From: Chris Hegarty [mailto:chris.hega...@oracle.com]
> Sent: Montag, 30. April 2018 11:45
> To: Langer, Christoph ; Thomas Stüfe
> 
> Cc: net-dev@openjdk.java.net
> Subject: Re: RFR(XS): 8202181: Correctly specify size of hostname buffer in
> Unix Inet*AddressImpl_getLocalHostName implementations
> 
> 
> On 30/04/18 09:00, Langer, Christoph wrote:
> > ...
> >>> Taking this input I have updated my webrev to round things up a little
> bit:
> >> http://cr.openjdk.java.net/~clanger/webrevs/8202181.1/
> Thank you Christophe, this latest version looks good.
> I'll run some tests on it and report the results by tomorrow.
> 
> -Chris.
> 
> P.S. I do remember adding the unconditional null termination
> to this code years ago, but I suspect that it's been touched
> a few times since.


RE: RFR(XS): 8202181: Correctly specify size of hostname buffer in Unix Inet*AddressImpl_getLocalHostName implementations

2018-04-30 Thread Langer, Christoph
Hi Thomas,

thanks for reviewing.

The test has also passed our internal testing. I'll run it through jdk-submit 
and then push it in the course of the day.

Best regards
Christoph

> -Original Message-
> From: Thomas Stüfe [mailto:thomas.stu...@gmail.com]
> Sent: Freitag, 27. April 2018 18:15
> To: Langer, Christoph 
> Cc: vyom tewari ; net-dev@openjdk.java.net;
> Brian Burkhalter 
> Subject: Re: RFR(XS): 8202181: Correctly specify size of hostname buffer in
> Unix Inet*AddressImpl_getLocalHostName implementations
> 
> Hi Christoph,
> 
> On Fri, Apr 27, 2018 at 9:35 AM, Langer, Christoph
>  wrote:
> > Hi all,
> >
> > thanks for looking into this. Here are a few comments
> >
> > First of all, there are no real life issues I have seen with this. It is 
> > just
> something that occurred to me when working with the code. But, why not fix
> it, even it is a corner case that might never happen.
> >
> > @Thomas: As for the zero termination of the hostname result after the call
> to gethostname: Yes, we should unconditionally terminate the result, which
> we do. Unfortunately this part of code cannot be moved outside the solaris
> #ifdef because the part in the #ifdef contains variable declarations. And you
> know - the C compatibility issue...
> >
> 
> Ok.
> 
> > I looked again into the macro definitions for for HOST_NAME_MAX and
> NI_MAXHOST. HOST_NAME_MAX is mentioned in the gethostname docs
> ([1] and [2]). Glibc docs indicate it is 64 Byte or 255 Byte. So it looks 
> like it is a
> quite small buffer, compared to NI_MAXHOST from netdb.h, which is the
> value that getnameinfo likes to work with, as per [3]. Posix genameinfo doc
> ([4]) does not mention NI_MAXHOST but Linux doc says it is 1025 which is
> what we'd define if it is not set.
> >
> 
> Okay, thanks for the research! This is weird, why two different
> defines for the same thing.
> 
> The only (probably highly theoretical) problem I see is that there may
> be platforms which do not define NI_MAXHOST but where
> HOST_MAX_NAME is
> defined and larger than 1025 char sized output buffers. Then, we would
> artificially limit ourselves to 1025 characters. (Was Matthias not
> working on a problem with truncated host names in our hpux port?).
> 
> One could in theory solve this by falling back to HOST_MAX_NAME if
> NI_MAXHOST is undefined:
> 
> #ifdnef NI_MAXHOST
> #ifdef HOST_MAX_NAME
> #define NI_MAXHOST HOST_MAX_NAME
> #else
> #define NI_MAXHOST 1025
> #endif
> #endif
> 
> However, I am fine with your current patch. I think redefinition of
> NI_MAXHOST - if even necessary - should be done in a follow up issue.
> 
> > Taking this input I have updated my webrev to round things up a little bit:
> http://cr.openjdk.java.net/~clanger/webrevs/8202181.1/
> >
> > I moved the definition of NI_MAXHOST into net_util_md.h and updated
> the comment a little bit to make clearer why it is there. In 
> Inet4AddressImpl.c
> and Inet6AddressImpl.c I also fixed the other places where getnameinfo is
> called to use sizeof(buffer) instead of NI_MAXHOST.
> >
> 
> All looks well. Again, thanks for the research.
> 
> ... Thomas
> 
> > Best regards
> > Christoph
> >
> > [1] http://man7.org/linux/man-pages/man2/gethostname.2.html
> > [2]
> http://pubs.opengroup.org/onlinepubs/009695399/functions/gethostname.
> html
> > [3] http://man7.org/linux/man-pages/man3/getnameinfo.3.html
> > [4]
> http://pubs.opengroup.org/onlinepubs/9699919799/functions/getnameinfo.
> html
> >
> >
> >> -Original Message-
> >> From: vyom tewari [mailto:vyom.tew...@oracle.com]
> >> Sent: Freitag, 27. April 2018 08:38
> >> To: Thomas Stüfe 
> >> Cc: Langer, Christoph ; net-
> >> d...@openjdk.java.net
> >> Subject: Re: RFR(XS): 8202181: Correctly specify size of hostname buffer in
> >> Unix Inet*AddressImpl_getLocalHostName implementations
> >>
> >>
> >>
> >> On Friday 27 April 2018 10:58 AM, Thomas Stüfe wrote:
> >> > On Fri, Apr 27, 2018 at 5:57 AM, vyom tewari
> 
> >> wrote:
> >> >> Hi Christoph,
> >> >>
> >> >>
> >> >> On Tuesday 24 April 2018 04:45 PM, Langer, Christoph wrote:
> >> >>
> >> >> Hi Vyom,
> >> >>
> >> >> I think, it is intentional to handle case where return "hostname" is to
> >> >> large to
> >> >> fit in  array.  if you see the man page(http://man7.org/linux/man-
> >> >> pages/man2/gethostname.2.html) it says that it i

RE: RFR(XS): 8202181: Correctly specify size of hostname buffer in Unix Inet*AddressImpl_getLocalHostName implementations

2018-04-27 Thread Langer, Christoph
Hi all,

thanks for looking into this. Here are a few comments

First of all, there are no real life issues I have seen with this. It is just 
something that occurred to me when working with the code. But, why not fix it, 
even it is a corner case that might never happen.

@Thomas: As for the zero termination of the hostname result after the call to 
gethostname: Yes, we should unconditionally terminate the result, which we do. 
Unfortunately this part of code cannot be moved outside the solaris #ifdef 
because the part in the #ifdef contains variable declarations. And you know - 
the C compatibility issue...

I looked again into the macro definitions for for HOST_NAME_MAX and NI_MAXHOST. 
HOST_NAME_MAX is mentioned in the gethostname docs ([1] and [2]). Glibc docs 
indicate it is 64 Byte or 255 Byte. So it looks like it is a quite small 
buffer, compared to NI_MAXHOST from netdb.h, which is the value that 
getnameinfo likes to work with, as per [3]. Posix genameinfo doc ([4]) does not 
mention NI_MAXHOST but Linux doc says it is 1025 which is what we'd define if 
it is not set.

Taking this input I have updated my webrev to round things up a little bit: 
http://cr.openjdk.java.net/~clanger/webrevs/8202181.1/ 

I moved the definition of NI_MAXHOST into net_util_md.h and updated the comment 
a little bit to make clearer why it is there. In Inet4AddressImpl.c and 
Inet6AddressImpl.c I also fixed the other places where getnameinfo is called to 
use sizeof(buffer) instead of NI_MAXHOST.

Best regards
Christoph

[1] http://man7.org/linux/man-pages/man2/gethostname.2.html
[2] http://pubs.opengroup.org/onlinepubs/009695399/functions/gethostname.html 
[3] http://man7.org/linux/man-pages/man3/getnameinfo.3.html 
[4] http://pubs.opengroup.org/onlinepubs/9699919799/functions/getnameinfo.html 


> -Original Message-
> From: vyom tewari [mailto:vyom.tew...@oracle.com]
> Sent: Freitag, 27. April 2018 08:38
> To: Thomas Stüfe 
> Cc: Langer, Christoph ; net-
> d...@openjdk.java.net
> Subject: Re: RFR(XS): 8202181: Correctly specify size of hostname buffer in
> Unix Inet*AddressImpl_getLocalHostName implementations
> 
> 
> 
> On Friday 27 April 2018 10:58 AM, Thomas Stüfe wrote:
> > On Fri, Apr 27, 2018 at 5:57 AM, vyom tewari 
> wrote:
> >> Hi Christoph,
> >>
> >>
> >> On Tuesday 24 April 2018 04:45 PM, Langer, Christoph wrote:
> >>
> >> Hi Vyom,
> >>
> >> I think, it is intentional to handle case where return "hostname" is to
> >> large to
> >> fit in  array.  if you see the man page(http://man7.org/linux/man-
> >> pages/man2/gethostname.2.html) it says that it is unspecified whether
> >> returned buffer includes a terminating null byte.
> >>
> >> current code will put null in case of large "hostname", What do you think ?
> >>
> >> yes, I had read the man page and saw this point of the spec. But exactly
> for
> >> this purpose there's this code:
> >>
> >> // make sure string is null-terminated
> >> hostname[NI_MAXHOST] = '\0';
> >>
> >> If we only hand 'NI_MAXHOST' as size value into gethostname, then the
> >> function might only write NI_MAXHOST - 1 characters of the hostname
> into the
> >> buffer.
> >>
> >> doc says it will copy len bytes into buffer and will not copy null 
> >> character
> >> into the buffer.
> >>
> >> 
> >>
> >> C library/kernel differences
> >> The GNU C library does not employ the gethostname() system call;
> >> instead, it implements gethostname() as a library function that 
> >> calls
> >> uname(2) and copies up to len bytes from the returned nodename
> field
> >> into name.  Having performed the copy, the function then checks if
> >> the length of the nodename was greater than or equal to len, and if
> >> it is, then the function returns -1 with errno set to ENAMETOOLONG;
> >> in this case, a terminating null byte is not included in the 
> >> returned
> >> name.
> >>
> ##
> ##
> >>
> > This is shared code, so we should refer to Posix, not linux specific man
> pages.
> >
> >
> http://pubs.opengroup.org/onlinepubs/9699919799/functions/gethostname
> .html
> >
> > 
> >
> > DESCRIPTION
> >
> > The gethostname() function shall return the standard host name for the
> > current machine. The namelen argument shall specify the size of the
> > array pointed to by the name arg

RE: RFR(XS): 8202181: Correctly specify size of hostname buffer in Unix Inet*AddressImpl_getLocalHostName implementations

2018-04-26 Thread Langer, Christoph
Ping, can some reviewer please have a look at this small fix?

Thanks
Christoph

From: Langer, Christoph
Sent: Dienstag, 24. April 2018 11:39
To: net-dev@openjdk.java.net
Subject: RFR(XS): 8202181: Correctly specify size of hostname buffer in Unix 
Inet*AddressImpl_getLocalHostName implementations

Hi,

please help reviewing a small change that I stumbled over when looking into the 
getLocalHostName implementation. I found that the length of the hostname buffer 
is not correctly passed to sub functions. The buffer size is specified as 
"NI_MAXHOST + 1", so this size should be handed down to gethostname() and 
getnameinfo() calls, not just NI_MAXHOST. I also moved the solaris #ifdefs into 
the else clause to spare a few lines of code.

Bug: https://bugs.openjdk.java.net/browse/JDK-8202181
Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8202181.0/

Thanks
Christoph



RE: RFR:8194298 Add support for per Socket configuration of TCP keepalive

2018-04-26 Thread Langer, Christoph
Ok, let's get some more opinions on that... 😊

> -Original Message-
> From: vyom tewari [mailto:vyom.tew...@oracle.com]
> Sent: Donnerstag, 26. April 2018 12:31
> To: Langer, Christoph ; Chris Hegarty
> ; OpenJDK Network Dev list  d...@openjdk.java.net>
> Subject: Re: RFR:8194298 Add support for per Socket configuration of TCP
> keepalive
> 
> 
> 
> On Thursday 26 April 2018 03:48 PM, Langer, Christoph wrote:
> > Hi Vyom,
> >
> > what about my suggestions for renaming?
> >
> > src/jdk.net/macosx/classes/jdk/net/UnixSocketOptions.java ->
> src/jdk.net/macosx/classes/jdk/net/MacOSXSocketOptions.java
> > src/jdk.net/macosx/native/libextnet/UnixSocketOptions.c ->
> src/jdk.net/macosx/native/libextnet/MacOSXSocketOptions.c
> till now we don't have file name like MacOSX* so i choose to leave
> as it is but if people think "MacOSXSocketOption" is more appropriate, i
> will change the filename name in my next webrev.
> 
> Thanks,
> Vyom
> >
> > This would be more consistent as we have:
> > src/jdk.net/linux/native/libextnet/LinuxSocketOptions.c
> > src/jdk.net/linux/classes/jdk/net/LinuxSocketOptions.java
> > ...
> > src/jdk.net/solaris/native/libextnet/SolarisSocketOptions.c
> > src/jdk.net/solaris/classes/jdk/net/SolarisSocketOptions.java
> >
> > Thanks
> > Christoph
> >
> >> -Original Message-
> >> From: net-dev [mailto:net-dev-boun...@openjdk.java.net] On Behalf Of
> >> vyom tewari
> >> Sent: Montag, 23. April 2018 14:06
> >> To: Chris Hegarty ; OpenJDK Network Dev list
> >> 
> >> Subject: Re: RFR:8194298 Add support for per Socket configuration of TCP
> >> keepalive
> >>
> >> Hi,
> >>
> >> Please find the latest
> >>
> webrev(http://cr.openjdk.java.net/~vtewari/8194298/webrev0.1/index.htm
> >> l).
> >> I incorporated  most of the review comments.
> >>
> >> Thanks,
> >>
> >> Vyom
> >>
> >>
> >> On Wednesday 18 April 2018 07:44 PM, Chris Hegarty wrote:
> >>> Vyom,
> >>>
> >>> On 13/04/18 10:50, vyom tewari wrote:
> >>>> Hi All,
> >>>>
> >>>> Please review the below code.
> >>>>
> >>>> BugId    : https://bugs.openjdk.java.net/browse/JDK-8194298
> >>>>
> >>>> webrev :
> >>>> http://cr.openjdk.java.net/~vtewari/8194298/webrev0.0/index.html
> >>> Here is some proposed wording for the JDK-specific extended socket
> >>> options specification.
> >>>
> >>> 1) Uses a consistent style across all three new options,
> >>>     and is in line with existing extended options.
> >>> 2) Renamed the options slightly, to improve readability
> >>>     ( they don't need to conform to the native option names )
> >>> 3) Reordered them so the build up is more logical
> >>> 4) Removed inheritance from server sockets
> >>> 5) Added standard verbiage about being "platform and
> >>>     system dependent
> >>> 6) Added typical values. I think this is useful, as it
> >>>     gives an idea to the developer, but maybe it could be
> >>>     misleading. Can be dropped if there are concerns.
> >>>
> >>> Can you please confirm that this is accurate, and also
> >>> will leave enough "wriggle" room when additional platform
> >>> support is added.
> >>>
> >>> ---
> >>>
> >>>      /**
> >>>   * Keep-Alive idle time.
> >>>   *
> >>>   *  When the {@link
> java.net.StandardSocketOptions#SO_KEEPALIVE
> >>>   * SO_KEEPALIVE} option is enabled, TCP probes a connection that
> >>> has been
> >>>   * idle for some amount of time. The default value for this idle
> >>> period is
> >>>   * system dependent, but is typically 2 hours. The {@code
> >>> TCP_KEEPIDLE}
> >>>   * option can be used to affect this value for a given socket.
> >>>   *
> >>>   *  The value of this socket option is an {@code Integer} that
> >>> is the
> >>>   * number of seconds of idle time before keep-alive initiates a
> >>> probe. The
> >>>   * socket option is specific to stream-oriented sockets using the
> >>> TCP/IP
> >>>   * protocol. The exact semantics of this socket option are socke

RE: RFR:8194298 Add support for per Socket configuration of TCP keepalive

2018-04-26 Thread Langer, Christoph
Hi Vyom,

what about my suggestions for renaming?

src/jdk.net/macosx/classes/jdk/net/UnixSocketOptions.java -> 
src/jdk.net/macosx/classes/jdk/net/MacOSXSocketOptions.java
src/jdk.net/macosx/native/libextnet/UnixSocketOptions.c -> 
src/jdk.net/macosx/native/libextnet/MacOSXSocketOptions.c

This would be more consistent as we have:
src/jdk.net/linux/native/libextnet/LinuxSocketOptions.c
src/jdk.net/linux/classes/jdk/net/LinuxSocketOptions.java
...
src/jdk.net/solaris/native/libextnet/SolarisSocketOptions.c
src/jdk.net/solaris/classes/jdk/net/SolarisSocketOptions.java

Thanks
Christoph

> -Original Message-
> From: net-dev [mailto:net-dev-boun...@openjdk.java.net] On Behalf Of
> vyom tewari
> Sent: Montag, 23. April 2018 14:06
> To: Chris Hegarty ; OpenJDK Network Dev list
> 
> Subject: Re: RFR:8194298 Add support for per Socket configuration of TCP
> keepalive
> 
> Hi,
> 
> Please find the latest
> webrev(http://cr.openjdk.java.net/~vtewari/8194298/webrev0.1/index.htm
> l).
> I incorporated  most of the review comments.
> 
> Thanks,
> 
> Vyom
> 
> 
> On Wednesday 18 April 2018 07:44 PM, Chris Hegarty wrote:
> > Vyom,
> >
> > On 13/04/18 10:50, vyom tewari wrote:
> >> Hi All,
> >>
> >> Please review the below code.
> >>
> >> BugId    : https://bugs.openjdk.java.net/browse/JDK-8194298
> >>
> >> webrev :
> >> http://cr.openjdk.java.net/~vtewari/8194298/webrev0.0/index.html
> >
> > Here is some proposed wording for the JDK-specific extended socket
> > options specification.
> >
> > 1) Uses a consistent style across all three new options,
> >    and is in line with existing extended options.
> > 2) Renamed the options slightly, to improve readability
> >    ( they don't need to conform to the native option names )
> > 3) Reordered them so the build up is more logical
> > 4) Removed inheritance from server sockets
> > 5) Added standard verbiage about being "platform and
> >    system dependent
> > 6) Added typical values. I think this is useful, as it
> >    gives an idea to the developer, but maybe it could be
> >    misleading. Can be dropped if there are concerns.
> >
> > Can you please confirm that this is accurate, and also
> > will leave enough "wriggle" room when additional platform
> > support is added.
> >
> > ---
> >
> >     /**
> >  * Keep-Alive idle time.
> >  *
> >  *  When the {@link java.net.StandardSocketOptions#SO_KEEPALIVE
> >  * SO_KEEPALIVE} option is enabled, TCP probes a connection that
> > has been
> >  * idle for some amount of time. The default value for this idle
> > period is
> >  * system dependent, but is typically 2 hours. The {@code
> > TCP_KEEPIDLE}
> >  * option can be used to affect this value for a given socket.
> >  *
> >  *  The value of this socket option is an {@code Integer} that
> > is the
> >  * number of seconds of idle time before keep-alive initiates a
> > probe. The
> >  * socket option is specific to stream-oriented sockets using the
> > TCP/IP
> >  * protocol. The exact semantics of this socket option are socket
> > type and
> >  * system dependent.
> >  *
> >  *  @since 11
> >  */
> >     public static final SocketOption TCP_KEEPIDLE
> >     = new ExtSocketOption("TCP_KEEPIDLE",
> > Integer.class);
> >
> >     /**
> >  * Keep-Alive retransmission interval time.
> >  *
> >  *  When the {@link java.net.StandardSocketOptions#SO_KEEPALIVE
> >  * SO_KEEPALIVE} option is enabled, TCP probes a connection that
> > has been
> >  * idle for some amount of time. If the remote system does not
> > respond to a
> >  * keep-alive probe, TCP retransmits the probe after some amount
> > of time.
> >  * The default value for this retransmition interval is system
> > dependent,
> >  * but is typically 75 seconds. The {@code TCP_KEEPINTERVAL}
> > option can be
> >  * used to affect this value for a given socket.
> >  *
> >  *  The value of this socket option is an {@code Integer} that
> > is the
> >  * number of seconds to wait before retransmitting a keep-alive
> > probe. The
> >  * socket option is specific to stream-oriented sockets using the
> > TCP/IP
> >  * protocol. The exact semantics of this socket option are socket
> > type and
> >  * system dependent.
> >  *
> >  * @since 11
> >  */
> >     public static final SocketOption TCP_KEEPINTERVAL
> >     = new ExtSocketOption("TCP_KEEPINTERVAL",
> > Integer.class);
> >
> >     /**
> >  * Keep-Alive retransmission maximum limit.
> >  *
> >  *  When the {@link java.net.StandardSocketOptions#SO_KEEPALIVE
> >  * SO_KEEPALIVE} option is enabled, TCP probes a connection that
> > has been
> >  * idle for some amount of time. If the remote system does not
> > respond to a
> >  * keep-alive probe, TCP retransmits the probe a certain number of
> > times
> >  * before a connection is considered to be broken. The default
> > value for
> >  * this keep-al

RE: RFR(XS): 8202181: Correctly specify size of hostname buffer in Unix Inet*AddressImpl_getLocalHostName implementations

2018-04-24 Thread Langer, Christoph
Hi Vyom,

> I think, it is intentional to handle case where return "hostname" is to large 
> to
> fit in  array.  if you see the man page(http://man7.org/linux/man-
> pages/man2/gethostname.2.html) it says that it is unspecified whether
> returned buffer includes a terminating null byte.
> 
> current code will put null in case of large "hostname", What do you think ?

yes, I had read the man page and saw this point of the spec. But exactly for 
this purpose there's this code:

// make sure string is null-terminated
hostname[NI_MAXHOST] = '\0';

If we only hand 'NI_MAXHOST' as size value into gethostname, then the function 
might only write NI_MAXHOST - 1 characters of the hostname into the buffer.

Best regards
Christoph



RFR(XS): 8202181: Correctly specify size of hostname buffer in Unix Inet*AddressImpl_getLocalHostName implementations

2018-04-24 Thread Langer, Christoph
Hi,

please help reviewing a small change that I stumbled over when looking into the 
getLocalHostName implementation. I found that the length of the hostname buffer 
is not correctly passed to sub functions. The buffer size is specified as 
"NI_MAXHOST + 1", so this size should be handed down to gethostname() and 
getnameinfo() calls, not just NI_MAXHOST. I also moved the solaris #ifdefs into 
the else clause to spare a few lines of code.

Bug: https://bugs.openjdk.java.net/browse/JDK-8202181
Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8202181.0/

Thanks
Christoph



RE: RFR 8202154 : Remove unused code in java.base/windows/native/libnet

2018-04-24 Thread Langer, Christoph
Hi Ivan,

looks good. I agree that the fields that Vyom mentioned should be final.

Best regards
Christoph

From: net-dev [mailto:net-dev-boun...@openjdk.java.net] On Behalf Of vyom tewari
Sent: Dienstag, 24. April 2018 08:29
To: net-dev@openjdk.java.net
Subject: Re: RFR 8202154 : Remove unused code in java.base/windows/native/libnet


Hi Ivan,

code looks good to me, thanks for doing this cleanup. One minor comment, in 
PortConfig.java you can make defaultUpper& defaultLower as final.I see that 
Microsoft changed dynamic port range recently do we need to put some comment in 
PortConfig.java ?

Thanks,

Vyom



On Tuesday 24 April 2018 09:47 AM, Ivan Gerasimov wrote:
Hello again!

A few other files contain obsolete code, so they can be combined together in 
one fix.

The webrev was updated in place:
http://cr.openjdk.java.net/~igerasim/8202154/00/webrev/

Here's the summary of additional changes:
- sun.net.PortConfig.getLower()/getUpper() always return the same range, so it 
can be defined with constants,
- NET_GetDefaultTOS() always returns zero, so it can be removed.

Would you please help review this?

With kind regards,
Ivan

On 4/23/18 2:29 PM, Ivan Gerasimov wrote:

Hello!

Some code in TwoStacksPlainDatagramSocketImpl.c is only relevant for earlier 
versions of Windows, which are no longer supported as of JDK 11.
This code can be safely removed.

Also removing an unused argument at 
DualStackPlainDatagramSocketImpl.socketCreate().

Would you please help review this cleanup?

BUGURL: https://bugs.openjdk.java.net/browse/JDK-8202154
WEBREV: http://cr.openjdk.java.net/~igerasim/8202154/00/webrev/

Thanks in advance!




[8u] RFR(XS): 8201369: Inet4AddressImpl_getLocalHostName reverse lookup on Solaris only

2018-04-23 Thread Langer, Christoph
Hi,

I created a JDK 8 backport proposal for JDK-8201369: 
http://cr.openjdk.java.net/~clanger/webrevs/8201369-jdk8.0/
This is the Bug: https://bugs.openjdk.java.net/browse/JDK-8201369
The issue was recently discussed and reviewed here: 
http://mail.openjdk.java.net/pipermail/net-dev/2018-April/011322.html

Apart from limiting the getaddrinfo/getnameinfo lookup in 
Java_java_net_Inet4AddressImpl_getLocalHostName to Solaris only, I took over 
the formatting cleanups to make both Ipv4 and IPv6 versions look similar and 
resemble the layout in the current jdk depot.

Please help reviewing this  for JDK 8.

Thanks & Best regards
Christoph



RE: RFR 8202091 : Rename DualStackPlainSocketImpl to PlainSocketImpl [win]

2018-04-22 Thread Langer, Christoph
Hi Ivan,

this looks good to me. Next nice step in cleaning up the socket implementation.

Best regards
Christoph

> -Original Message-
> From: net-dev [mailto:net-dev-boun...@openjdk.java.net] On Behalf Of
> Ivan Gerasimov
> Sent: Freitag, 20. April 2018 22:40
> To: net-dev@openjdk.java.net
> Subject: Re: RFR 8202091 : Rename DualStackPlainSocketImpl to
> PlainSocketImpl [win]
> 
> The correct links to the Bug and webrev are:
> 
> BUGURL: https://bugs.openjdk.java.net/browse/JDK-8202091
> WEBREV: http://cr.openjdk.java.net/~igerasim/8202091/00/webrev/
> 
> Sorry for confusion!
> 
> With kind regards,
> Ivan
> 
> On 4/20/18 1:32 PM, Ivan Gerasimov wrote:
> > Hello!
> >
> > After integrating the fix for JDK-8201510, there is only one
> > implementation of PlainSocketImpl left on Windows.
> >
> > It is proposed to just rename DualStackPlainSocketImpl to
> > PlainSocketImpl and avoid the indirection.
> >
> > Some minor cleanup was done with this change:
> > - dropping static PlainSocketImpl.isReusePortAvailable() -- it wasn't
> > called anyway and just hid the same named function in the super class,
> > - minor re-formatting.
> >
> > Would you please help review the fix?
> >
> > Thanks in advance!
> >
> 
> --
> With kind regards,
> Ivan Gerasimov



RE: RFR(XS): 8201369: Inet4AddressImpl_getLocalHostName reverse lookup on Solaris only

2018-04-18 Thread Langer, Christoph
Thank you. Pushed: http://hg.openjdk.java.net/jdk/jdk/rev/a838e3707f3a


> -Original Message-
> From: Chris Hegarty [mailto:chris.hega...@oracle.com]
> Sent: Mittwoch, 18. April 2018 16:20
> To: Langer, Christoph 
> Cc: Srividya Shamaiah ; OpenJDK Network Dev list
> 
> Subject: Re: RFR(XS): 8201369: Inet4AddressImpl_getLocalHostName reverse
> lookup on Solaris only
> 
> Christophe,
> 
> On 18/04/18 14:44, Langer, Christoph wrote:
> > Hi Chris,
> >
> > our testing didn’t show regressions. Are we good to push?
> 
> My testing was positive too.
> 
> I am happy to see this pushed to jdk/jdk.
> 
> -Chris.


RE: RFR(XS): 8201369: Inet4AddressImpl_getLocalHostName reverse lookup on Solaris only

2018-04-18 Thread Langer, Christoph
Hi Chris,

our testing didn’t show regressions. Are we good to push?

Best regards
Christoph

From: Chris Hegarty [mailto:chris.hega...@oracle.com]
Sent: Montag, 16. April 2018 16:39
To: Langer, Christoph 
Cc: Srividya Shamaiah ; OpenJDK Network Dev list 

Subject: Re: RFR(XS): 8201369: Inet4AddressImpl_getLocalHostName reverse lookup 
on Solaris only




On 16 Apr 2018, at 10:29, Langer, Christoph 
mailto:christoph.lan...@sap.com>> wrote:

Hi Srividya,

thanks for doing this work.

Change looks good from my side, except for a small indentation flaw in lines 91 
and 94 and the copyright year that needs to be adjusted. But I can fix this 
when I push it.

+1


Let’s wait for another review (Chris) before we can push it. I’ll also do some 
testing.

I’ll run some tests with this patch too.


I’ll also take care of the backport after the push to jdk11.

Yes. Let’s first push this to jdk/jdk, then follow up with a back port request 
as appropriate.

-Chris.


Best regards
Christoph

From: Srividya Shamaiah [mailto:ssham...@in.ibm.com]
Sent: Montag, 16. April 2018 10:44
To: Langer, Christoph 
mailto:christoph.lan...@sap.com>>
Cc: Chris Hegarty mailto:chris.hega...@oracle.com>>; 
OpenJDK Network Dev list 
mailto:net-dev@openjdk.java.net>>
Subject: RFR(XS): 8201369: Inet4AddressImpl_getLocalHostName reverse lookup on 
Solaris only

Hi Chris,
Please review the attached patch in
http://cr.openjdk.java.net/~mhorie/8201369/webrev/

Can you also backport this to JDK 8, we have customers waiting for this fix at 
JDK 8 level.

Thanks,
Srividya S

Srividya Shamaiah---11/04/2018 03:35:38 PM---Thanks Chris , As 
you suggested, I will provide the patch based on jdk 11. Thanks,

From: Srividya Shamaiah/India/IBM
To: "Langer, Christoph" 
mailto:christoph.lan...@sap.com>>
Cc: Chris Hegarty mailto:chris.hega...@oracle.com>>, 
OpenJDK Network Dev list 
mailto:net-dev@openjdk.java.net>>
Date: 11/04/2018 03:35 PM
Subject: RE: 8169865 : Changes not ported to IPv4



Thanks Chris , As you suggested, I will provide the patch based on jdk 11.

Thanks,
Srividya S


"Langer, Christoph" ---11/04/2018 02:51:51 PM---Hi Srividya, I 
would also welcome this fix.

From: "Langer, Christoph" 
mailto:christoph.lan...@sap.com>>
To: Srividya Shamaiah mailto:ssham...@in.ibm.com>>, Chris 
Hegarty mailto:chris.hega...@oracle.com>>
Cc: OpenJDK Network Dev list 
mailto:net-dev@openjdk.java.net>>
Date: 11/04/2018 02:51 PM
Subject: RE: 8169865 : Changes not ported to IPv4




Hi Srividya,

I would also welcome this fix.

Will you do the fix based on the jdk (11) depot? I think 
Java_java_net_Inet4AddressImpl_getLocalHostName should then be exactly the same 
as Java_java_net_Inet6AddressImpl_getLocalHostName. I can assist you with 
sponsoring/backporting to JDK8, if you like.

Best regards
Christoph



From: net-dev [mailto:net-dev-boun...@openjdk.java.net] On Behalf Of Srividya 
Shamaiah
Sent: Mittwoch, 11. April 2018 09:19
To: Chris Hegarty mailto:chris.hega...@oracle.com>>
Cc: OpenJDK Network Dev list 
mailto:net-dev@openjdk.java.net>>
Subject: Re: 8169865 : Changes not ported to IPv4
Thank you Chris for opening the JIRA bug, I will work on the fix and contribute 
it .

Thanks,
Srividya S

Chris Hegarty ---10/04/2018 08:51:05 PM---> On 10 Apr 2018, at 
12:34, Srividya Shamaiah mailto:ssham...@in.ibm.com>> 
wrote: >

From: Chris Hegarty mailto:chris.hega...@oracle.com>>
To: Srividya Shamaiah mailto:ssham...@in.ibm.com>>
Cc: OpenJDK Network Dev list 
mailto:net-dev@openjdk.java.net>>
Date: 10/04/2018 08:51 PM
Subject: Re: 8169865 : Changes not ported to IPv4






> On 10 Apr 2018, at 12:34, Srividya Shamaiah 
> mailto:ssham...@in.ibm.com>> wrote:
>
> Hi Chris,
>
> One of our customer reported a similar issue and the issue can be resolved 
> through the bug fix 8169865 which was include at 8u152 level. We were looking 
> this issue from AIX perspective as it did not do the reverse lookup with bug 
> fix 8169865 (as reverse lookup is limited to solaris after the bug fix).
>
> While implementing the fix, we want to make sure the fix works for all 
> scenario. As there is an inconsistency between IPv6 and IPv4 after 8169865 
> (as reverse lookup still exists for IPv4 on AIX and Linux), we are afraid 
> whether customer can hit the same issue if they use IPv4.
>
> Please confirm whether it makes sense to remove the reverse lookup of IPv4 
> for AIX and linux platforms so that IPv4 and IPv6 processing is consistent 
> for those platforms.

Yes, I believe it does.

I filed the follow JIRA issue to track this:
https://urldefense.proofpoint.com/v2/url?u=https-3A__bugs.openjdk.java.net_browse_JDK-2D8201369&d=DwIFAg&c=jf_iaSHvJObTbx-siA1ZOg&r=cY5OjfQF2gZ_G00XrJYGrxPgLDHmXjFqs49sDD9oJN0&m=-LhngTQSiYD1d12WSDvX2Jldxusyok9A7LqJ4ZEIzos&s=8fDzPwCaD2hwIOSWkfchiRBeDz3uSyzk81kDXZFarXo&e=

-Chris.








RE: RFR:8194298 Add support for per Socket configuration of TCP keepalive

2018-04-16 Thread Langer, Christoph
Hi Vyom,

I had a quick glance through your changes.

Apart from the suggestions you've already got (use snprintf instead of string 
concatenation, method ordering...), I think 
"src/jdk.net/macosx/classes/jdk/net/UnixSocketOptions.java" and " 
src/jdk.net/macosx/native/libextnet/UnixSocketOptions.c " should be renamed to 
MacOSXSocketOptions. to be consistent with the other platforms (Linux 
and Solaris at the moment).

Furthermore, I think most of the options can/should also be supported for AIX. 
I'm willing to contribute this. However, I should probably do this in a 
separate bug once you have completed your work on this one. I would also add 
TCP QUICKACK then.

Best regards
Christoph

> -Original Message-
> From: net-dev [mailto:net-dev-boun...@openjdk.java.net] On Behalf Of
> vyom tewari
> Sent: Freitag, 13. April 2018 11:51
> To: OpenJDK Network Dev list 
> Subject: RFR:8194298 Add support for per Socket configuration of TCP
> keepalive
> 
> Hi All,
> 
> Please review the below code.
> 
> BugId    : https://bugs.openjdk.java.net/browse/JDK-8194298
> 
> webrev :
> http://cr.openjdk.java.net/~vtewari/8194298/webrev0.0/index.html
> 
> Currently Java supports SO_KEEPALIVE, whose default value is 7200
> seconds which is too long for most of the applications. This code change
> will allow us to set the keepalive
> parameters(TCP_KEEPIDLE,TCP_KEEPCNT,TCP_KEEPINTVL)  which will
> configure
> the idle time on per socket basis.
> 
> I did code changes for Linux & Mac only, support for other platforms can
> be added in future if needed.
> 
> Thanks,
> 
> Vyom



RE: RFR(XS): 8201369: Inet4AddressImpl_getLocalHostName reverse lookup on Solaris only

2018-04-16 Thread Langer, Christoph
Hi Srividya,

thanks for doing this work.

Change looks good from my side, except for a small indentation flaw in lines 91 
and 94 and the copyright year that needs to be adjusted. But I can fix this 
when I push it.

Let's wait for another review (Chris) before we can push it. I'll also do some 
testing.

I'll also take care of the backport after the push to jdk11.

Best regards
Christoph

From: Srividya Shamaiah [mailto:ssham...@in.ibm.com]
Sent: Montag, 16. April 2018 10:44
To: Langer, Christoph 
Cc: Chris Hegarty ; OpenJDK Network Dev list 

Subject: RFR(XS): 8201369: Inet4AddressImpl_getLocalHostName reverse lookup on 
Solaris only


Hi Chris,
Please review the attached patch in
http://cr.openjdk.java.net/~mhorie/8201369/webrev/

Can you also backport this to JDK 8, we have customers waiting for this fix at 
JDK 8 level.

Thanks,
Srividya S

[Inactive hide details for Srividya Shamaiah---11/04/2018 03:35:38 PM---Thanks 
Chris , As you suggested, I will provide the patc]Srividya 
Shamaiah---11/04/2018 03:35:38 PM---Thanks Chris , As you suggested, I will 
provide the patch based on jdk 11. Thanks,

From: Srividya Shamaiah/India/IBM
To: "Langer, Christoph" 
mailto:christoph.lan...@sap.com>>
Cc: Chris Hegarty mailto:chris.hega...@oracle.com>>, 
OpenJDK Network Dev list 
mailto:net-dev@openjdk.java.net>>
Date: 11/04/2018 03:35 PM
Subject: RE: 8169865 : Changes not ported to IPv4




Thanks Chris , As you suggested, I will provide the patch based on jdk 11.

Thanks,
Srividya S


[Inactive hide details for "Langer, Christoph" ---11/04/2018 02:51:51 PM---Hi 
Srividya, I would also welcome this fix.]"Langer, Christoph" ---11/04/2018 
02:51:51 PM---Hi Srividya, I would also welcome this fix.

From: "Langer, Christoph" 
mailto:christoph.lan...@sap.com>>
To: Srividya Shamaiah mailto:ssham...@in.ibm.com>>, Chris 
Hegarty mailto:chris.hega...@oracle.com>>
Cc: OpenJDK Network Dev list 
mailto:net-dev@openjdk.java.net>>
Date: 11/04/2018 02:51 PM
Subject: RE: 8169865 : Changes not ported to IPv4




Hi Srividya,

I would also welcome this fix.

Will you do the fix based on the jdk (11) depot? I think 
Java_java_net_Inet4AddressImpl_getLocalHostName should then be exactly the same 
as Java_java_net_Inet6AddressImpl_getLocalHostName. I can assist you with 
sponsoring/backporting to JDK8, if you like.

Best regards
Christoph



From: net-dev [mailto:net-dev-boun...@openjdk.java.net] On Behalf Of Srividya 
Shamaiah
Sent: Mittwoch, 11. April 2018 09:19
To: Chris Hegarty mailto:chris.hega...@oracle.com>>
Cc: OpenJDK Network Dev list 
mailto:net-dev@openjdk.java.net>>
Subject: Re: 8169865 : Changes not ported to IPv4

Thank you Chris for opening the JIRA bug, I will work on the fix and contribute 
it .

Thanks,
Srividya S

[Inactive hide details for Chris Hegarty ---10/04/2018 08:51:05 PM---> On 10 
Apr 2018, at 12:34, Srividya Shamaiah  On 10 Apr 2018, at 12:34, Srividya Shamaiah 
mailto:ssham...@in.ibm.com>> wrote: >

From: Chris Hegarty mailto:chris.hega...@oracle.com>>
To: Srividya Shamaiah mailto:ssham...@in.ibm.com>>
Cc: OpenJDK Network Dev list 
mailto:net-dev@openjdk.java.net>>
Date: 10/04/2018 08:51 PM
Subject: Re: 8169865 : Changes not ported to IPv4







> On 10 Apr 2018, at 12:34, Srividya Shamaiah 
> mailto:ssham...@in.ibm.com>> wrote:
>
> Hi Chris,
>
> One of our customer reported a similar issue and the issue can be resolved 
> through the bug fix 8169865 which was include at 8u152 level. We were looking 
> this issue from AIX perspective as it did not do the reverse lookup with bug 
> fix 8169865 (as reverse lookup is limited to solaris after the bug fix).
>
> While implementing the fix, we want to make sure the fix works for all 
> scenario. As there is an inconsistency between IPv6 and IPv4 after 8169865 
> (as reverse lookup still exists for IPv4 on AIX and Linux), we are afraid 
> whether customer can hit the same issue if they use IPv4.
>
> Please confirm whether it makes sense to remove the reverse lookup of IPv4 
> for AIX and linux platforms so that IPv4 and IPv6 processing is consistent 
> for those platforms.

Yes, I believe it does.

I filed the follow JIRA issue to track this:
https://urldefense.proofpoint.com/v2/url?u=https-3A__bugs.openjdk.java.net_browse_JDK-2D8201369&d=DwIFAg&c=jf_iaSHvJObTbx-siA1ZOg&r=cY5OjfQF2gZ_G00XrJYGrxPgLDHmXjFqs49sDD9oJN0&m=-LhngTQSiYD1d12WSDvX2Jldxusyok9A7LqJ4ZEIzos&s=8fDzPwCaD2hwIOSWkfchiRBeDz3uSyzk81kDXZFarXo&e=

-Chris.






RE: JDK-8200719: Cannot connect to IPv6 host when exists any active network interface without IPv6 address

2018-04-16 Thread Langer, Christoph
Hi,

thanks, I've pushed it then: http://hg.openjdk.java.net/jdk/jdk/rev/bc1c7e41e285

Best regards
Christoph

> -Original Message-
> From: Chris Hegarty [mailto:chris.hega...@oracle.com]
> Sent: Freitag, 13. April 2018 16:07
> To: Langer, Christoph ; Joel Peláez Jorge
> ; net-dev@openjdk.java.net
> Subject: Re: JDK-8200719: Cannot connect to IPv6 host when exists any active
> network interface without IPv6 address
> 
> On 13/04/18 14:57, Langer, Christoph wrote:
> > Hi Chris, Joel,
> >
> > testing went fine and did not show regressions. Same for the Oracle tests?
> 
> Yes. nothing untoward observed.
> 
> -Chris.
> 
> >
> > Best regards
> > Christoph
> >
> >> -Original Message-
> >> From: Chris Hegarty [mailto:chris.hega...@oracle.com]
> >> Sent: Mittwoch, 11. April 2018 18:11
> >> To: Langer, Christoph ; Joel Peláez Jorge
> >> ; net-dev@openjdk.java.net
> >> Subject: Re: JDK-8200719: Cannot connect to IPv6 host when exists any
> active
> >> network interface without IPv6 address
> >>
> >> On 11/04/18 15:44, Langer, Christoph wrote:
> >>> Hi Joel,
> >>>
> >>> Sounds good to me then. I created a webrev and uploaded it:
> >> http://cr.openjdk.java.net/~clanger/webrevs/8200719.0/
> >>>
> >>> I will run it through the tests here at SAP and check the results.
> >>
> >> I will run it through the test system here in Oracle too.
> >>
> >>> @All: Could we please get another review?
> >>
> >> You have my Reviewed ( subject to successful testing ).
> >>
> >> -Chris.


RE: JDK-8200719: Cannot connect to IPv6 host when exists any active network interface without IPv6 address

2018-04-13 Thread Langer, Christoph
Hi Chris, Joel,

testing went fine and did not show regressions. Same for the Oracle tests?

Best regards
Christoph

> -Original Message-
> From: Chris Hegarty [mailto:chris.hega...@oracle.com]
> Sent: Mittwoch, 11. April 2018 18:11
> To: Langer, Christoph ; Joel Peláez Jorge
> ; net-dev@openjdk.java.net
> Subject: Re: JDK-8200719: Cannot connect to IPv6 host when exists any active
> network interface without IPv6 address
> 
> On 11/04/18 15:44, Langer, Christoph wrote:
> > Hi Joel,
> >
> > Sounds good to me then. I created a webrev and uploaded it:
> http://cr.openjdk.java.net/~clanger/webrevs/8200719.0/
> >
> > I will run it through the tests here at SAP and check the results.
> 
> I will run it through the test system here in Oracle too.
> 
> > @All: Could we please get another review?
> 
> You have my Reviewed ( subject to successful testing ).
> 
> -Chris.


RE: JDK-8200719: Cannot connect to IPv6 host when exists any active network interface without IPv6 address

2018-04-11 Thread Langer, Christoph
Hi Joel,

Sounds good to me then. I created a webrev and uploaded it: 
http://cr.openjdk.java.net/~clanger/webrevs/8200719.0/ 

I will run it through the tests here at SAP and check the results.

@All: Could we please get another review?

Thanks
Christoph

> -Original Message-
> From: Joel Peláez Jorge [mailto:joelpel...@gmail.com]
> Sent: Mittwoch, 11. April 2018 12:54
> To: Langer, Christoph ; net-
> d...@openjdk.java.net
> Subject: Re: JDK-8200719: Cannot connect to IPv6 host when exists any active
> network interface without IPv6 address
> 
> Hi Christoph,
> 
> OS X has a issue that needs "always" add a sin6_scope_id a multicast packet, I
> check that exists a old issue that logged it and add that piece of code:
> https://bugs.openjdk.java.net/browse/JDK-7144274
> 
> The DefaultInterface class in OS X is a plain Java class that list all 
> interfaces
> and discards loopback and ppp interfaces, and prefers multistack interfaces
> than ones with only IPv4. I think that this class must use the network
> interface order defined in System Configuration but I don't find a way to use
> it.
> 
> I already signed the OCA, then I would like to have your help.
> 
> El 11/04/2018 a las 04:42, Langer, Christoph escribió:
> > Hi Joel,
> >
> > your fix sounds reasonable. In fact, I'm not even sure if the sin6_scope_id
> should be set for multicast. Maybe it should be done only for link local
> addresses.
> >
> > Additionally, I guess the selection of the default interface (for IPv6) 
> > should
> be improved because I still can imagine a scenario where the first interface
> only has IPv4 but you want to do an IPv6 link local connect where you need a
> scope.
> >
> > Do you have signed an OCA as per [1], section "0. Become a Contributor"?
> Then I can help you with sponsoring this...
> >
> > Best regards
> > Christoph
> >
> > [1] http://openjdk.java.net/contribute/
> >
> >> -Original Message-
> >> From: net-dev [mailto:net-dev-boun...@openjdk.java.net] On Behalf Of
> >> Joel Peláez Jorge
> >> Sent: Dienstag, 10. April 2018 00:37
> >> To: net-dev@openjdk.java.net
> >> Subject: JDK-8200719: Cannot connect to IPv6 host when exists any active
> >> network interface without IPv6 address
> >>
> >> Hi,
> >>
> >> I am new in the OpenJDK Community and I contribute with a fix for the
> bug
> >> 8200719 related to networking in macOS system.
> >>
> >> I wrote a minimal patch that only set the scope id when the address is
> link-
> >> local or multicast. This change avoid send IPv6 packets on a wrong
> interface.
> >>
> >> Bug: https://bugs.openjdk.java.net/browse/JDK-8200719
> >>
> >> Patch:
> >>
> >> diff -r f088ec60bed5 src/java.base/unix/native/libnet/net_util_md.c
> >> --- a/src/java.base/unix/native/libnet/net_util_md.c Mon Apr 09 10:39:29
> >> 2018 -0700
> >> +++ b/src/java.base/unix/native/libnet/net_util_md.c Mon Apr 09
> 16:50:18
> >> 2018 -0500
> >> @@ -89,7 +89,8 @@
> >>}
> >>int defaultIndex;
> >>struct sockaddr_in6 *sin6 = (struct sockaddr_in6 *)him;
> >> -if (sin6->sin6_family == AF_INET6 && (sin6->sin6_scope_id == 0)) {
> >> +if (sin6->sin6_family == AF_INET6 && (sin6->sin6_scope_id == 0) &&
> >> +(IN6_IS_ADDR_LINKLOCAL(&sin6->sin6_addr) ||
> >> IN6_IS_ADDR_MULTICAST(&sin6->sin6_addr))) {
> >>defaultIndex = (*env)->GetStaticIntField(env, ni_class,
> >> ni_defaultIndexID);
> >>sin6->sin6_scope_id = defaultIndex;
> >>
> >>
> >> Thanks,
> >> Joel


RE: JDK-8200719: Cannot connect to IPv6 host when exists any active network interface without IPv6 address

2018-04-11 Thread Langer, Christoph
Hi Joel,

your fix sounds reasonable. In fact, I'm not even sure if the sin6_scope_id 
should be set for multicast. Maybe it should be done only for link local 
addresses.

Additionally, I guess the selection of the default interface (for IPv6) should 
be improved because I still can imagine a scenario where the first interface 
only has IPv4 but you want to do an IPv6 link local connect where you need a 
scope.

Do you have signed an OCA as per [1], section "0. Become a Contributor"? Then I 
can help you with sponsoring this...

Best regards
Christoph

[1] http://openjdk.java.net/contribute/

> -Original Message-
> From: net-dev [mailto:net-dev-boun...@openjdk.java.net] On Behalf Of
> Joel Peláez Jorge
> Sent: Dienstag, 10. April 2018 00:37
> To: net-dev@openjdk.java.net
> Subject: JDK-8200719: Cannot connect to IPv6 host when exists any active
> network interface without IPv6 address
> 
> Hi,
> 
> I am new in the OpenJDK Community and I contribute with a fix for the bug
> 8200719 related to networking in macOS system.
> 
> I wrote a minimal patch that only set the scope id when the address is link-
> local or multicast. This change avoid send IPv6 packets on a wrong interface.
> 
> Bug: https://bugs.openjdk.java.net/browse/JDK-8200719
> 
> Patch:
> 
> diff -r f088ec60bed5 src/java.base/unix/native/libnet/net_util_md.c
> --- a/src/java.base/unix/native/libnet/net_util_md.c Mon Apr 09 10:39:29
> 2018 -0700
> +++ b/src/java.base/unix/native/libnet/net_util_md.c Mon Apr 09 16:50:18
> 2018 -0500
> @@ -89,7 +89,8 @@
>   }
>   int defaultIndex;
>   struct sockaddr_in6 *sin6 = (struct sockaddr_in6 *)him;
> -if (sin6->sin6_family == AF_INET6 && (sin6->sin6_scope_id == 0)) {
> +if (sin6->sin6_family == AF_INET6 && (sin6->sin6_scope_id == 0) &&
> +(IN6_IS_ADDR_LINKLOCAL(&sin6->sin6_addr) ||
> IN6_IS_ADDR_MULTICAST(&sin6->sin6_addr))) {
>   defaultIndex = (*env)->GetStaticIntField(env, ni_class,
>ni_defaultIndexID);
>   sin6->sin6_scope_id = defaultIndex;
> 
> 
> Thanks,
> Joel


RE: 8169865 : Changes not ported to IPv4

2018-04-11 Thread Langer, Christoph
Hi Srividya,

I would also welcome this fix.

Will you do the fix based on the jdk (11) depot? I think 
Java_java_net_Inet4AddressImpl_getLocalHostName should then be exactly the same 
as Java_java_net_Inet6AddressImpl_getLocalHostName. I can assist you with 
sponsoring/backporting to JDK8, if you like.

Best regards
Christoph



From: net-dev [mailto:net-dev-boun...@openjdk.java.net] On Behalf Of Srividya 
Shamaiah
Sent: Mittwoch, 11. April 2018 09:19
To: Chris Hegarty 
Cc: OpenJDK Network Dev list 
Subject: Re: 8169865 : Changes not ported to IPv4


Thank you Chris for opening the JIRA bug, I will work on the fix and contribute 
it .

Thanks,
Srividya S

[Inactive hide details for Chris Hegarty ---10/04/2018 08:51:05 PM---> On 10 
Apr 2018, at 12:34, Srividya Shamaiah  On 10 Apr 2018, at 12:34, Srividya Shamaiah 
mailto:ssham...@in.ibm.com>> wrote: >

From: Chris Hegarty mailto:chris.hega...@oracle.com>>
To: Srividya Shamaiah mailto:ssham...@in.ibm.com>>
Cc: OpenJDK Network Dev list 
mailto:net-dev@openjdk.java.net>>
Date: 10/04/2018 08:51 PM
Subject: Re: 8169865 : Changes not ported to IPv4






> On 10 Apr 2018, at 12:34, Srividya Shamaiah 
> mailto:ssham...@in.ibm.com>> wrote:
>
> Hi Chris,
>
> One of our customer reported a similar issue and the issue can be resolved 
> through the bug fix 8169865 which was include at 8u152 level. We were looking 
> this issue from AIX perspective as it did not do the reverse lookup with bug 
> fix 8169865 (as reverse lookup is limited to solaris after the bug fix).
>
> While implementing the fix, we want to make sure the fix works for all 
> scenario. As there is an inconsistency between IPv6 and IPv4 after 8169865 
> (as reverse lookup still exists for IPv4 on AIX and Linux), we are afraid 
> whether customer can hit the same issue if they use IPv4.
>
> Please confirm whether it makes sense to remove the reverse lookup of IPv4 
> for AIX and linux platforms so that IPv4 and IPv6 processing is consistent 
> for those platforms.

Yes, I believe it does.

I filed the follow JIRA issue to track this:
 
https://urldefense.proofpoint.com/v2/url?u=https-3A__bugs.openjdk.java.net_browse_JDK-2D8201369&d=DwIFAg&c=jf_iaSHvJObTbx-siA1ZOg&r=cY5OjfQF2gZ_G00XrJYGrxPgLDHmXjFqs49sDD9oJN0&m=-LhngTQSiYD1d12WSDvX2Jldxusyok9A7LqJ4ZEIzos&s=8fDzPwCaD2hwIOSWkfchiRBeDz3uSyzk81kDXZFarXo&e=

-Chris.




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

2018-03-27 Thread Langer, Christoph
Hi Ivan,

I went through your changes and I really like this work.

Nothing to complain about - especially if there are no regressions with the 
additional test coverage.

Best regards
Christoph


> -Original Message-
> From: net-dev [mailto:net-dev-boun...@openjdk.java.net] On Behalf Of
> Ivan Gerasimov
> Sent: Dienstag, 27. März 2018 00:08
> To: Chris Hegarty ; net-dev@openjdk.java.net;
> Alan Bateman 
> Subject: Re: RFR [11] 8198358 : Align organization of
> DualStackPlainSocketImpl with TwoStacksPlainSocketImp [win]
> 
> Hello everyone!
> 
> A few modifications were done to the patch.
> Below is the list of updated parts of the patch with comments about what
> has changed, comparing to the previous version of the patch.
> 
> The patched JDK builds fine and all the tests pass well.
> 
> WEBREV-000:
> http://cr.openjdk.java.net/~igerasim/8198358/04/000/webrev/
> WEBREV-001:
> http://cr.openjdk.java.net/~igerasim/8198358/04/001/webrev/
> WEBREV-002:
> http://cr.openjdk.java.net/~igerasim/8198358/04/002/webrev/
> WEBREV-003:
> http://cr.openjdk.java.net/~igerasim/8198358/04/003/webrev/
> - Rebased to reflect recent changes to socketListen()
> 
> WEBREV-004:
> http://cr.openjdk.java.net/~igerasim/8198358/04/004/webrev/
> WEBREV-005:
> http://cr.openjdk.java.net/~igerasim/8198358/04/005/webrev/
> WEBREV-006:
> http://cr.openjdk.java.net/~igerasim/8198358/04/006/webrev/
> WEBREV-007:
> http://cr.openjdk.java.net/~igerasim/8198358/04/007/webrev/
> WEBREV-008:
> http://cr.openjdk.java.net/~igerasim/8198358/04/008/webrev/
> - The check the line 400 was changed to test if (newfd < 0) and then if
> (newfd == -2).
> I don't think the later is quite accurate (cannot find in the
> documentation that accept can return -2), but it is how it was done in
> TwoStacks originally, so no regression is expected.
> 
> WEBREV-009:
> http://cr.openjdk.java.net/~igerasim/8198358/04/009/webrev/
> - Added missing 'return -1;' at the line 188.
> 
> WEBREV-010:
> http://cr.openjdk.java.net/~igerasim/8198358/04/010/webrev/
> WEBREV-011:
> http://cr.openjdk.java.net/~igerasim/8198358/04/011/webrev/
> WEBREV-012:
> http://cr.openjdk.java.net/~igerasim/8198358/04/012/webrev/
> WEBREV-013:
> http://cr.openjdk.java.net/~igerasim/8198358/04/013/webrev/
> WEBREV-014:
> http://cr.openjdk.java.net/~igerasim/8198358/04/014/webrev/
> 
> With kind regards,
> Ivan



RE: 8199329: Remove code that attempts to read bytes after connection reset reported

2018-03-14 Thread Langer, Christoph
Hi Alan,

looks good to me, +1.

Best regards
Christoph

> -Original Message-
> From: net-dev [mailto:net-dev-boun...@openjdk.java.net] On Behalf Of
> Alan Bateman
> Sent: Mittwoch, 14. März 2018 15:31
> To: OpenJDK Network Dev list 
> Subject: 8199329: Remove code that attempts to read bytes after connection
> reset reported
> 
> 
> Classic networking has some curious code to deal with connection resets.
> I needed to dig into ancient history to find the issues and original
> motivations.
> 
> When a connection reset is reported (usually ECONNRESET) then further
> attempts to read from the socket will typically return -1 (EOF). Classic
> networking papers over this so that further attempts with the socket
> APIs to read bytes will continue to throw SocketException "connection
> reset". Furthermore, it allows for cases where the platform can read
> bytes from the socket buffer after ECONNRESET has been reported. None of
> the main stream platforms do this and it's hard to imagine anything
> depending on such unpredictable behavior. I'm running into this odd code
> as part of cleanup to the read/write code paths and reducing them down
> to a single blocking call.
> 
> I would like to remove the legacy/undocumented behavior that attempts to
> read beyond the connection reset. The code to consistently throw
> IOException once ECONNRESET is returned is retained as it's possible
> that code relies on this.
> 
> The proposed changes are here:
>     http://cr.openjdk.java.net/~alanb/8199329/webrev/
> 
> All existing tests pass with these changes.
> 
> -Alan


RE: httpserver does not close connections when RejectedExecutionException occurs

2018-03-07 Thread Langer, Christoph
Hi Yuji,

looks good.

Best regards
Christoph

> -Original Message-
> From: KUBOTA Yuji [mailto:kubota.y...@gmail.com]
> Sent: Donnerstag, 8. März 2018 03:57
> To: Daniel Fuchs 
> Cc: Langer, Christoph ; Yasumasa Suenaga
> ; OpenJDK Network Dev list  d...@openjdk.java.net>
> Subject: Re: httpserver does not close connections when
> RejectedExecutionException occurs
> 
> Hi Daniel,
> 
> Thank you for your sponsoring!
> 
> 2018-03-08 0:56 GMT+09:00 Daniel Fuchs :
> > Could you prepare a final changeset with a properly formatted
> > comment [1], use jcheck [2] to verify that your changeset conforms
> > to the OpenJDK rules (no trailing whitespaces, no tabs etc...),
> > and upload it somewhere to http://cr.openjdk.java.net/~ykubota/8169358
> ?
> 
> My final changeset is below. (no jcheck error from webrev.04)
> http://cr.openjdk.java.net/~ykubota/8169358/final_changeset_jckecked
> 
> If Christoph could not check webrev.04, please use the following changeset.
> http://cr.openjdk.java.net/~ykubota/8169358/final_changeset_jckecked-
> two-reviewers
> 
> Best regards
> Yuji


RE: RFR 8198302: VS2017 (C4477) java.base/windows/native/libnet/NetworkInterface_winXP.c incorrect printf format strings

2018-03-06 Thread Langer, Christoph
Looks good, Brian.

From: net-dev [mailto:net-dev-boun...@openjdk.java.net] On Behalf Of Brian 
Burkhalter
Sent: Dienstag, 6. März 2018 16:27
To: OpenJDK Network Dev list 
Subject: RFR 8198302: VS2017 (C4477) 
java.base/windows/native/libnet/NetworkInterface_winXP.c incorrect printf 
format strings

https://bugs.openjdk.java.net/browse/JDK-8198302

Changes are in the diff below.

Thanks,

Brian

--- a/src/java.base/windows/native/libnet/NetworkInterface_winXP.c
+++ b/src/java.base/windows/native/libnet/NetworkInterface_winXP.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2003, 2017, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2003, 2018, 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
@@ -39,14 +39,15 @@
 #ifdef DEBUG
 void printnif (netif *nif) {
 #ifdef _WIN64
-printf ("nif:0x%I64x name:%s\n", nif,nif->name);
+printf ("nif:0x%I64x name:%s\n", (UINT_PTR)nif, nif->name);
 #else
-printf ("nif:0x%x name:%s\n", nif,nif->name);
+printf ("nif:0x%x name:%s\n", nif, nif->name);
 #endif
 if (nif->dNameIsUnicode) {
-printf ("dName:%S index:%d ", nif->displayName,nif->index);
+printf ("dName:%S index:%d ", (unsigned short *)nif->displayName,
+nif->index);
 } else {
-printf ("dName:%s index:%d ", nif->displayName,nif->index);
+printf ("dName:%s index:%d ", nif->displayName, nif->index);
 }
 printf ("naddrs:%d\n", nif->naddrs);
 }



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

2018-03-06 Thread Langer, Christoph
Hi Ivan,

I went through the changes a bit and I would think it is really a good cleanup 
and will make the future merge of the implementations easier.

One thing I saw was that TwoStacksPlainSocketImpl.c has an #include  
in line 25. I think that can be removed!?

I had problems importing the patch(set) via mercurial queues - but maybe it is 
an issue of my local mercurial version.

Best regards
Christoph

> -Original Message-
> From: net-dev [mailto:net-dev-boun...@openjdk.java.net] On Behalf Of
> Ivan Gerasimov
> Sent: Freitag, 2. März 2018 05:43
> To: Chris Hegarty ; net-dev@openjdk.java.net
> Subject: RFR [11] 8198358 : Align organization of DualStackPlainSocketImpl
> with TwoStacksPlainSocketImp [win]
> 
> Hello!
> 
> I'd like to do the next step toward removing the TwoStacks socket
> implementation on Windows.
> 
> It would be aligning the two implementations (DualStack and TwoStacks),
> so they can be easier merged together during the next step.
> 
> There are three PlainSocketImpl implementations in JDK:
> java.base/windows/classes/java/net/DualStackPlainSocketImpl.java
> java.base/windows/classes/java/net/TwoStacksPlainSocketImpl.java
> java.base/unix/classes/java/net/PlainSocketImpl.java
> 
> While two later have very similar organization (in particular, set of
> native methods), the former is organized slightly differently.
> In order to merge the two Windows implementation together, they first
> need to be organized in a similar way.
> For consistency, DualStack implementation will be reorganized to be
> aligned with TwoStacks and unix/PlainSocketImpl.
> 
> Bug: https://bugs.openjdk.java.net/browse/JDK-8198358
> Webrev: http://cr.openjdk.java.net/~igerasim/8198358/00/webrev/
> 
> The change looks somewhat messy, but in fact it was a series of
> incremental changes, which I still keep in the mercurial 'mq'.
> 
> (I wish the webrev could be made incremental based on the mq patches, to
> make it easier to review.)
> 
> The patched JDK builds fine and all the regression tests pass Okay.
> 
> 
> Thanks in advance!
> --
> 
> With kind regards,
> Ivan Gerasimov



RE: httpserver does not close connections when RejectedExecutionException occurs

2018-03-06 Thread Langer, Christoph
Hi Yuji,

I had a look and to me it seems safe to do like webrev.03 suggests. That is, 
remove the throws clause. As Daniel pointed out, it seems that all callers of 
the "handle" method would do merely the same. The only thing I'm not sure about 
is the CancelledKeyException caught in line 413. In that case no exception 
would be logged currently. With your change, it would now inside "handle". 
Maybe you want to handle a CancelledKeyException explicitly in "handle" now and 
suppress the log?

What tests did you run?

BTW: you could remove line 396: ' boolean closed;' while you are touching this 
file. It is not needed.

Best regards
Christoph

> -Original Message-
> From: net-dev [mailto:net-dev-boun...@openjdk.java.net] On Behalf Of
> KUBOTA Yuji
> Sent: Donnerstag, 1. März 2018 12:41
> To: OpenJDK Network Dev list 
> Cc: Yasumasa Suenaga 
> Subject: Re: httpserver does not close connections when
> RejectedExecutionException occurs
> 
> Hi all,
> 
> Could you please review my patch(s)?
> 
> Thanks,
> Yuji
> 
> 2018-02-20 14:21 GMT+09:00 KUBOTA Yuji :
> > Hi Daniel,
> >
> > Thank you for your comment.
> >
> > Dispatcher is package-private class and handle method is called at
> > only this file in the package (sun.net.httpserver), and all call sites
> > look like to handle exception suitably. So we can remove `throws
> > IOException` clause without modifying the call site logic as like
> > webrev.03.
> > http://cr.openjdk.java.net/~ykubota/8169358/webrev.03
> >
> > However, I could not find whether can I change a signature of public
> > method without specified steps. If we need to write CSR to remove
> > `throws IOException` clause, we should remove the clause by other
> > issue. And I want to commit webrev.02  at this issue.
> > http://cr.openjdk.java.net/~ykubota/8169358/webrev.02
> >
> > I'd like to get your thoughts on that.
> >
> > P.S. I'm an author, not a committer, so I cannot access Mach5.
> >
> > Thanks,
> > Yuji
> >
> > 2018-02-17 1:11 GMT+09:00 Daniel Fuchs :
> >> Hi,
> >>
> >> On the surface I'd say that this change looks reasonable.
> >> However, handle is declared to throw IOException, and
> >> with this change it is guaranteed to never throw even
> >> RuntimeException.
> >>
> >> It seems that the code that calls handle() - at least in
> >> this file, has some logic to handle IOException - or
> >> other exception possibly thrown by Dispatcher::handle,
> >> which as far as I can see is not doing much more than what
> >> closeConnection does. So it looks as if calling
> >> closeConnection in handle() instead of throwing could be OK.
> >>
> >> However it would be good to at least try to figure out
> >> whether there are any other call sites for Dispatcher::handle,
> >> validate that no longer throwing will not modify the call site
> >> logic, and possibly investigate if the 'throws IOException' clause
> >> can be removed from Dispatcher::handle (that is: look
> >> whether Dispatcher is exposed and/or can be subclassed and
> >> if there are any existing subclasses).
> >>
> >> best regards,
> >>
> >> -- daniel
> >>
> >>
> >> On 16/02/2018 15:29, KUBOTA Yuji wrote:
> >>>
> >>> Hi Chris and Yasumasa,
> >>>
> >>> Sorry to have remained this issue for a long time. I come back.
> >>>
> >>> I updated my patch for the following three reasons.
> >>> http://cr.openjdk.java.net/~ykubota/8169358/webrev.02/
> >>>
> >>> * applies to the latest jdk/jdk instead of jdk9/dev
> >>> * adds test by modifying reproducer as like tests on
> >>> test/jdk/com/sun/net/httpserver
> >>> * prevents potential connection leak as Yasumasa said. For example, a
> >>> user sets own implementation of the thread pool to HttpServer and
> then
> >>> throws unexpected exceptions and errors. I think that we should save
> >>> existing exception handler to keep compatibility and minimize the risk
> >>> of connection leak by catching Throwable.
> >>>
> >>> I've tested test/jdk/com/sun/net/httpserver and passed.
> >>> I'm not a committer, so I can not access March 5.
> >>>
> >>> Could you review and sponsor it?
> >>>
> >>> Thanks,
> >>> Yuji
> >>>
> >>> 2016-11-11 12:11 GMT+09:00 KUBOTA Yuji :
> 
>  Hi Chris and Yasumasa,
> 
>  Thank you for your comments!
> 
>  I had faced connection leak on production by this handler. It seems
>  like a corner-case but it's a real risk to me.
>  I had focused REE on this issue, but it is a subclass of
>  RuntimeException, so I think that we should investigate
>  RuntimeException, too.
> 
>  If Chris agrees with the covering RuntimeException by JDK-8169358, I
>  will investigate it and update my patch.
>  If we should investigate on another issue, I want to add a test case
>  to check fixing about REE like existing jtreg, and I will create a new
>  issue after an investigation about RuntimeException.
> 
>  Any thoughts?
> 
>  Thank you!
>  Yuji
> 
>  2016-11-11 0:56 GMT+09:00 Chris Hegarty :
> >
> 

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

2018-03-06 Thread Langer, Christoph
Hi Pallavi,

this change looks good to me. The only thing left to mention is that you will 
need to update the copyright years - I guess you would have done it on submit 
anyway.

Best regards
Christoph

From: net-dev [mailto:net-dev-boun...@openjdk.java.net] On Behalf Of Pallavi 
Sonal
Sent: Freitag, 2. März 2018 02:55
To: net-dev@openjdk.java.net
Subject: RFR: JDK-8144300 : Java does not honor http.nonProxyHosts value having 
wildcard * both at end and start

Hi,
Please review the following change to fix JDK-8144300.
Bug : https://bugs.openjdk.java.net/browse/JDK-8144300
Webrev : http://cr.openjdk.java.net/~rpatil/8144300/webrev.00/
Whenever there is a wildcard(*) at either the beginning or the end of a 
hostname specified in the list of http.nonProxyHosts , the code works as 
expected and DIRECT connection is used for them. But in scenarios, where there 
is a wildcard both at the beginning and end of the hostname, proxy is used 
instead of DIRECT connection.

All the tier1 and tier2 Mach 5 tests have passed.

Thanks,
Pallavi Sonal


RE: JDK-8080990: libdt_socket/socket_md.c(202) : warning C4996: 'gethostbyname': Use getaddrinfo() or GetAddrInfoW()

2018-02-05 Thread Langer, Christoph
Hi Gary,

ok, I would agree that a change like globally moving from dbgSysInetAddr to 
something like dbgsysPToN should at least be handled by a different change.

Best regards
Christoph

From: Gary Adams [mailto:gary.ad...@oracle.com]
Sent: Montag, 5. Februar 2018 20:15
To: gary.ad...@oracle.com
Cc: Langer, Christoph ; OpenJDK Serviceability 
; OpenJDK Build 
; OpenJDK Networking 
Subject: Re: JDK-8080990: libdt_socket/socket_md.c(202) : warning C4996: 
'gethostbyname': Use getaddrinfo() or GetAddrInfoW()

One more to fix to cover the remaining test failures I was seeing.

Previously, using inet_addr, there was a single -1 return that needed to be 
checked.
Now that we're using inet_pton, there is a -1 and a 0 error return to be 
considered.

My preference would be to leave the dbgSysInetAddr name unchanged and have the 
low level
check for inet_pton errors to simply return the -1 that was previously checked.

This specific issue is about the deprecation of library functions on windows. 
I'd recommend not
upgrading the other platforms at this time until a complete update is done for 
ipv6 support.

I'll post a new webrev when I've retested using the updated inet_pton error 
checking.

...


Hi Gary,





> Here's a revised webrev

>

>
> http://cr.openjdk.java.net/~gadams/8080990/webrev.01/index.html<http://cr.openjdk.java.net/%7Egadams/8080990/webrev.01/index.html>

>

> Still testing ...

>

> Using shutdown() fixed problems reported by the

> java/nio/channelSocketChannel tests.



The fix looks good. I would think we should rename dbgsysInetAddr to dbgsysPToN 
and use inet_pton also for the Unix/Linux platforms. It would be the better 
choice there as well, though we still only support IPv4 in libdt_socket.



>

> I also noticed prior use of getaddrinfo for "localhost" was not calling

> freeaddrinfo.



Thanks for spotting/fixing that.



Best regards

Christoph



RE: JDK-8080990: libdt_socket/socket_md.c(202) : warning C4996: 'gethostbyname': Use getaddrinfo() or GetAddrInfoW()

2018-02-02 Thread Langer, Christoph
Hi Gary,


> Here's a revised webrev
>
>http://cr.openjdk.java.net/~gadams/8080990/webrev.01/index.html
>
> Still testing ...
>
> Using shutdown() fixed problems reported by the 
> java/nio/channelSocketChannel tests.

The fix looks good. I would think we should rename dbgsysInetAddr to dbgsysPToN 
and use inet_pton also for the Unix/Linux platforms. It would be the better 
choice there as well, though we still only support IPv4 in libdt_socket.

>
> I also noticed prior use of getaddrinfo for "localhost" was not calling 
> freeaddrinfo.

Thanks for spotting/fixing that.

Best regards
Christoph


RE: JDK-8080990: libdt_socket/socket_md.c(202) : warning C4996: 'gethostbyname': Use getaddrinfo() or GetAddrInfoW()

2018-02-01 Thread Langer, Christoph
But WSASendDisconnect isn't deprecated, right? So you wanted to get rid of it? 
I still don't see the reason...

-Original Message-
From: gary.ad...@oracle.com [mailto:gary.ad...@oracle.com] 
Sent: Donnerstag, 1. Februar 2018 12:17
To: Langer, Christoph ; OpenJDK Serviceability 
; OpenJDK Build 
; OpenJDK Networking 
Subject: Re: JDK-8080990: libdt_socket/socket_md.c(202) : warning C4996: 
'gethostbyname': Use getaddrinfo() or GetAddrInfoW()

First pass over the code I disabled the compilation flag and then
did quick substitution for the easier functions. I commented out the
WSASendDisconnect calls so I could see what tests would fail if
the function was just removed. I have a replacement now that uses
"shutdown(fd,SD_SEND)", but I still have a few more test failures to
investigate.

I updated the error message text for "getaddrinfo".

I'll post a revised webrev after the 4 jshell errors are corrected.


On 2/1/18 3:11 AM, Langer, Christoph wrote:
> Hi Gary,
>
> I was having a look at your changes.
>
> I'm wondering what the reason is behind uncommenting WSASendDisconnect in 
> Java_sun_nio_ch_SocketDispatcher_preClose0 of file SocketDispatcher.c? And in 
> dbgsysSocketClose?
>
> In socketTransport.c, line:
>  331 setLastError(0, "gethostbyname: unknown host");
> you should change the description text of the error to getaddrinfo instead of 
> gethostbyname.
>
> Best regards
> Christoph
>
>
>
> -Original Message-
> From: build-dev [mailto:build-dev-boun...@openjdk.java.net] On Behalf Of Gary 
> Adams
> Sent: Donnerstag, 25. Januar 2018 19:47
> To: OpenJDK Serviceability ; OpenJDK 
> Build ; OpenJDK Networking 
> 
> Subject: RFR: JDK-8080990: libdt_socket/socket_md.c(202) : warning C4996: 
> 'gethostbyname': Use getaddrinfo() or GetAddrInfoW()
>
> Here's a first pass attempt to remove the -D_WINSOCK_DEPRECATED_NO_WARNINGS
> build flag and update the winsock deprecated functions.
>
> Issue: https://bugs.openjdk.java.net/browse/JDK-8080990
> Webrev: http://cr.openjdk.java.net/~gadams/8080990/
>
> These are the initial deprecated functions updated:
>  gethostbyname -> getaddrinfo
>  inet_addr -> inet_pton
>  inet_ntoa -> inet_ntop
>
> I'm not sure how to replace the existing WSASendDisconnect calls.
> It's not clear that it actually provides a graceful disconnect.
>
>



RE: JDK-8080990: libdt_socket/socket_md.c(202) : warning C4996: 'gethostbyname': Use getaddrinfo() or GetAddrInfoW()

2018-02-01 Thread Langer, Christoph
Hi Gary,

I was having a look at your changes.

I'm wondering what the reason is behind uncommenting WSASendDisconnect in 
Java_sun_nio_ch_SocketDispatcher_preClose0 of file SocketDispatcher.c? And in 
dbgsysSocketClose?

In socketTransport.c, line:
331 setLastError(0, "gethostbyname: unknown host");
you should change the description text of the error to getaddrinfo instead of 
gethostbyname.

Best regards
Christoph



-Original Message-
From: build-dev [mailto:build-dev-boun...@openjdk.java.net] On Behalf Of Gary 
Adams
Sent: Donnerstag, 25. Januar 2018 19:47
To: OpenJDK Serviceability ; OpenJDK Build 
; OpenJDK Networking 
Subject: RFR: JDK-8080990: libdt_socket/socket_md.c(202) : warning C4996: 
'gethostbyname': Use getaddrinfo() or GetAddrInfoW()

Here's a first pass attempt to remove the -D_WINSOCK_DEPRECATED_NO_WARNINGS
build flag and update the winsock deprecated functions.

   Issue: https://bugs.openjdk.java.net/browse/JDK-8080990
   Webrev: http://cr.openjdk.java.net/~gadams/8080990/

These are the initial deprecated functions updated:
gethostbyname -> getaddrinfo
inet_addr -> inet_pton
inet_ntoa -> inet_ntop

I'm not sure how to replace the existing WSASendDisconnect calls.
It's not clear that it actually provides a graceful disconnect.




RE: 8193034: Optimize URL.toExternalForm

2017-12-04 Thread Langer, Christoph
Hi Martin,

I think the new code is more compact and readable which makes it nice. I 
reviewed it to check that the result is still the same as before. However, I 
can’t assess if it is acceptable from a performance point of view and would 
defer this assessment to some other reviewer. But apart from performance 
constraints the change seems good.

Best regards
Christoph

From: net-dev [mailto:net-dev-boun...@openjdk.java.net] On Behalf Of Martin 
Buchholz
Sent: Dienstag, 5. Dezember 2017 05:01
To: net-dev 
Subject: RFR: 8193034: Optimize URL.toExternalForm

http://cr.openjdk.java.net/~martin/webrevs/jdk/URL.toExternalForm/


RE: RFR(S): 8155590: Dubious collection management in sun.net.www.http.KeepAliveCache

2017-10-19 Thread Langer, Christoph
Hi Roger,

ah, I see. I pushed with removing the comment then: 
http://hg.openjdk.java.net/jdk10/master/rev/74e1913a98c0

Thanks
Christoph

From: Roger Riggs [mailto:roger.ri...@oracle.com]
Sent: Mittwoch, 18. Oktober 2017 20:37
To: Langer, Christoph ; net-dev@openjdk.java.net
Subject: Re: RFR(S): 8155590: Dubious collection management in 
sun.net.www.http.KeepAliveCache

Hi Christoph,


Right, my mistake, I meant CME.  My point was that ArrayDeque does not throw 
CME from remove().
It is not multi-thread safe and does not check for CME.

That remove might have been coded using the iterator:

synchronized boolean remove(HttpClient h) {

for (Iterator it = this.iterator();

 it.hasNext();) {

KeepAliveEntry curr = it.next();

if (curr.hc == h) {

it.remove();

return true;

}

}

return false;

}

your choice

Thanks, Roger




On 10/18/17 9:47 AM, Langer, Christoph wrote:
Hi Roger,

maybe we have a little disconnect here in understanding. I thought you mean 
ClassCastException with CCE but I don't see this mentioned anywhere. My comment 
talks about ConcurrentModificationException (CME). I mentioned that because, 
when the Collection is modified while iterating (which I do by calling 
super.remove()) then the next call to the Iterator would throw a CME. But we 
don't go back to the iterator as we return after removing.

Nevertheless, I can still remove the comment or change it... let me know.

Thanks
Christoph


From: Roger Riggs [mailto:roger.ri...@oracle.com]
Sent: Mittwoch, 18. Oktober 2017 17:28
To: Langer, Christoph 
<mailto:christoph.lan...@sap.com>; 
net-dev@openjdk.java.net<mailto:net-dev@openjdk.java.net>
Subject: Re: RFR(S): 8155590: Dubious collection management in 
sun.net.www.http.KeepAliveCache

Hi Christoph,

Looks ok.

The comment in remove() about CCE can be removed.  ArrayDeque is not thread safe
and doesn't check for CCE (and the method is synchronized).

Thanks, Roger


On 10/18/17 6:05 AM, Langer, Christoph wrote:
Hi Roger,

thanks for looking. So you motivated me to do some more cleanup. :)
Here is the new webrev: 
http://cr.openjdk.java.net/~clanger/webrevs/8155590.1/<http://cr.openjdk.java.net/%7Eclanger/webrevs/8155590.1/>
I replaced the Stack with an ArrayDeque and did some more rather cosmetical 
changes to make KeepAliveCache more appealing. I verified the change by running 
the jtreg tests jdk/sun/net/www/http/* and it all passes.

As for the CCE: I don't think this should be checked as the Stack/ArrayDeque is 
typed to KeepAliveEntry and no code appears to be in place which could trick 
this.

Best regards
Christoph

From: net-dev [mailto:net-dev-boun...@openjdk.java.net] On Behalf Of Roger Riggs
Sent: Dienstag, 17. Oktober 2017 16:53
To: net-dev@openjdk.java.net<mailto:net-dev@openjdk.java.net>
Subject: Re: RFR(S): 8155590: Dubious collection management in 
sun.net.www.http.KeepAliveCache

Hi,

Keep the synchronized, it will be low overhead since the Vector operations
are synchronized and in the same thread.

I think a CCE could occur during the iteration to find the entry when 
Vector.Itr.next() checks.

(It you want to more radical fix, replace the Vector with something more 
current.
It would be one less Vector).

Roger



On 10/16/17 2:33 AM, Langer, Christoph wrote:
Hi Vyom,

thanks for your feedback.

I'm not so sure about dropping "synchronized". In the new remove method of 
ClientVector we are iterating ourself. If this is not done under 
synchronization, there is risk to run into a ConcurrentModificationException. 
But under the assumption that all access to ClientVector comes from 
synchronized methods of KeepAliveCache, one could argue to drop all 
synchronized modifiers for ClientVector, though.

Let's wait for other opinions :)

Best regards
Christoph


From: net-dev [mailto:net-dev-boun...@openjdk.java.net] On Behalf Of vyom tewari
Sent: Montag, 16. Oktober 2017 10:27
To: net-dev@openjdk.java.net<mailto:net-dev@openjdk.java.net>
Subject: Re: RFR(S): 8155590: Dubious collection management in 
sun.net.www.http.KeepAliveCache


Hi Christoph,

Thanks for doing this, i think you don't need to synchronize the 
"remove(HttpClient h)".  This remove is get called from synchronize "remove 
(HttpClient h, Object obj)" and the underline data structure is which is 
java.util.Vector(ClientVector extends java.util.Stack) is also thread safe.

What do you think ?

Thanks,

Vyom

On Monday 16 October 2017 12:52 PM, Langer, Christoph wrote:
Hi,

Here is a proposal for a fix for bug 8155590. I already made this fix a while 
ago in our JDK clone and I'd like to contribute this.

Bug: https://bugs.openjdk.java.net/browse/JDK-8155590
Webrev: 
http://cr.openjdk.java.net/~clanger/webrevs/8155590.0/<http://cr.openjdk.java.net/%7Eclanger/webrevs/8155590.0/>

Please review.

Thanks
Christoph







RE: RFR 8145635 : Add TCP_QUICKACK socket option

2017-10-18 Thread Langer, Christoph
Hi Vyom,

looks good to me.

Best regards
Christoph

> -Original Message-
> From: net-dev [mailto:net-dev-boun...@openjdk.java.net] On Behalf Of
> vyom tewari
> Sent: Donnerstag, 19. Oktober 2017 04:56
> To: net-dev@openjdk.java.net
> Subject: Re: RFR 8145635 : Add TCP_QUICKACK socket option
> 
> Hi All,
> 
> please find the latest
> webrev(http://cr.openjdk.java.net/~vtewari/8145635/webrev0.7/index.htm
> l).
> 
> Thanks,
> 
> Vyom
> 
> 
> On Wednesday 18 October 2017 08:21 PM, Langer, Christoph wrote:
> > Hi Vyom,
> >
> > I just looked at your latest webrev which in general looks fine to me.  Some
> minor remarks:
> >
> > make/lib/Lib-jdk.net.gmk:
> > - copyright year needs to be updated
> >
> > src/java.base/unix/classes/java/net/PlainDatagramSocketImpl.java:
> > - private void addExtSocketOptions(...) looks a bit awful concerning its
> formatting. I suggest something like this:
> >
> > private void addExtSocketOptions(Set> extOptions,
> >   Set> options) {
> >  // TCP_QUICKACK is applicable for TCP Sockets only.
> >  extOptions.stream()
> >  .filter((option) -> !option.name().equals("TCP_QUICKACK"))
> >  .forEach((option) -> options.add(option));
> > }
> >
> > src/jdk.net/share/classes/jdk/net/ExtendedSocketOptions.java:
> > - copyright year needs to be updated
> > - the equals sign should be placed on line 98 here:
> >
> > 98 public static final SocketOption TCP_QUICKACK
> > 99 = new ExtSocketOption("TCP_QUICKACK",
> Boolean.class);
> >
> > Other than that you should consider it reviewed from my end. No need for
> further webrev...
> >
> > Best regards
> > Christoph
> >
> >> -Original Message-
> >> From: net-dev [mailto:net-dev-boun...@openjdk.java.net] On Behalf Of
> >> vyom tewari
> >> Sent: Dienstag, 17. Oktober 2017 10:37
> >> To: net-dev@openjdk.java.net
> >> Subject: Re: RFR 8145635 : Add TCP_QUICKACK socket option
> >>
> >> Hi Roger,
> >>
> >> Thanks for the review , please find the updated
> >>
> webrev(http://cr.openjdk.java.net/~vtewari/8145635/webrev0.6/index.htm
> >> l).
> >>
> >> Thanks,
> >>
> >> Vyom
> >>
> >>
> >> On Tuesday 17 October 2017 06:35 AM, Roger Riggs wrote:
> >>> Hi Vyom,
> >>>
> >>> A few suggestions:
> >>>
> >>> PlainDatagramSocketImpl.java:
> >>>   - line 95/96:  I think you can use just forEach, the order version is
> >>> not necessary.
> >>>      The code will be a bit more readable if the .filter and .forEach
> >>> are on a new line and don't wrap.
> >>>      You can also remove the extra "(" and ")
> >>>
> >>>   - line 87-94: these are confusing and imply some implicit resetting
> >>> of the option.
> >>>   - use @since 10
> >>> - 209/268: the native setQuickAck method should use boolean as its
> >>> argument to enable/disable
> >>>    Since enable is a boolean; it does not need "== true'
> >>>
> >>> LinuxSocketOptions.java/c:
> >>>    - 52: setQuickAck0 should use boolean for the 2nd argument; (The
> >>> native code already does)
> >>>
> >>> Thanks, Roger
> >>>
> >>>
> >>> On 10/15/17 11:58 PM, vyom tewari wrote:
> >>>> Hi Chris,
> >>>>
> >>>> Thanks for review. Please find the latest
> >>>>
> >>
> webrev(http://cr.openjdk.java.net/~vtewari/8145635/webrev0.5/index.htm
> >> l).
> >>>> Thanks,
> >>>>
> >>>> Vyom
> >>>>



RE: RFR(S): 8155590: Dubious collection management in sun.net.www.http.KeepAliveCache

2017-10-18 Thread Langer, Christoph
Hi Roger,

maybe we have a little disconnect here in understanding. I thought you mean 
ClassCastException with CCE but I don't see this mentioned anywhere. My comment 
talks about ConcurrentModificationException (CME). I mentioned that because, 
when the Collection is modified while iterating (which I do by calling 
super.remove()) then the next call to the Iterator would throw a CME. But we 
don't go back to the iterator as we return after removing.

Nevertheless, I can still remove the comment or change it... let me know.

Thanks
Christoph


From: Roger Riggs [mailto:roger.ri...@oracle.com]
Sent: Mittwoch, 18. Oktober 2017 17:28
To: Langer, Christoph ; net-dev@openjdk.java.net
Subject: Re: RFR(S): 8155590: Dubious collection management in 
sun.net.www.http.KeepAliveCache

Hi Christoph,

Looks ok.

The comment in remove() about CCE can be removed.  ArrayDeque is not thread safe
and doesn't check for CCE (and the method is synchronized).

Thanks, Roger

On 10/18/17 6:05 AM, Langer, Christoph wrote:
Hi Roger,

thanks for looking. So you motivated me to do some more cleanup. :)
Here is the new webrev: 
http://cr.openjdk.java.net/~clanger/webrevs/8155590.1/<http://cr.openjdk.java.net/%7Eclanger/webrevs/8155590.1/>
I replaced the Stack with an ArrayDeque and did some more rather cosmetical 
changes to make KeepAliveCache more appealing. I verified the change by running 
the jtreg tests jdk/sun/net/www/http/* and it all passes.

As for the CCE: I don't think this should be checked as the Stack/ArrayDeque is 
typed to KeepAliveEntry and no code appears to be in place which could trick 
this.

Best regards
Christoph

From: net-dev [mailto:net-dev-boun...@openjdk.java.net] On Behalf Of Roger Riggs
Sent: Dienstag, 17. Oktober 2017 16:53
To: net-dev@openjdk.java.net<mailto:net-dev@openjdk.java.net>
Subject: Re: RFR(S): 8155590: Dubious collection management in 
sun.net.www.http.KeepAliveCache

Hi,

Keep the synchronized, it will be low overhead since the Vector operations
are synchronized and in the same thread.

I think a CCE could occur during the iteration to find the entry when 
Vector.Itr.next() checks.

(It you want to more radical fix, replace the Vector with something more 
current.
It would be one less Vector).

Roger


On 10/16/17 2:33 AM, Langer, Christoph wrote:
Hi Vyom,

thanks for your feedback.

I'm not so sure about dropping "synchronized". In the new remove method of 
ClientVector we are iterating ourself. If this is not done under 
synchronization, there is risk to run into a ConcurrentModificationException. 
But under the assumption that all access to ClientVector comes from 
synchronized methods of KeepAliveCache, one could argue to drop all 
synchronized modifiers for ClientVector, though.

Let's wait for other opinions :)

Best regards
Christoph


From: net-dev [mailto:net-dev-boun...@openjdk.java.net] On Behalf Of vyom tewari
Sent: Montag, 16. Oktober 2017 10:27
To: net-dev@openjdk.java.net<mailto:net-dev@openjdk.java.net>
Subject: Re: RFR(S): 8155590: Dubious collection management in 
sun.net.www.http.KeepAliveCache


Hi Christoph,

Thanks for doing this, i think you don't need to synchronize the 
"remove(HttpClient h)".  This remove is get called from synchronize "remove 
(HttpClient h, Object obj)" and the underline data structure is which is 
java.util.Vector(ClientVector extends java.util.Stack) is also thread safe.

What do you think ?

Thanks,

Vyom

On Monday 16 October 2017 12:52 PM, Langer, Christoph wrote:
Hi,

Here is a proposal for a fix for bug 8155590. I already made this fix a while 
ago in our JDK clone and I'd like to contribute this.

Bug: https://bugs.openjdk.java.net/browse/JDK-8155590
Webrev: 
http://cr.openjdk.java.net/~clanger/webrevs/8155590.0/<http://cr.openjdk.java.net/%7Eclanger/webrevs/8155590.0/>

Please review.

Thanks
Christoph






RE: RFR 8145635 : Add TCP_QUICKACK socket option

2017-10-18 Thread Langer, Christoph
Hi Vyom,

I just looked at your latest webrev which in general looks fine to me.  Some 
minor remarks:

make/lib/Lib-jdk.net.gmk:
- copyright year needs to be updated

src/java.base/unix/classes/java/net/PlainDatagramSocketImpl.java:
- private void addExtSocketOptions(...) looks a bit awful concerning its 
formatting. I suggest something like this:

private void addExtSocketOptions(Set> extOptions,
 Set> options) {
// TCP_QUICKACK is applicable for TCP Sockets only.
extOptions.stream()
.filter((option) -> !option.name().equals("TCP_QUICKACK"))
.forEach((option) -> options.add(option));
}

src/jdk.net/share/classes/jdk/net/ExtendedSocketOptions.java:
- copyright year needs to be updated
- the equals sign should be placed on line 98 here:

98 public static final SocketOption TCP_QUICKACK
99 = new ExtSocketOption("TCP_QUICKACK", Boolean.class);

Other than that you should consider it reviewed from my end. No need for 
further webrev...

Best regards
Christoph

> -Original Message-
> From: net-dev [mailto:net-dev-boun...@openjdk.java.net] On Behalf Of
> vyom tewari
> Sent: Dienstag, 17. Oktober 2017 10:37
> To: net-dev@openjdk.java.net
> Subject: Re: RFR 8145635 : Add TCP_QUICKACK socket option
> 
> Hi Roger,
> 
> Thanks for the review , please find the updated
> webrev(http://cr.openjdk.java.net/~vtewari/8145635/webrev0.6/index.htm
> l).
> 
> Thanks,
> 
> Vyom
> 
> 
> On Tuesday 17 October 2017 06:35 AM, Roger Riggs wrote:
> > Hi Vyom,
> >
> > A few suggestions:
> >
> > PlainDatagramSocketImpl.java:
> >  - line 95/96:  I think you can use just forEach, the order version is
> > not necessary.
> >     The code will be a bit more readable if the .filter and .forEach
> > are on a new line and don't wrap.
> >     You can also remove the extra "(" and ")
> >
> >  - line 87-94: these are confusing and imply some implicit resetting
> > of the option.
> >  - use @since 10
> > - 209/268: the native setQuickAck method should use boolean as its
> > argument to enable/disable
> >   Since enable is a boolean; it does not need "== true'
> >
> > LinuxSocketOptions.java/c:
> >   - 52: setQuickAck0 should use boolean for the 2nd argument; (The
> > native code already does)
> >
> > Thanks, Roger
> >
> >
> > On 10/15/17 11:58 PM, vyom tewari wrote:
> >> Hi Chris,
> >>
> >> Thanks for review. Please find the latest
> >>
> webrev(http://cr.openjdk.java.net/~vtewari/8145635/webrev0.5/index.htm
> l).
> >>
> >> Thanks,
> >>
> >> Vyom
> >>
> >



RE: RFR(S): 8155590: Dubious collection management in sun.net.www.http.KeepAliveCache

2017-10-18 Thread Langer, Christoph
Hi Roger,

thanks for looking. So you motivated me to do some more cleanup. :)
Here is the new webrev: http://cr.openjdk.java.net/~clanger/webrevs/8155590.1/
I replaced the Stack with an ArrayDeque and did some more rather cosmetical 
changes to make KeepAliveCache more appealing. I verified the change by running 
the jtreg tests jdk/sun/net/www/http/* and it all passes.

As for the CCE: I don't think this should be checked as the Stack/ArrayDeque is 
typed to KeepAliveEntry and no code appears to be in place which could trick 
this.

Best regards
Christoph

From: net-dev [mailto:net-dev-boun...@openjdk.java.net] On Behalf Of Roger Riggs
Sent: Dienstag, 17. Oktober 2017 16:53
To: net-dev@openjdk.java.net
Subject: Re: RFR(S): 8155590: Dubious collection management in 
sun.net.www.http.KeepAliveCache

Hi,

Keep the synchronized, it will be low overhead since the Vector operations
are synchronized and in the same thread.

I think a CCE could occur during the iteration to find the entry when 
Vector.Itr.next() checks.

(It you want to more radical fix, replace the Vector with something more 
current.
It would be one less Vector).

Roger

On 10/16/17 2:33 AM, Langer, Christoph wrote:
Hi Vyom,

thanks for your feedback.

I'm not so sure about dropping "synchronized". In the new remove method of 
ClientVector we are iterating ourself. If this is not done under 
synchronization, there is risk to run into a ConcurrentModificationException. 
But under the assumption that all access to ClientVector comes from 
synchronized methods of KeepAliveCache, one could argue to drop all 
synchronized modifiers for ClientVector, though.

Let's wait for other opinions :)

Best regards
Christoph


From: net-dev [mailto:net-dev-boun...@openjdk.java.net] On Behalf Of vyom tewari
Sent: Montag, 16. Oktober 2017 10:27
To: net-dev@openjdk.java.net<mailto:net-dev@openjdk.java.net>
Subject: Re: RFR(S): 8155590: Dubious collection management in 
sun.net.www.http.KeepAliveCache


Hi Christoph,

Thanks for doing this, i think you don't need to synchronize the 
"remove(HttpClient h)".  This remove is get called from synchronize "remove 
(HttpClient h, Object obj)" and the underline data structure is which is 
java.util.Vector(ClientVector extends java.util.Stack) is also thread safe.

What do you think ?

Thanks,

Vyom

On Monday 16 October 2017 12:52 PM, Langer, Christoph wrote:
Hi,

Here is a proposal for a fix for bug 8155590. I already made this fix a while 
ago in our JDK clone and I'd like to contribute this.

Bug: https://bugs.openjdk.java.net/browse/JDK-8155590
Webrev: 
http://cr.openjdk.java.net/~clanger/webrevs/8155590.0/<http://cr.openjdk.java.net/%7Eclanger/webrevs/8155590.0/>

Please review.

Thanks
Christoph





RE: RFR(S): 8155590: Dubious collection management in sun.net.www.http.KeepAliveCache

2017-10-16 Thread Langer, Christoph
Hi Vyom,

thanks for your feedback.

I'm not so sure about dropping "synchronized". In the new remove method of 
ClientVector we are iterating ourself. If this is not done under 
synchronization, there is risk to run into a ConcurrentModificationException. 
But under the assumption that all access to ClientVector comes from 
synchronized methods of KeepAliveCache, one could argue to drop all 
synchronized modifiers for ClientVector, though.

Let's wait for other opinions :)

Best regards
Christoph


From: net-dev [mailto:net-dev-boun...@openjdk.java.net] On Behalf Of vyom tewari
Sent: Montag, 16. Oktober 2017 10:27
To: net-dev@openjdk.java.net
Subject: Re: RFR(S): 8155590: Dubious collection management in 
sun.net.www.http.KeepAliveCache


Hi Christoph,

Thanks for doing this, i think you don't need to synchronize the 
"remove(HttpClient h)".  This remove is get called from synchronize "remove 
(HttpClient h, Object obj)" and the underline data structure is which is 
java.util.Vector(ClientVector extends java.util.Stack) is also thread safe.

What do you think ?

Thanks,

Vyom

On Monday 16 October 2017 12:52 PM, Langer, Christoph wrote:
Hi,

Here is a proposal for a fix for bug 8155590. I already made this fix a while 
ago in our JDK clone and I'd like to contribute this.

Bug: https://bugs.openjdk.java.net/browse/JDK-8155590
Webrev: 
http://cr.openjdk.java.net/~clanger/webrevs/8155590.0/<http://cr.openjdk.java.net/%7Eclanger/webrevs/8155590.0/>

Please review.

Thanks
Christoph




RFR(S): 8155590: Dubious collection management in sun.net.www.http.KeepAliveCache

2017-10-16 Thread Langer, Christoph
Hi,

Here is a proposal for a fix for bug 8155590. I already made this fix a while 
ago in our JDK clone and I'd like to contribute this.

Bug: https://bugs.openjdk.java.net/browse/JDK-8155590
Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8155590.0/

Please review.

Thanks
Christoph



RE: RFR 8085875/10, java/net/DatagramSocket/PortUnreachable.java fails intermittently: Address already in use

2017-09-07 Thread Langer, Christoph
Hi Mark,

I overlooked that, you are completely right, recreateServerSocket is fine then.

Best regards
Christoph

> -Original Message-
> From: Mark Sheppard [mailto:mark.shepp...@oracle.com]
> Sent: Donnerstag, 7. September 2017 13:07
> To: Langer, Christoph ; Felix Yang
> 
> Cc: net-dev@openjdk.java.net
> Subject: Re: RFR 8085875/10,
> java/net/DatagramSocket/PortUnreachable.java fails intermittently: Address
> already in use
> 
> Hi Christoph,
>     looking at the logic of the test, it is seen that a "server socket"
> is created in the execute method, and is then closed,
> and a bunch of datagrams sent to it.  Then in the serverSend method an
> attempt is made to (re-)create a DatagramSocket using the
> same port as the original "server socket", so recreate would seem to
> convey an appropriate semantics.
> WRT an upper limit for retryCount, that's a reasonable point, but
> probably a "static final int" rather than just static field.
> 
> regards
> Mark
> 
> On 07/09/2017 10:40, Langer, Christoph wrote:
> > Hi Felix,
> >
> > this looks good in general.
> >
> > I would, however, suggest to rename the method 'recreateServerSocket'
> into ' createServerSocket' as the former name suggests that something
> which existed once was recreated. But in this case the socket is simply
> created with a few retries when exceptions occur.
> >
> > I'd also prefer if the retry count could be stored in a static field to 
> > allow for
> easy modification, instead of literally coding it to the value of 5 in the
> method.
> >
> > And a minor thing: in line 63 there's a blank in between
> "recreateServerSocket" and "(int serverPort)" which should be removed.
> >
> > Best regards
> > Christoph
> >
> >> -Original Message-
> >> From: net-dev [mailto:net-dev-boun...@openjdk.java.net] On Behalf Of
> >> Felix Yang
> >> Sent: Mittwoch, 6. September 2017 10:06
> >> To: net-dev@openjdk.java.net; Mark Sheppard
> >> 
> >> Subject: RFR 8085875/10,
> java/net/DatagramSocket/PortUnreachable.java
> >> fails intermittently: Address already in use
> >>
> >> Hi all,
> >>
> >>       please review a patch to add retries to avoid possible port
> >> conflicts during re-binding.
> >>
> >> Bug:
> >>
> >>       https://bugs.openjdk.java.net/browse/JDK-8085875
> >>
> >> Webrev:
> >>
> >>       http://cr.openjdk.java.net/~xiaofeya/8085875/webrev.00/
> >>
> >> Thanks,
> >>
> >> Felix



RE: RFR 8085875/10, java/net/DatagramSocket/PortUnreachable.java fails intermittently: Address already in use

2017-09-07 Thread Langer, Christoph
Hi Felix,

this looks good in general.

I would, however, suggest to rename the method 'recreateServerSocket' into ' 
createServerSocket' as the former name suggests that something which existed 
once was recreated. But in this case the socket is simply created with a few 
retries when exceptions occur.

I'd also prefer if the retry count could be stored in a static field to allow 
for easy modification, instead of literally coding it to the value of 5 in the 
method.

And a minor thing: in line 63 there's a blank in between "recreateServerSocket" 
and "(int serverPort)" which should be removed.

Best regards
Christoph

> -Original Message-
> From: net-dev [mailto:net-dev-boun...@openjdk.java.net] On Behalf Of
> Felix Yang
> Sent: Mittwoch, 6. September 2017 10:06
> To: net-dev@openjdk.java.net; Mark Sheppard
> 
> Subject: RFR 8085875/10, java/net/DatagramSocket/PortUnreachable.java
> fails intermittently: Address already in use
> 
> Hi all,
> 
>      please review a patch to add retries to avoid possible port
> conflicts during re-binding.
> 
> Bug:
> 
>      https://bugs.openjdk.java.net/browse/JDK-8085875
> 
> Webrev:
> 
>      http://cr.openjdk.java.net/~xiaofeya/8085875/webrev.00/
> 
> Thanks,
> 
> Felix



RE: RFR : 8182672: Java 8u121 on Linux intermittently returns null for MAC address

2017-06-23 Thread Langer, Christoph
Hi,

looks like a great piece of modern concurrent Java coding :) Well done! +1

Best regards
Christoph

> -Original Message-
> From: Chris Hegarty [mailto:chris.hega...@oracle.com]
> Sent: Freitag, 23. Juni 2017 15:28
> To: Seán Coffey ; Langer, Christoph
> 
> Cc: net-dev 
> Subject: Re: RFR : 8182672: Java 8u121 on Linux intermittently returns null 
> for
> MAC address
> 
> 
> 
> On 23/06/17 10:56, Seán Coffey wrote:
> > Thanks to Christoph, Vyom and Mark for the reviews.
> >
> > I've improved the testcase as per feedback. Hope this meets all requests :
> >
> > http://cr.openjdk.java.net/~coffeys/webrev.8182672.jdk10.v2/webrev/
> 
> The change looks good.
> 
> Sean and I did a live coding session and arrived at the following
> version of the test.
> 
> -Chris.
> 
> /*
>   * Copyright (c) 2017, 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
>   * under the terms of the GNU General Public License version 2 only, as
>   * published by the Free Software Foundation.
>   *
>   * This code is distributed in the hope that it will be useful, but WITHOUT
>   * ANY WARRANTY; without even the implied warranty of
> MERCHANTABILITY or
>   * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public
> License
>   * version 2 for more details (a copy is included in the LICENSE file that
>   * accompanied this code).
>   *
>   * You should have received a copy of the GNU General Public License
> version
>   * 2 along with this work; if not, write to the Free Software Foundation,
>   * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
>   *
>   * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065
> USA
>   * or visit www.oracle.com if you need additional information or have any
>   * questions.
>   */
> 
> /*
>   * @test
>   * @bug 8182672
>   * @summary Java 8u121 on Linux intermittently returns null for MAC
> address
>   */
> 
> import java.net.NetworkInterface;
> import java.util.ArrayList;
> import java.util.List;
> import java.util.concurrent.Callable;
> import java.util.concurrent.ExecutorService;
> import java.util.concurrent.Executors;
> import java.util.concurrent.Future;
> import java.util.concurrent.Phaser;
> import java.util.function.Predicate;
> import java.util.stream.Collectors;
> 
> public class GetMacAddress implements Callable {
>  static final int NUM_THREADS = 5;
>  static final int NUM_ITERS = 100;
>  static volatile boolean failed; // false
> 
>  final String threadName;
>  final NetworkInterface ni;
>  final Phaser startingGate;
> 
>  public GetMacAddress(NetworkInterface ni, String name, Phaser phaser) {
>  this.ni = ni;
>  this.threadName = name;
>  this.startingGate = phaser;
>  }
> 
>  @Override
>  public Exception call() {
>  int count = 0;
>  startingGate.arriveAndAwaitAdvance();
>  try {
>  for (int i = 0; i < NUM_ITERS; i++) {
>  ni.getMTU();
>  byte[] addr = ni.getHardwareAddress();
>  if (addr == null) {
>  System.out.println(threadName + ". mac id is null");
>  failed = true;
>  }
>  count = count + 1;
>  if (count % 100 == 0) {
>  System.out.println(threadName + ". count is " + count);
>  }
>  }
>  } catch (Exception ex) {
>  System.out.println(threadName + ". Not expecting
> exception:" + ex.getMessage());
>  failed = true;
>  return ex;
>  }
>  return null;
>  }
> 
>  static final Predicate hasHardwareAddress = ni -> {
>  try {
>  if (ni.getHardwareAddress() == null) {
>  System.out.println("Not testing null addr: " +
> ni.getName());
>  return false;
>  }
>  } catch (Exception ex) {
>  System.out.println("Not testing: " + ni.getName() +
>  " " + ex.getMessage());
>  }
>  return true;
>  };
> 
>  public static void main(String[] args) throws Exception {
>  List toTest =
> NetworkInterface.networkInterfaces()
>  .filter(hasHardwareAddress)
>  .collect(Collectors.toList());
> 
>

RE: RFR : 8182672: Java 8u121 on Linux intermittently returns null for MAC address

2017-06-23 Thread Langer, Christoph
Hi Seán,

this looks very good.

I've got a few minor points:

1. In line 65 where you catch a possible unexpected exception, you could also 
mention the thread name in the output.
2. I would check for failure condition at the end of each test loop(line 91, 
for (NetworkInterface ni : toTest)... ) after a particular interface was tested 
and in the case of failed==true, terminate the loop. If we see an error with 
one interface, it's probably not necessary to test others.
3. line 100,101:
while (!executor.isTerminated()) {
}
This looks a bit odd. Maybe you should call executor.awaitTermination() inside 
the loop? Or you could as well use executor.invokeAll() which will return only 
after all threads are done.
4. line 96: 
new GetMacAddress(ni, ni.getName() +"-Thread-" + i);
There's a space missing between + and "-Thread-".

Best regards
Christoph



> -Original Message-
> From: Seán Coffey [mailto:sean.cof...@oracle.com]
> Sent: Freitag, 23. Juni 2017 11:57
> To: Langer, Christoph 
> Cc: Vyom Tewari ; net-dev  d...@openjdk.java.net>; Mark Sheppard 
> Subject: Re: RFR : 8182672: Java 8u121 on Linux intermittently returns null 
> for
> MAC address
> 
> Thanks to Christoph, Vyom and Mark for the reviews.
> 
> I've improved the testcase as per feedback. Hope this meets all requests :
> 
> http://cr.openjdk.java.net/~coffeys/webrev.8182672.jdk10.v2/webrev/
> 
> Regards,
> Sean.
> 
> On 22/06/17 22:36, Langer, Christoph wrote:
> > Hi Sean,
> >
> > I played with it a bit more and now really understand Vyoms observation.
> So, what he sees is not the original concurrency issue but he encounters a
> SocketException on some interface, where this is supposed to occur upon
> calling getHardwareAddress().
> >
> > So, to enable the testcase to run robustly on any platform, any hardware
> and any network configuration, I suggest to modify the test like this:
> >
> > 1. In main, enumerate all interfaces once
> > 2. Iterate over all interfaces, exlude loopback and see if a single call to
> getHardwareAddress() won't yield null or an Exception.
> > 3. For each interface where a valid mac address could be obtained once,
> start the executor threads to stress getHardwareAddress() in parallel, e.g.
> like 5 threads doing it 100 times in parallel. I would also suggest to use per
> thread counters instead of one global 'count' as we have right now.
> >
> > Furthermore, the test output could be improved a bit, e.g. when it comes
> to an exception, the thread name could be mentioned, too.
> >
> > Best regards
> > Christoph
> >
> >> -Original Message-
> >> From: net-dev [mailto:net-dev-boun...@openjdk.java.net] On Behalf Of
> >> Seán Coffey
> >> Sent: Donnerstag, 22. Juni 2017 20:17
> >> To: Vyom Tewari ; net-dev  >> d...@openjdk.java.net>
> >> Subject: Re: RFR : 8182672: Java 8u121 on Linux intermittently returns null
> for
> >> MAC address
> >>
> >> Hi Vyom,
> >>
> >> thanks for testing. Null is a valid value for some interfaces. I've
> >> excluded the loopback interface from being testing. Perhaps there's
> >> other issues at play here.  We know that getHardwareAddress() can
> throw
> >> SocketException if I/O fails. For this particular scenario we're not
> >> interested in that and perhaps that can be ignored.
> >>
> >> I'll take another look.
> >>
> >> regards,
> >> Sean.
> >>
> >> On 22/06/2017 18:50, Vyom Tewari wrote:
> >>> Hi Sean,
> >>>
> >>> with your patch as well your test case is failing on my
> >>> laptop(Ubuntu16.04), when i tried to run  on jdk8 i am getting below
> >>> error.
> >>>
> >>> java.net.SocketException: No such device (ioctl(SIOCGIFHWADDR)
> failed)
> >>>  at java.net.NetworkInterface.getMacAddr0(Native Method)
> >>>  at
> >>>
> >>
> java.net.NetworkInterface.getHardwareAddress(NetworkInterface.java:457
> >> )
> >>>  at com.java.test.GetMacAddress.run(GetMacAddress.java:66)
> >>>  at
> >>>
> >>
> java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.jav
> >> a:1142)
> >>>  at
> >>>
> >>
> java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.ja
> >> va:617)
> >>>  at java.lang.Thread.run(Thread.java:745)
> >>> java.net.SocketException: No such device (ioctl

RE: RFR : 8182672: Java 8u121 on Linux intermittently returns null for MAC address

2017-06-22 Thread Langer, Christoph
Hi Sean,

I played with it a bit more and now really understand Vyoms observation. So, 
what he sees is not the original concurrency issue but he encounters a 
SocketException on some interface, where this is supposed to occur upon calling 
getHardwareAddress().

So, to enable the testcase to run robustly on any platform, any hardware and 
any network configuration, I suggest to modify the test like this:

1. In main, enumerate all interfaces once
2. Iterate over all interfaces, exlude loopback and see if a single call to 
getHardwareAddress() won't yield null or an Exception.
3. For each interface where a valid mac address could be obtained once, start 
the executor threads to stress getHardwareAddress() in parallel, e.g. like 5 
threads doing it 100 times in parallel. I would also suggest to use per thread 
counters instead of one global 'count' as we have right now.

Furthermore, the test output could be improved a bit, e.g. when it comes to an 
exception, the thread name could be mentioned, too.

Best regards
Christoph

> -Original Message-
> From: net-dev [mailto:net-dev-boun...@openjdk.java.net] On Behalf Of
> Seán Coffey
> Sent: Donnerstag, 22. Juni 2017 20:17
> To: Vyom Tewari ; net-dev  d...@openjdk.java.net>
> Subject: Re: RFR : 8182672: Java 8u121 on Linux intermittently returns null 
> for
> MAC address
> 
> Hi Vyom,
> 
> thanks for testing. Null is a valid value for some interfaces. I've
> excluded the loopback interface from being testing. Perhaps there's
> other issues at play here.  We know that getHardwareAddress() can throw
> SocketException if I/O fails. For this particular scenario we're not
> interested in that and perhaps that can be ignored.
> 
> I'll take another look.
> 
> regards,
> Sean.
> 
> On 22/06/2017 18:50, Vyom Tewari wrote:
> > Hi Sean,
> >
> > with your patch as well your test case is failing on my
> > laptop(Ubuntu16.04), when i tried to run  on jdk8 i am getting below
> > error.
> >
> > java.net.SocketException: No such device (ioctl(SIOCGIFHWADDR) failed)
> > at java.net.NetworkInterface.getMacAddr0(Native Method)
> > at
> >
> java.net.NetworkInterface.getHardwareAddress(NetworkInterface.java:457
> )
> > at com.java.test.GetMacAddress.run(GetMacAddress.java:66)
> > at
> >
> java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.jav
> a:1142)
> > at
> >
> java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.ja
> va:617)
> > at java.lang.Thread.run(Thread.java:745)
> > java.net.SocketException: No such device (ioctl(SIOCGIFHWADDR) failed)
> > at java.net.NetworkInterface.getMacAddr0(Native Method)
> > at
> >
> java.net.NetworkInterface.getHardwareAddress(NetworkInterface.java:457
> )
> > at com.java.test.GetMacAddress.run(GetMacAddress.java:66)
> > at
> >
> java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.jav
> a:1142)
> > at
> >
> java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.ja
> va:617)
> > at java.lang.Thread.run(Thread.java:745)
> > Exception in thread "main" java.lang.RuntimeException: Failed
> > at com.java.test.GetMacAddress.main(GetMacAddress.java:96)
> > mac id is null for interface cscotun0- Thread0
> > Testing: cscotun0
> > mac id is null for interface cscotun0- Thread3
> > Testing: cscotun0
> >
> > Thanks,
> >
> > Vyom
> >
> >
> > On Thursday 22 June 2017 09:59 PM, Seán Coffey wrote:
> >> JDK 10 fix required to correct a race issue in NetworkInterface. I
> >> don't believe the ifreq struct needs to be static in any case. New
> >> auto unit testcase also. I propose to skip this fix for JDK 9 and fix
> >> in an update release for that family. I also plan to port this to
> >> jdk8u-dev.
> >>
> >> https://bugs.openjdk.java.net/browse/JDK-8182672
> >> webrev :
> >> http://cr.openjdk.java.net/~coffeys/webrev.8182672.jdk10/webrev/
> >>
> >> regards,
> >> Sean.
> >



RE: RFR : 8182672: Java 8u121 on Linux intermittently returns null for MAC address

2017-06-22 Thread Langer, Christoph
Hi Sean,

the patch to change the static declaration of struct ifreq ifr into a non 
static declaration should be done in any case. I had suspected the issue should 
disappear then but as Vyom told otherwise, I'll also try to test this.

As for the test: NetworkInterface.getNetworkInterfaces() will return a typed 
enumeration, so you could iterate like:

for (NetworkInterface ni : NetworkInterface.getNetworkInterfaces()) {
   ...
}

Then the code would be more compact.

I'll get back to you with the results of my testing.

Best regards
Christoph

> -Original Message-
> From: Seán Coffey [mailto:sean.cof...@oracle.com]
> Sent: Donnerstag, 22. Juni 2017 18:29
> To: OpenJDK Network Dev list ; Langer,
> Christoph 
> Subject: RFR : 8182672: Java 8u121 on Linux intermittently returns null for
> MAC address
> 
> JDK 10 fix required to correct a race issue in NetworkInterface. I don't
> believe the ifreq struct needs to be static in any case. New auto unit
> testcase also. I propose to skip this fix for JDK 9 and fix in an update
> release for that family. I also plan to port this to jdk8u-dev.
> 
> https://bugs.openjdk.java.net/browse/JDK-8182672
> webrev :
> http://cr.openjdk.java.net/~coffeys/webrev.8182672.jdk10/webrev/
> 
> regards,
> Sean.


RE: RFR (xxs): 8180424: Another build issue on AIX after 8034174

2017-05-17 Thread Langer, Christoph
Hi Thomas,

this looks good and should definitely be downported to JDK9 as soon as possible.

Best regards
Christoph

From: ppc-aix-port-dev [mailto:ppc-aix-port-dev-boun...@openjdk.java.net] On 
Behalf Of Thomas Stüfe
Sent: Dienstag, 16. Mai 2017 14:51
To: ppc-aix-port-...@openjdk.java.net; net-dev 
Subject: RFR (xxs): 8180424: Another build issue on AIX after 8034174

Hi all,

may I have a review for this tiny fix:

Issue: https://bugs.openjdk.java.net/browse/JDK-8180424
webrev: 
http://cr.openjdk.java.net/~stuefe/webrevs/8180424-another-build-issue-on-aix-after-8034174/webrev.00/webrev/

The prototypes for NET_RecvFrom and NET_Accept do not match their 
implementations for AIX since 8034174. This did not lead to an error in jdk9 
because there, the header (net_util_md.h) was not included by aix_close.c. In 
JDK10, it is included and therefore does not build.

I believe this did not lead to a runtime error on jdk9, at least not for the 
typical values involved; the mismatch is between int* and unsigned int* (native 
socklen_t).

Kind Regards, Thomas


RE: RFR 8179602: Fix for JDK-8165437 is broken on 32-bit Linux

2017-05-04 Thread Langer, Christoph
Hi Vyom,

the fix looks good and seems straightforward to resolve the reported issue.

Small item: I spotted an extra space in net_util_md.h, line 75, between "jlong" 
and "nanoTimeStamp" which could be removed.

Reviewed.

Best regards
Christoph

> -Original Message-
> From: net-dev [mailto:net-dev-boun...@openjdk.java.net] On Behalf Of
> Vyom Tewari
> Sent: Donnerstag, 4. Mai 2017 10:50
> To: net-dev 
> Subject: RFR 8179602: Fix for JDK-8165437 is broken on 32-bit Linux
> 
> Hi All,
> 
> Please  review the below change.
> 
> Webrev:
> http://cr.openjdk.java.net/~vtewari/8179602/webrev0.0/index.html
> 
> Bugid: https://bugs.openjdk.java.net/browse/JDK-8179602
> 
> This issue is because of side effect of "JDK-8165437" where we are using
> "JVM_NanoTime" which returns a 64 bit jlong and return value was getting
> assigned to long type. On 32 bit OS long is 4 byte, which leads to
> integer overflow.
> 
> Our internal test JPRT is  still running.
> 
> Thanks,
> 
> Vyom



RE: JDK10 RFR: 8165437 Evaluate the use of gettimeofday in Networking code

2017-04-27 Thread Langer, Christoph
Hi Vyom,

I’ve just got a small formatting remark about the order of includes:

Generally I tried to follow the rule 1. Common os headers, 2. Platform os 
headers,  3. Jvm/jdk headers, 4. JNI headers in my latest refactorings. So, to 
keep this up, can you move #include “jvm.h” in the line before #include 
“net_util.h” in each file? And pull it before the JNI headers. Like, e.g. in 
net_util_md.c you should move #include "jvm.h" to line 52.

Thanks & Best regards
Christoph

From: net-dev [mailto:net-dev-boun...@openjdk.java.net] On Behalf Of Vyom Tewari
Sent: Donnerstag, 27. April 2017 06:16
To: Thomas Stüfe ; Chris Hegarty 

Cc: net-dev 
Subject: Re: JDK10 RFR: 8165437 Evaluate the use of gettimeofday in Networking 
code


Hi,

please find the updated 
webrev(http://cr.openjdk.java.net/~vtewari/8165437/webrev0.7/index.html).

Thanks,

Vyom



On Tuesday 25 April 2017 07:34 PM, Thomas Stüfe wrote:
Hi Chris, Vyom,

I have preferences as expressed earlier, but no strong emotions. I can live 
with the fix as it is now.

Thanks all, and Kind Regards, Thomas


On Tue, Apr 25, 2017 at 3:31 PM, Chris Hegarty 
mailto:chris.hega...@oracle.com>> wrote:
> On Tue, Apr 18, 2017 at 7:08 AM, Vyom Tewari 
> mailto:vyom.tew...@oracle.com>> wrote:
> ...
>
> Thanks for review, please find the updated 
> webrev(http://cr.openjdk.java.net/~vtewari/8165437/webrev0.6/index.html)

The changes mainly look good to me, just a few comments:

1) src/java.base/unix/native/libnet/PlainSocketImpl.c

   L235 jlong nanoTimeout = timeout * NET_NSEC_PER_MSEC;

   Can you please move this to the latest block of code that requires it, i.e..
   just after L327 if (connect_rv != 0) { …

2) src/java.base/share/native/libnet/net_util.h

   Should these definitions be moved to 
src/java.base/unix/native/libnet/net_util_md.h?


Regarding JVM_NanoTime being a JVM_LEAF and/or taking a JNIEnv (
both of which are, in todays hotspot VM, effectively ignored ), this is a
separate issue. I have raised it off list with others from the VM team,
without much interest. I will refrain from filing a JIRA issue to track 
potential
changes in the VM for this. Others are welcome to do so, if they wish. I
suggest that we simply continue to pass a valid JNIEnv, since it is not
much extra effort to do so. ( this can be refactored later, if the VM interface
is ever updated ).


> On 24 Apr 2017, at 12:07, Thomas Stüfe 
> mailto:thomas.stu...@gmail.com>> wrote:
>
> ...
> That aside, I am not a big fan of the removal of the old NET_Timeout. Before, 
> there were two functions, "NET_Timeout" just taking the timeout value, 
> "NET_Timeout0" taking a timeout and a start value. You removed the first 
> variant and therefore added the many additional JVM_NanoTime() calls at 
> NET_Timeout callsites. This makes the code harder to read and also kind of 
> exposes the internal implementation of NET_Timeout (namely the fact that 
> NET_Timeout uses JVM_NanoTime for time measurement). Could you please let 
> both variants live, optionally with better names?

I think that it may not be worth added back the simpler variant. It
would only be used by PlainDatagramSocketImpl, correct? All other
usages would use the variant that accepts the current nano time.

-Chris.




RE: [PATCH] 8035653: jdk8u152-b01 windows crash on DatagramSocket.getLocalAddress

2017-04-24 Thread Langer, Christoph
Hi Alex,

to me your patch looks good.

But since I'm not a JDK8 Reviewer, someone with this merits has to review it. 
@Chris: May I ask you to have a look?

Thanks
Christoph

> -Original Message-
> From: Alex Kashchenko [mailto:akash...@redhat.com]
> Sent: Montag, 24. April 2017 15:22
> To: Langer, Christoph 
> Cc: net-dev@openjdk.java.net
> Subject: Re: [PATCH] 8035653: jdk8u152-b01 windows crash on
> DatagramSocket.getLocalAddress
> 
> Hi Christoph,
> 
> On 04/19/2017 10:53 AM, Langer, Christoph wrote:
> > Hi Alex,
> >
> > I've just quickly checked this and it seems worthwile to me to downport
> 8035653 to JDK8.
> >
> > As the patch will probably not apply cleanly to JDK8 after unshuffling [1] ,
> you will need to create a new public review and post it on this mailing list.
> Some JDK8 reviewer needs to review it and then you need to request
> approval for push into jdk8u-dev using this mail template [2]. After you've
> posted the webrev I can help you with the process.
> >
> > Generally, information about Java 8 backports can be found here: [3].
> 
> Thanks for your comments! I uploaded review [1] to downport 8035653 [2]
> to jdk8u/jdk8u-dev.
> 
> Patch from jdk9/dev [3] does not apply cleanly, change to
> DualStackPlainDatagramSocketImpl.java was already added to jdk8 as part
> of 8072466 [4][5].
> 
> This patch brings changes to DualStackPlainDatagramSocketImpl.c and also
> includes B8035653.java test.
> 
> 
> [1] http://cr.openjdk.java.net/~akasko/jdk8u/8035653/webrev.00/
> [2] https://bugs.openjdk.java.net/browse/JDK-8035653
> [3] http://hg.openjdk.java.net/jdk9/dev/jdk/rev/8e5e92e0530e
> [4] https://bugs.openjdk.java.net/browse/JDK-8072466
> [5] http://hg.openjdk.java.net/jdk8u/jdk8u-dev/jdk/rev/3623f1b29b58#l7.1
> 
> >
> > Best regards
> > Christoph
> >
> > [1] http://cr.openjdk.java.net/~chegar/docs/portingScript.html
> > [2] http://openjdk.java.net/projects/jdk8u/approval-template.html
> > [3] http://openjdk.java.net/projects/jdk8u/
> >
> >
> >
> >
> >> -Original Message-
> >> From: net-dev [mailto:net-dev-boun...@openjdk.java.net] On Behalf Of
> >> Alex Kashchenko
> >> Sent: Donnerstag, 23. März 2017 00:51
> >> To: net-dev@openjdk.java.net
> >> Subject: [PATCH] 8035653: jdk8u152-b01 windows crash on
> >> DatagramSocket.getLocalAddress
> >>
> >> Hi,
> >>
> >> We found that 8035653 test from jdk9 [1] crashes jdk8u152-b01 on
> windows
> >> at this point [2] because "ia6_class" is not initialized.
> >>
> >> It looks like the following bit of 8035653 is missed in jdk8u152-b01:
> >>
> >> diff -r 83726fe0f756
> >> src/windows/native/java/net/DualStackPlainDatagramSocketImpl.c
> >> --- a/src/windows/native/java/net/DualStackPlainDatagramSocketImpl.c
> >> Tue Mar 21 17:08:03 2017 -0700
> >> +++
> b/src/windows/native/java/net/DualStackPlainDatagramSocketImpl.c
> >> Wed Mar 22 23:18:30 2017 +
> >> @@ -87,6 +87,8 @@
> >>   IO_fd_fdID = NET_GetFileDescriptorID(env);
> >>   CHECK_NULL(IO_fd_fdID);
> >>   JNU_CHECK_EXCEPTION(env);
> >> +
> >> +initInetAddressIDs(env);
> >>   }
> >>
> >> If it will be convenient, I can submit an issue+webrev and re-send this
> >> to jdk8u-dev list. But as 8u152 is not yet released and the fix is
> >> already in jdk9, I will appreciate some guidance on this problem.
> >>
> >>
> >> [1]
> >>
> http://hg.openjdk.java.net/jdk9/jdk9/jdk/file/f6bf027e88e9/test/java/net/
> >> DatagramSocket/B8035653.java
> >> [2]
> >>
> http://hg.openjdk.java.net/jdk8u/jdk8u/jdk/file/83726fe0f756/src/share/na
> >> tive/java/net/net_util.c#l222
> >>
> >> --
> >> -Alex
> 
> 
> --
> -Alex


RE: [PATCH] 8035653: jdk8u152-b01 windows crash on DatagramSocket.getLocalAddress

2017-04-19 Thread Langer, Christoph
Hi Alex,

I've just quickly checked this and it seems worthwile to me to downport 8035653 
to JDK8.

As the patch will probably not apply cleanly to JDK8 after unshuffling [1] , 
you will need to create a new public review and post it on this mailing list. 
Some JDK8 reviewer needs to review it and then you need to request approval for 
push into jdk8u-dev using this mail template [2]. After you've posted the 
webrev I can help you with the process.

Generally, information about Java 8 backports can be found here: [3].

Best regards
Christoph

[1] http://cr.openjdk.java.net/~chegar/docs/portingScript.html
[2] http://openjdk.java.net/projects/jdk8u/approval-template.html
[3] http://openjdk.java.net/projects/jdk8u/




> -Original Message-
> From: net-dev [mailto:net-dev-boun...@openjdk.java.net] On Behalf Of
> Alex Kashchenko
> Sent: Donnerstag, 23. März 2017 00:51
> To: net-dev@openjdk.java.net
> Subject: [PATCH] 8035653: jdk8u152-b01 windows crash on
> DatagramSocket.getLocalAddress
> 
> Hi,
> 
> We found that 8035653 test from jdk9 [1] crashes jdk8u152-b01 on windows
> at this point [2] because "ia6_class" is not initialized.
> 
> It looks like the following bit of 8035653 is missed in jdk8u152-b01:
> 
> diff -r 83726fe0f756
> src/windows/native/java/net/DualStackPlainDatagramSocketImpl.c
> --- a/src/windows/native/java/net/DualStackPlainDatagramSocketImpl.c
> Tue Mar 21 17:08:03 2017 -0700
> +++ b/src/windows/native/java/net/DualStackPlainDatagramSocketImpl.c
> Wed Mar 22 23:18:30 2017 +
> @@ -87,6 +87,8 @@
>   IO_fd_fdID = NET_GetFileDescriptorID(env);
>   CHECK_NULL(IO_fd_fdID);
>   JNU_CHECK_EXCEPTION(env);
> +
> +initInetAddressIDs(env);
>   }
> 
> If it will be convenient, I can submit an issue+webrev and re-send this
> to jdk8u-dev list. But as 8u152 is not yet released and the fix is
> already in jdk9, I will appreciate some guidance on this problem.
> 
> 
> [1]
> http://hg.openjdk.java.net/jdk9/jdk9/jdk/file/f6bf027e88e9/test/java/net/
> DatagramSocket/B8035653.java
> [2]
> http://hg.openjdk.java.net/jdk8u/jdk8u/jdk/file/83726fe0f756/src/share/na
> tive/java/net/net_util.c#l222
> 
> --
> -Alex


RE: Let's improve IPv6 support

2017-03-18 Thread Langer, Christoph
Hi Martin and colleagues,

Sounds good. I’m supporting that and I’m willing to assist with 
testing/reviewing.

But if it comes to larger changes and JEPs/CCCs (or the new CSR), it will need 
an owner at Oracle… like Chris.

Best regards
Christoph

From: net-dev [mailto:net-dev-boun...@openjdk.java.net] On Behalf Of Martin 
Buchholz
Sent: Freitag, 17. März 2017 19:48
To: net-dev ; Alexander Smundak 
; Paul Marks ; Chuck Rasbold 
; Chris Hegarty 
Subject: Let's improve IPv6 support

Google cares a lot about IPv6, and not only because Vint Cerf works at Google.

We have some local modifications and some networking expertise and intend to 
port/contribute that to openjdk10.

Most of this is the work of my colleagues Alexander Smundak and Paul Marks.

We hope we can do most of this work without asking too much of other net-dev 
engineers, but we will likely need help with porting to non-Linux platforms, 
running JPRT, and perhaps CCC/JEP process.

We expect to make a large number of commits.  Should there be a JEP to cover 
this?  Is there a current net-dev engineer who feels they "own" this problem 
(Chris?)?


RE: RFR(M): 8167457: Fixes for InetAddressImpl native coding on Windows

2017-01-26 Thread Langer, Christoph
Hi Michael,

Thanks for reviewing. So I'll push them in jdk10.

Best regards
Christoph

From: Michael McMahon [mailto:michael.x.mcma...@oracle.com]
Sent: Donnerstag, 26. Januar 2017 15:51
To: Langer, Christoph 
Cc: Chris Hegarty ; net-dev@openjdk.java.net
Subject: Re: RFR(M): 8167457: Fixes for InetAddressImpl native coding on Windows

Christoph,

I would prefer to not push these native code change to JDK 9 at this stage.
They look fine for JDK 10 though. Consider them both reviewed.

Thanks
Michael.

On 26/01/2017, 10:02, Langer, Christoph wrote:
Hi Chris, Michael,

as I consider this change final from my side and the jdk10 forest is open now, 
I guess it is a good point to come back to this.

I personally would prefer if I could still submit it to jdk9, given that it is 
automatically consolidated to jdk10 and the code base would remain the same. 
The change set is only a cleanup to the Inet*AddressImpl.c coding and should be 
quite manageable.

The same goes for this one: 
http://mail.openjdk.java.net/pipermail/net-dev/2016-December/010571.html

In any case, I'd like you to decide this now and give me a review in short 
term, that I can get it out of my head. :)

Thanks & Best regards
Christoph

From: Langer, Christoph
Sent: Mittwoch, 21. Dezember 2016 15:57
To: net-dev@openjdk.java.net<mailto:net-dev@openjdk.java.net>
Subject: RFR(M): 8167457: Fixes for InetAddressImpl native coding on Windows

Hi again,

after pushing the change for 8171077 
(http://hg.openjdk.java.net/jdk9/dev/jdk/rev/24f8703890b2), I have updated my 
patch for 8167457 and I have also updated the bug description to reflect the 
reduced scope.

Bug: https://bugs.openjdk.java.net/browse/JDK-8167457
Webrev: 
http://cr.openjdk.java.net/~clanger/webrevs/8167457.0/<http://cr.openjdk.java.net/%7Eclanger/webrevs/8167457.0/>

The change now merely is code cleanup which should not change the current 
behavior apart from removing the isspace(hostname[0]) check and the 
isDottedIPAddress check in Java_java_net_Inet4AddressImpl_lookupAllHostAddr. 
But testing lets me think it is not needed any longer for the supported Windows 
versions of JDK9.

Same as with 8167420, from my point of view this change is ready to push and it 
runs nightly in our builds/tests of OpenJDK on several platforms. Though I 
guess you want to postpone this to JDK10, I'd be glad if you could still 
consider it for JDK9. But I won't pressure otherwise ;-)

Thanks & Best regards
Christoph



RE: RFR(M): 8167457: Fixes for InetAddressImpl native coding on Windows

2017-01-26 Thread Langer, Christoph
Hi Chris, Michael,

as I consider this change final from my side and the jdk10 forest is open now, 
I guess it is a good point to come back to this.

I personally would prefer if I could still submit it to jdk9, given that it is 
automatically consolidated to jdk10 and the code base would remain the same. 
The change set is only a cleanup to the Inet*AddressImpl.c coding and should be 
quite manageable.

The same goes for this one: 
http://mail.openjdk.java.net/pipermail/net-dev/2016-December/010571.html

In any case, I'd like you to decide this now and give me a review in short 
term, that I can get it out of my head. :)

Thanks & Best regards
Christoph

From: Langer, Christoph
Sent: Mittwoch, 21. Dezember 2016 15:57
To: net-dev@openjdk.java.net
Subject: RFR(M): 8167457: Fixes for InetAddressImpl native coding on Windows

Hi again,

after pushing the change for 8171077 
(http://hg.openjdk.java.net/jdk9/dev/jdk/rev/24f8703890b2), I have updated my 
patch for 8167457 and I have also updated the bug description to reflect the 
reduced scope.

Bug: https://bugs.openjdk.java.net/browse/JDK-8167457
Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8167457.0/

The change now merely is code cleanup which should not change the current 
behavior apart from removing the isspace(hostname[0]) check and the 
isDottedIPAddress check in Java_java_net_Inet4AddressImpl_lookupAllHostAddr. 
But testing lets me think it is not needed any longer for the supported Windows 
versions of JDK9.

Same as with 8167420, from my point of view this change is ready to push and it 
runs nightly in our builds/tests of OpenJDK on several platforms. Though I 
guess you want to postpone this to JDK10, I'd be glad if you could still 
consider it for JDK9. But I won't pressure otherwise ;-)

Thanks & Best regards
Christoph



RE: RFR: 8170544: Fix code scan findings in libnet

2017-01-13 Thread Langer, Christoph
Hi,

I finally pushed the change: 
http://hg.openjdk.java.net/jdk9/dev/jdk/rev/f3115622562a

Best regards
Christoph

> -Original Message-
> From: Langer, Christoph
> Sent: Mittwoch, 11. Januar 2017 16:50
> To: 'Chris Hegarty' 
> Cc: Lindenmaier, Goetz ; net-
> d...@openjdk.java.net; Michael McMahon 
> Subject: RE: RFR: 8170544: Fix code scan findings in libnet
> 
> Hi Chris,
> 
> thanks for looking.
> 
> > 1) NetworkInterface.c
> >
> > I’m not sure that the close is really necessary, since a JNI pending
> > exception can only be thrown is sock is less than 0. I think just
> > removing the ' && (*env)->ExceptionOccurred(env)’ from the original
> > if statement should be sufficient, no?
> 
> I think you are right. An exception can only occur if socket is less than 0. 
> There
> is one case where socket could be less than 0 and no exception occurred. In
> that case we'd probably see an exception later on. But I think it would be 
> fine to
> return NULL in case socket is < 0. Generally, this coding needs to be 
> revisited in
> order to make it work for IPv6 only systems. We should do that when finishing
> up bug 8148424 [1].
> 
> > 2) net_util.c
> >
> > getInet6Address_scopeid_set should CHECK_NULL_RETURN(holder,
> JNI_FALSE)?
> > getInet6Address_scopeid now returns an unsigned in, why
> CHECK_NULL_RETURN(holder, -1)?
> >
> > Some of this, existing, code seems a little dubious.
> >
> 
> Good catch. The CHECK_NULL_RETURN macros need adaption. The reason why
> getInet6Address_scopeid should return unsigned int is that the struct
> sockaddr_in6 is also using unsigned int for the scope, e.g. on Linux [2] or
> Windows [3].
> 
> I've addressed your points in
> http://cr.openjdk.java.net/~clanger/webrevs/8170544.2/ Would you want to
> run this through JPRT again?
> 
> I would go and push it towards the end of the week.
> 
> Best regards
> Christoph
> 
> [1] https://bugs.openjdk.java.net/browse/JDK-8148424
> [2] http://man7.org/linux/man-pages/man7/ipv6.7.html
> [3] https://msdn.microsoft.com/en-
> us/library/windows/hardware/ff570824(v=vs.85).aspx
> 



  1   2   3   >