RFR: 8170920 SO_RCVBUF and SO_SNDBUF options problem for network channels on MacOS

classic Classic list List threaded Threaded
10 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

RFR: 8170920 SO_RCVBUF and SO_SNDBUF options problem for network channels on MacOS

Michael McMahon
Could I get the following change reviewed please?

http://cr.openjdk.java.net/~michaelm/8170920/webrev.1/

Thanks,
Michael.
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR: 8170920 SO_RCVBUF and SO_SNDBUF options problem for network channels on MacOS

Chris Hegarty

On 09/12/16 10:33, Michael McMahon wrote:
> Could I get the following change reviewed please?
>
> http://cr.openjdk.java.net/~michaelm/8170920/webrev.1/

Looks good.

You may just want to remove the "from" year from the copyright
year range in the test, before pushing.

-Chris.
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

RE: RFR: 8170920 SO_RCVBUF and SO_SNDBUF options problem for network channels on MacOS

Langer, Christoph
Hi Michael,

the bug then obviously was a side effect of my change http://hg.openjdk.java.net/jdk9/dev/jdk/rev/9f0ab4b20ff7 for 8167481. Sorry for that.

To follow the concept of my cleanups I'd prefer if you could use #if defined(MACOSX) rather than #ifdef MACOSX in net_util_md.c to be consistent within the file.

I also followed the platform order 1. Linux, 2. AIX, 3. Solaris, 4. Mac in all parts of the native code, so your newly introduced section should go after the "#if defined(__solaris__)" system header include parts, e.g. line 47 of the original file.

Thanks
Christoph

> -----Original Message-----
> From: net-dev [mailto:[hidden email]] On Behalf Of Chris
> Hegarty
> Sent: Freitag, 9. Dezember 2016 11:52
> To: Michael McMahon <[hidden email]>; OpenJDK Network
> Dev list <[hidden email]>
> Subject: Re: RFR: 8170920 SO_RCVBUF and SO_SNDBUF options problem for
> network channels on MacOS
>
>
> On 09/12/16 10:33, Michael McMahon wrote:
> > Could I get the following change reviewed please?
> >
> > http://cr.openjdk.java.net/~michaelm/8170920/webrev.1/
>
> Looks good.
>
> You may just want to remove the "from" year from the copyright
> year range in the test, before pushing.
>
> -Chris.
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR: 8170920 SO_RCVBUF and SO_SNDBUF options problem for network channels on MacOS

