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

2020-06-29 Thread Vyom Tiwari
Hi Alan,Daniel, Please find the latest webrev( http://cr.openjdk.java.net/~vtewari/8237858/webrev0.7/index.html). I have added a new native method and removed the hardcoding(SIGPIPE). I gave a thought on creating a separate thread and sending a signal but it will further increase the complexity

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

2020-06-29 Thread Severin Gehwolf
Hi Vyom! On Fri, 2020-06-26 at 15:55 +0530, Vyom Tiwari wrote: > Hi Severin, > > Overall code changes looks ok to me, I build & tested at my local REL > it worked fine for me. Thanks for the review! > Below are few minor comments. > > 1-> Net.java > 1.1-> I think you don't need to explicit

Re: RFR 8245245 : WebSocket can loose the URL encoding of URI query parameters

2020-06-29 Thread Chris Hegarty
> On 26 Jun 2020, at 18:18, Daniel Fuchs wrote: > > I concur. Rahul has convinced me. > Rahul also pointed me to a test that verifies that the IAE is > thrown, so I believe that > http://cr.openjdk.java.net/~ryadav/webrev_8245245/index.html LGTM. -Chris.

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

2020-06-29 Thread Vyom Tiwari
Hi Severin, Latest code looks good, I think in src/solaris/native/java/net/ExtendedOptionsImpl.c, you don't need #ifdef blocks. In case of "flowOption" it was required because flow is specific to Solaris. In case of KEEPALIVE we are using the POSIX api(setsockopt/getsockopt) which exists on all P

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

2020-06-29 Thread Severin Gehwolf
Hi Vyom, On Mon, 2020-06-29 at 17:11 +0530, Vyom Tiwari wrote: > Hi Severin, > > Latest code looks good Thanks! > I think in src/solaris/native/java/net/ExtendedOptionsImpl.c, you > don't need #ifdef blocks. In case of "flowOption" it was required > because flow is specific to Solaris. > > In

8246114: java/net/MulticastSocket/Promiscuous.java fails after 8241072 (multi-homed systems)

2020-06-29 Thread Daniel Fuchs
Hi, Please find a trivial fix for: 8246114: java/net/MulticastSocket/Promiscuous.java fails after 8241072 (multi-homed systems) https://bugs.openjdk.java.net/browse/JDK-8246114 webrev: http://cr.openjdk.java.net/~dfuchs/webrev_8246114/webrev.00/ The test should only use NetworkInterfa

Re: 8246114: java/net/MulticastSocket/Promiscuous.java fails after 8241072 (multi-homed systems)

2020-06-29 Thread Alan Bateman
On 29/06/2020 15:09, Daniel Fuchs wrote: Hi, Please find a trivial fix for: 8246114: java/net/MulticastSocket/Promiscuous.java fails after 8241072 (multi-homed systems) https://bugs.openjdk.java.net/browse/JDK-8246114 webrev: http://cr.openjdk.java.net/~dfuchs/webrev_8246114/webrev.00

Re: 8246114: java/net/MulticastSocket/Promiscuous.java fails after 8241072 (multi-homed systems)

2020-06-29 Thread Daniel Fuchs
Ah! Good catch Alan! http://cr.openjdk.java.net/~dfuchs/webrev_8246114/webrev.01/ best regards, -- daniel On 29/06/2020 15:40, Alan Bateman wrote: Do you need to do the same filtering at L158? I guess I would have expected leaveGroup to fail because the joinGroup has been skipped.

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

2020-06-29 Thread Vyom Tiwari
Hi Severin, thanks for clarification, we can still simplify the ExtendedOptionsImpl.c. Please have a look on below change and do let me know if it makes sense. I moved the "#if defined(__linux__) || defined(MACOSX)" inside the corresponding methods and this way we will eliminate lot's of duplica

Re: 8246114: java/net/MulticastSocket/Promiscuous.java fails after 8241072 (multi-homed systems)

2020-06-29 Thread Alan Bateman
On 29/06/2020 16:01, Daniel Fuchs wrote: Ah! Good catch Alan! http://cr.openjdk.java.net/~dfuchs/webrev_8246114/webrev.01/ This looks good to me.

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

2020-06-29 Thread Severin Gehwolf
Hi Yyom, On Mon, 2020-06-29 at 21:18 +0530, Vyom Tiwari wrote: > Hi Severin, > > thanks for clarification, we can still simplify the > ExtendedOptionsImpl.c. Please have a look on below change and do let > me know if it makes sense. > > I moved the "#if defined(__linux__) || defined(MACOSX)" ins

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

2020-06-29 Thread Daniel Fuchs
Hi Vyom, On 29/06/2020 08:03, Vyom Tiwari wrote: Hi Alan,Daniel, Please find the latest webrev(http://cr.openjdk.java.net/~vtewari/8237858/webrev0.7/index.html). I have added a new native method and removed the hardcoding(SIGPIPE). That fails to build on windows: [2020-06-29T15:26:16,899Z

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

2020-06-29 Thread Andrew Haley
On 29/06/2020 17:06, Severin Gehwolf wrote: > It's a possibility. IMHO, it doesn't really make the code easier to > read, though. Some duplication for clarity seems OK to me in this case. > I'm not too fond of over-use of ifdef so I'd rather keep it at v5. > YMMV. OK. -- Andrew Haley (he/him) J

Re: 8246114: java/net/MulticastSocket/Promiscuous.java fails after 8241072 (multi-homed systems)

2020-06-29 Thread Amy Lu
On 6/29/20 11:01 PM, Daniel Fuchs wrote: http://cr.openjdk.java.net/~dfuchs/webrev_8246114/webrev.01/ Looks good. (And I verified the fix on the system where issue reproducible.) Thank you Daniel! Thanks, Amy

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

2020-06-29 Thread Vyom Tiwari
Hi Daniel, Thanks for review please find the latest webrev( http://cr.openjdk.java.net/~vtewari/8237858/webrev0.8/index.html). I fixed the Windows build issue. Thanks, Vyom On Mon, Jun 29, 2020 at 10:23 PM Daniel Fuchs wrote: > Hi Vyom, > > > On 29/06/2020 08:03, Vyom Tiwari wrote: > > Hi Alan