Bernd Eckenfels-4
In reply to this post by Michael McMahon
Should the test maybe assert that a change was made (or maybe be a bit more whitebox and assert the result is >64k? Should it test SND+RCV?





On Fri, Dec 9, 2016 at 12:04 PM +0100, "Michael McMahon" <[hidden email]> wrote:

Could I get the following change reviewed please?

http://cr.openjdk.java.net/~michaelm/8170920/webrev.1/

Thanks,
Michael.
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR: 8170920 SO_RCVBUF and SO_SNDBUF options problem for network channels on MacOS

Michael McMahon
Bernd,

I'll add a test for SO_SNDBUF. These parameters are too system dependent
to use hard coded values in the tests though. I'm not sure you could depend
on the initial/default value never being the same as the maximum value either.

Thanks,
Michael

On 09/12/2016, 11:31, Bernd Eckenfels wrote:
Should the test maybe assert that a change was made (or maybe be a bit more whitebox and assert the result is >64k? Should it test SND+RCV?





On Fri, Dec 9, 2016 at 12:04 PM +0100, "Michael McMahon" <[hidden email]> wrote:

Could I get the following change reviewed please?

http://cr.openjdk.java.net/~michaelm/8170920/webrev.1/

Thanks,
Michael.
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR: 8170920 SO_RCVBUF and SO_SNDBUF options problem for network channels on MacOS

Michael McMahon
In reply to this post by Langer, Christoph
Those suggestions are reasonable Christoph. I think given the subtle
nature of the bug
(removing the #include not causing a compile error) I'm a bit wary about
pushing wide scale
native code changes at this stage of JDK 9. We might therefore defer the
other cleanups until 10
opens up in the new year.

Michael.

On 09/12/2016, 11:16, Langer, Christoph wrote:

> Hi Michael,
>
> the bug then obviously was a side effect of my change http://hg.openjdk.java.net/jdk9/dev/jdk/rev/9f0ab4b20ff7 for 8167481. Sorry for that.
>
> To follow the concept of my cleanups I'd prefer if you could use #if defined(MACOSX) rather than #ifdef MACOSX in net_util_md.c to be consistent within the file.
>
> I also followed the platform order 1. Linux, 2. AIX, 3. Solaris, 4. Mac in all parts of the native code, so your newly introduced section should go after the "#if defined(__solaris__)" system header include parts, e.g. line 47 of the original file.
>
> Thanks
> Christoph
>
>> -----Original Message-----
>> From: net-dev [mailto:[hidden email]] On Behalf Of Chris
>> Hegarty
>> Sent: Freitag, 9. Dezember 2016 11:52
>> To: Michael McMahon<[hidden email]>; OpenJDK Network
>> Dev list<[hidden email]>
>> Subject: Re: RFR: 8170920 SO_RCVBUF and SO_SNDBUF options problem for
>> network channels on MacOS
>>
>>
>> On 09/12/16 10:33, Michael McMahon wrote:
>>> Could I get the following change reviewed please?
>>>
>>> http://cr.openjdk.java.net/~michaelm/8170920/webrev.1/
>> Looks good.
>>
>> You may just want to remove the "from" year from the copyright
>> year range in the test, before pushing.
>>
>> -Chris.
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

RE: RFR: 8170920 SO_RCVBUF and SO_SNDBUF options problem for network channels on MacOS

Langer, Christoph
Hi Michael,

yes, I understand and agree that it's probably not the right time to do larger refactoring at this time in JDK9. And, yes, the changes for 8167420 and 8167457 have evolved to turn the native Inet*AddressImpl files completely over, so we should therefore defer the full cleanup to JDK10.

However, I still like to get 2 things into the current repo - which were the initial triggers for my changes. One is to remove the duplicate coding for a rather obsolete BSD environment in Inet4AddresImpl and the other is to use getnameinfo/getaddrinfo in Windows Inet4AddressImpl. I'll open new bugs for that and post the reduced changesets for review.

Best regards
Christoph

> -----Original Message-----
> From: Michael McMahon [mailto:[hidden email]]
> Sent: Freitag, 9. Dezember 2016 17:19
> To: Langer, Christoph <[hidden email]>
> Cc: Chris Hegarty <[hidden email]>; OpenJDK Network Dev list
> <[hidden email]>
> Subject: Re: RFR: 8170920 SO_RCVBUF and SO_SNDBUF options problem for
> network channels on MacOS
>
> Those suggestions are reasonable Christoph. I think given the subtle
> nature of the bug
> (removing the #include not causing a compile error) I'm a bit wary about
> pushing wide scale
> native code changes at this stage of JDK 9. We might therefore defer the
> other cleanups until 10
> opens up in the new year.
>
> Michael.
>
> On 09/12/2016, 11:16, Langer, Christoph wrote:
> > Hi Michael,
> >
> > the bug then obviously was a side effect of my change
> http://hg.openjdk.java.net/jdk9/dev/jdk/rev/9f0ab4b20ff7 for 8167481. Sorry
> for that.
> >
> > To follow the concept of my cleanups I'd prefer if you could use #if
> defined(MACOSX) rather than #ifdef MACOSX in net_util_md.c to be consistent
> within the file.
> >
> > I also followed the platform order 1. Linux, 2. AIX, 3. Solaris, 4. Mac in all
> parts of the native code, so your newly introduced section should go after the
> "#if defined(__solaris__)" system header include parts, e.g. line 47 of the
> original file.
> >
> > Thanks
> > Christoph
> >
> >> -----Original Message-----
> >> From: net-dev [mailto:[hidden email]] On Behalf Of
> Chris
> >> Hegarty
> >> Sent: Freitag, 9. Dezember 2016 11:52
> >> To: Michael McMahon<[hidden email]>; OpenJDK
> Network
> >> Dev list<[hidden email]>
> >> Subject: Re: RFR: 8170920 SO_RCVBUF and SO_SNDBUF options problem for
> >> network channels on MacOS
> >>
> >>
> >> On 09/12/16 10:33, Michael McMahon wrote:
> >>> Could I get the following change reviewed please?
> >>>
> >>> http://cr.openjdk.java.net/~michaelm/8170920/webrev.1/
> >> Looks good.
> >>
> >> You may just want to remove the "from" year from the copyright
> >> year range in the test, before pushing.
> >>
> >> -Chris.
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

RE: RFR: 8170920 SO_RCVBUF and SO_SNDBUF options problem for network channels on MacOS

Lindenmaier, Goetz
In reply to this post by Michael McMahon
Hi Michael,

given the recent changes that were submitted (jigsaw update, aot)
I don't think Christoph's changes can be considered a major risk.
They have been worked on for quite a while now, and they are
tested well on our side.
So I really would appreciate if they could be submitted as-is.
Reworking them into smaller changes can introduce new issues
and will delay the push of the changes further.
I especially would like to see 8170544 finding its way into jdk9.

Best regards,
  Goetz.

> -----Original Message-----
> From: Langer, Christoph
> Sent: Dienstag, 13. Dezember 2016 10:54
> To: Lindenmaier, Goetz <[hidden email]>
> Subject: FW: RFR: 8170920 SO_RCVBUF and SO_SNDBUF options problem for
> network channels on MacOS
>
>
>
> -----Original Message-----
> From: Michael McMahon [mailto:[hidden email]]
> Sent: Freitag, 9. Dezember 2016 17:19
> To: Langer, Christoph <[hidden email]>
> Cc: Chris Hegarty <[hidden email]>; OpenJDK Network Dev list
> <[hidden email]>
> Subject: Re: RFR: 8170920 SO_RCVBUF and SO_SNDBUF options problem for
> network channels on MacOS
>
> Those suggestions are reasonable Christoph. I think given the subtle
> nature of the bug
> (removing the #include not causing a compile error) I'm a bit wary about
> pushing wide scale
> native code changes at this stage of JDK 9. We might therefore defer the
> other cleanups until 10
> opens up in the new year.
>
> Michael.
>
> On 09/12/2016, 11:16, Langer, Christoph wrote:
> > Hi Michael,
> >
> > the bug then obviously was a side effect of my change
> http://hg.openjdk.java.net/jdk9/dev/jdk/rev/9f0ab4b20ff7 for 8167481. Sorry
> for that.
> >
> > To follow the concept of my cleanups I'd prefer if you could use #if
> defined(MACOSX) rather than #ifdef MACOSX in net_util_md.c to be consistent
> within the file.
> >
> > I also followed the platform order 1. Linux, 2. AIX, 3. Solaris, 4. Mac in all
> parts of the native code, so your newly introduced section should go after the
> "#if defined(__solaris__)" system header include parts, e.g. line 47 of the
> original file.
> >
> > Thanks
> > Christoph
> >
> >> -----Original Message-----
> >> From: net-dev [mailto:[hidden email]] On Behalf Of
> Chris
> >> Hegarty
> >> Sent: Freitag, 9. Dezember 2016 11:52
> >> To: Michael McMahon<[hidden email]>; OpenJDK
> Network
> >> Dev list<[hidden email]>
> >> Subject: Re: RFR: 8170920 SO_RCVBUF and SO_SNDBUF options problem for
> >> network channels on MacOS
> >>
> >>
> >> On 09/12/16 10:33, Michael McMahon wrote:
> >>> Could I get the following change reviewed please?
> >>>
> >>> http://cr.openjdk.java.net/~michaelm/8170920/webrev.1/
> >> Looks good.
> >>
> >> You may just want to remove the "from" year from the copyright
> >> year range in the test, before pushing.
> >>
> >> -Chris.
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR: 8170920 SO_RCVBUF and SO_SNDBUF options problem for network channels on MacOS

Chris Hegarty
Hi Goetz,

Christoph has already done the work to split these changes into smaller
chunks, 8171075 and 8171077. He has reviews out, and they are on my list
to get to in the next few days. If it is ok, I’d like to proceed with this as is,
form an initial skim they appear easier to review.

-Chris.

> On 13 Dec 2016, at 10:14, Lindenmaier, Goetz <[hidden email]> wrote:
>
> Hi Michael,
>
> given the recent changes that were submitted (jigsaw update, aot)
> I don't think Christoph's changes can be considered a major risk.
> They have been worked on for quite a while now, and they are
> tested well on our side.
> So I really would appreciate if they could be submitted as-is.
> Reworking them into smaller changes can introduce new issues
> and will delay the push of the changes further.
> I especially would like to see 8170544 finding its way into jdk9.
>
> Best regards,
>  Goetz.
>
>> -----Original Message-----
>> From: Langer, Christoph
>> Sent: Dienstag, 13. Dezember 2016 10:54
>> To: Lindenmaier, Goetz <[hidden email]>
>> Subject: FW: RFR: 8170920 SO_RCVBUF and SO_SNDBUF options problem for
>> network channels on MacOS
>>
>>
>>
>> -----Original Message-----
>> From: Michael McMahon [mailto:[hidden email]]
>> Sent: Freitag, 9. Dezember 2016 17:19
>> To: Langer, Christoph <[hidden email]>
>> Cc: Chris Hegarty <[hidden email]>; OpenJDK Network Dev list
>> <[hidden email]>
>> Subject: Re: RFR: 8170920 SO_RCVBUF and SO_SNDBUF options problem for
>> network channels on MacOS
>>
>> Those suggestions are reasonable Christoph. I think given the subtle
>> nature of the bug
>> (removing the #include not causing a compile error) I'm a bit wary about
>> pushing wide scale
>> native code changes at this stage of JDK 9. We might therefore defer the
>> other cleanups until 10
>> opens up in the new year.
>>
>> Michael.
>>
>> On 09/12/2016, 11:16, Langer, Christoph wrote:
>>> Hi Michael,
>>>
>>> the bug then obviously was a side effect of my change
>> http://hg.openjdk.java.net/jdk9/dev/jdk/rev/9f0ab4b20ff7 for 8167481. Sorry
>> for that.
>>>
>>> To follow the concept of my cleanups I'd prefer if you could use #if
>> defined(MACOSX) rather than #ifdef MACOSX in net_util_md.c to be consistent
>> within the file.
>>>
>>> I also followed the platform order 1. Linux, 2. AIX, 3. Solaris, 4. Mac in all
>> parts of the native code, so your newly introduced section should go after the
>> "#if defined(__solaris__)" system header include parts, e.g. line 47 of the
>> original file.
>>>
>>> Thanks
>>> Christoph
>>>
>>>> -----Original Message-----
>>>> From: net-dev [mailto:[hidden email]] On Behalf Of
>> Chris
>>>> Hegarty
>>>> Sent: Freitag, 9. Dezember 2016 11:52
>>>> To: Michael McMahon<[hidden email]>; OpenJDK
>> Network
>>>> Dev list<[hidden email]>
>>>> Subject: Re: RFR: 8170920 SO_RCVBUF and SO_SNDBUF options problem for
>>>> network channels on MacOS
>>>>
>>>>
>>>> On 09/12/16 10:33, Michael McMahon wrote:
>>>>> Could I get the following change reviewed please?
>>>>>
>>>>> http://cr.openjdk.java.net/~michaelm/8170920/webrev.1/
>>>> Looks good.
>>>>
>>>> You may just want to remove the "from" year from the copyright
>>>> year range in the test, before pushing.
>>>>
>>>> -Chris.

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

RE: RFR: 8170920 SO_RCVBUF and SO_SNDBUF options problem for network channels on MacOS

Langer, Christoph
Hi Chris, Goetz,

I just want to mention that 8170544 is independent from the other changes to InetAddress and would apply with or without the patches from 8167420, 8167457, 8171075 and 8171077.

I'd prefer if we can close 8171075 and 8171077 soon and then look further if 8170544 can still be considered for 9 :)

Thanks
Christoph

> -----Original Message-----
> From: Chris Hegarty [mailto:[hidden email]]
> Sent: Dienstag, 13. Dezember 2016 11:34
> To: Lindenmaier, Goetz <[hidden email]>
> Cc: Langer, Christoph <[hidden email]>;
> [hidden email]; OpenJDK Network Dev list <net-
> [hidden email]>
> Subject: Re: RFR: 8170920 SO_RCVBUF and SO_SNDBUF options problem for
> network channels on MacOS
>
> Hi Goetz,
>
> Christoph has already done the work to split these changes into smaller
> chunks, 8171075 and 8171077. He has reviews out, and they are on my list
> to get to in the next few days. If it is ok, I’d like to proceed with this as is,
> form an initial skim they appear easier to review.
>
> -Chris.
>
> > On 13 Dec 2016, at 10:14, Lindenmaier, Goetz <[hidden email]>
> wrote:
> >
> > Hi Michael,
> >
> > given the recent changes that were submitted (jigsaw update, aot)
> > I don't think Christoph's changes can be considered a major risk.
> > They have been worked on for quite a while now, and they are
> > tested well on our side.
> > So I really would appreciate if they could be submitted as-is.
> > Reworking them into smaller changes can introduce new issues
> > and will delay the push of the changes further.
> > I especially would like to see 8170544 finding its way into jdk9.
> >
> > Best regards,
> >  Goetz.
> >
> >> -----Original Message-----
> >> From: Langer, Christoph
> >> Sent: Dienstag, 13. Dezember 2016 10:54
> >> To: Lindenmaier, Goetz <[hidden email]>
> >> Subject: FW: RFR: 8170920 SO_RCVBUF and SO_SNDBUF options problem
> for
> >> network channels on MacOS
> >>
> >>
> >>
> >> -----Original Message-----
> >> From: Michael McMahon [mailto:[hidden email]]
> >> Sent: Freitag, 9. Dezember 2016 17:19
> >> To: Langer, Christoph <[hidden email]>
> >> Cc: Chris Hegarty <[hidden email]>; OpenJDK Network Dev list
> >> <[hidden email]>
> >> Subject: Re: RFR: 8170920 SO_RCVBUF and SO_SNDBUF options problem for
> >> network channels on MacOS
> >>
> >> Those suggestions are reasonable Christoph. I think given the subtle
> >> nature of the bug
> >> (removing the #include not causing a compile error) I'm a bit wary about
> >> pushing wide scale
> >> native code changes at this stage of JDK 9. We might therefore defer the
> >> other cleanups until 10
> >> opens up in the new year.
> >>
> >> Michael.
> >>
> >> On 09/12/2016, 11:16, Langer, Christoph wrote:
> >>> Hi Michael,
> >>>
> >>> the bug then obviously was a side effect of my change
> >> http://hg.openjdk.java.net/jdk9/dev/jdk/rev/9f0ab4b20ff7 for 8167481.
> Sorry
> >> for that.
> >>>
> >>> To follow the concept of my cleanups I'd prefer if you could use #if
> >> defined(MACOSX) rather than #ifdef MACOSX in net_util_md.c to be
> consistent
> >> within the file.
> >>>
> >>> I also followed the platform order 1. Linux, 2. AIX, 3. Solaris, 4. Mac in all
> >> parts of the native code, so your newly introduced section should go after
> the
> >> "#if defined(__solaris__)" system header include parts, e.g. line 47 of the
> >> original file.
> >>>
> >>> Thanks
> >>> Christoph
> >>>
> >>>> -----Original Message-----
> >>>> From: net-dev [mailto:[hidden email]] On Behalf Of
> >> Chris
> >>>> Hegarty
> >>>> Sent: Freitag, 9. Dezember 2016 11:52
> >>>> To: Michael McMahon<[hidden email]>; OpenJDK
> >> Network
> >>>> Dev list<[hidden email]>
> >>>> Subject: Re: RFR: 8170920 SO_RCVBUF and SO_SNDBUF options problem
> for
> >>>> network channels on MacOS
> >>>>
> >>>>
> >>>> On 09/12/16 10:33, Michael McMahon wrote:
> >>>>> Could I get the following change reviewed please?
> >>>>>
> >>>>> http://cr.openjdk.java.net/~michaelm/8170920/webrev.1/
> >>>> Looks good.
> >>>>
> >>>> You may just want to remove the "from" year from the copyright
> >>>> year range in the test, before pushing.
> >>>>
> >>>> -Chris.

Loading...