Re: 8158519: Incorrect network mask and broadcast address on Linux when there are multiple IPv4 addresses on an interface

[Chris Hegarty]

Thanks. Pushed:
  http://hg.openjdk.java.net/jdk9/dev/jdk/rev/ac3e32924dfb

-Chris.

> On 8 Jun 2016, at 15:07, Langer, Christoph  wrote:
> 
> Hi Chris,
> 
> I now took time to look at your proposal based on ioctl calls. This looks 
> very good. I came up with some modifications to your patch and created 
> another webrev:
> 
> http://cr.openjdk.java.net/~clanger/webrevs/8158519.1/
> 
> This builds on Linux, AIX, Solaris and Mac and the basic test to list all 
> network interfaces on these platforms still succeeds. I also tested on Linux 
> that the bug reported by Doychin is fixed.
> 
> My changes in detail:
> - picked up the suggestions of Dmitry, except for the IFF_POINTOPOINT check 
> which I left untouched
> - corrected further indentations and removed a debug fprintf output which was 
> left
> - changed the order of arguments for function addif (you may or may not like 
> this, of course ;-))
> - renamed ifr_netmaskP to ifr_subnetaddrP as I think this fits more here 
> since the subnetaddr is converted to a netmask by computeMaskFromAddress 
> (which you may or may not like, as well)
> - AIX: need to use ifreqP->ifr_addr for SIOCGIFNETMASK since 
> ifreqP->ifr_netmask is not available on this platform
> 
> From my point of view this change could be pushed now.
> 
> As for the general discussion getifaddrs vs. ioctl: I would like to continue 
> refactoring the code ending up with 2 sets of enumInterfaces implementations. 
> One getifaddrs based and the other ioctl based. And then I would introduce 
> some #ifdef section which switches to the desired path per platform. I'll 
> come up with a proposal for that after this change got pushed.
> 
> Thanks and Best regards
> Christoph
> 
> 
>> -Original Message-
>> From: net-dev [mailto:net-dev-boun...@openjdk.java.net] On Behalf Of Dmitry
>> Samersoff
>> Sent: Donnerstag, 2. Juni 2016 19:35
>> To: Chris Hegarty ; Doychin Bondzhev
>> ; OpenJDK Network Dev list > d...@openjdk.java.net>
>> Subject: Re: 8158519: Incorrect network mask and broadcast address on Linux
>> when there are multiple IPv4 addresses on an interface
>> 
>> Chris,
>> 
>> Looks good for me. Only minor nits (no need to re-review).
>> 
>> 1040 missed space before {
>> 
>> 1158 please, add comment and {} around continue.
>> 
>> 1938  I'm not sure that check for IFF_POINTOPOINT is necessary (but
>> don't mid to leave it). Do you know the case when both flags are set?
>> 
>> -Dmitry
>> 
>> On 2016-06-02 15:06, Chris Hegarty wrote:
>>> Doychin, et al,
>>> 
>>> I finally got time to look at the issue you reported and your suggested 
>>> patch. I
>> filed
>>> 8158519 [1] to track the issue. I think your suggested patch may be ok, but 
>>> I
>> just
>>> wanted to push on the ioctl approach. I found an alternative, and verified 
>>> that
>> it
>>> works as expected. Please take a look, and verify in your environment.  Then
>> we
>>> need to weigh up the two separate approaches.
>>> 
>>> http://cr.openjdk.java.net/~chegar/8158519.00/
>>> 
>>> For the record, I don’t have any specific issue with using getifaddrs, I 
>>> just
>> wanted to
>>> see if there was a less invasive change. That said, using getifaddrs could 
>>> lead
>> to
>>> cleaner code, but we would need to check the situation on AIX ( which I 
>>> don’t
>> have
>>> access to ).
>>> 
>>> -Chris.
>>> 
>>> [1] https://bugs.openjdk.java.net/browse/JDK-8158519
>>> 
> 

RFR JDK-8159039: sun/net/httpclient/hpack/HeaderTableTest.java fails on some locales

[Pavel Rappo]

Hi,

Could you please review the following change for JDK-8159039?

http://cr.openjdk.java.net/~prappo/8159039/webrev.00/

This change contains a fix for 8159039 as well as a number of tiny editorial
changes.

Thanks,
-Pavel

Re: RFR [9] 8041924: [TESTBUG] sun/net/www/http/ChunkedOutputStream/checkError.java fails on some systems

[Seán Coffey]

Nice clean up Chris. Looks fine.

Regards,
Sean.

On 09/06/16 20:49, Chris Hegarty wrote:

This test has been seen to fail intermittently. There is an assumption in the
test that the client-side will fill the outgoing TCP buffer by writing 1 
megabyte
of data, allowing for the server-side to close the connection out from under
it. This is the crux of the test. By slowing down the writes on the client-side
we can eliminate this assumption. The remainder of the changes just
modernise the code, and remove many unnecessary synchronization points.

http://cr.openjdk.java.net/~chegar/8041924/
https://bugs.openjdk.java.net/browse/JDK-8041924

-Chris.

Re: RFR JDK-8159039: sun/net/httpclient/hpack/HeaderTableTest.java fails on some locales

[Chris Hegarty]

Thanks Pavel. Reviewed.

-Chris.

On 13/06/16 11:29, Pavel Rappo wrote:

Hi,

Could you please review the following change for JDK-8159039?

http://cr.openjdk.java.net/~prappo/8159039/webrev.00/

This change contains a fix for 8159039 as well as a number of tiny editorial
changes.

Thanks,
-Pavel

Re: RFR JDK-8157273: Simplify outgoing messages queueing policy in WebSocket API

[Simone Bordet]

Hi,

On Sat, Jun 11, 2016 at 1:20 PM, Pavel Rappo  wrote:
> Simone,
>
> What's your opinion on how send(Ping|Pong|Close) should work? I mean with "one
> outstanding send" policy you're advocating for, should all types of messages
> sent by an app honor this policy?
>
> If we allow Pings, unsolicited Pongs and Close messages to jump ahead of the
> "queue" and define this as an implementation specific detail, then we might 
> run
> into one of these traps you've mentioned. When the same application might work
> pretty differently against different implementations.

Pings should be sent as soon as possible as per specification, and
this is implementation dependent.
Unsolicited pongs, never seen them used, but I'd guess same behavior as above.
Close should definitely *not* "jump ahead of the queue", it should be
sent after queued stuff has been sent.

> Should the application be in charge of chopping outgoing messages in chunks in
> order to provide better responsiveness when needed?

Not sure what you mean here.

The current API allows for partial messages. That does not mean that
every call correspond to a frame.
The reason for this API is to provide a handy way to be able to read
from an InputStream and convert the whole stream into a single
WebSocket message.
In this case the application is "in charge of chopping" in the sense
that it is how the application works.
I would not give any other "chopping" semantic to the API rather than
just a utility method to write messages when the whole message is not
completely available upfront.

To me, pings (let's leave out unsolicited pongs) are an application
mean to measure roundtrip; because they can be interleaved at the
protocol level, I may be interested in knowing when they are fully
written, but I would not make this interfere with message sending.
Say for example you have a scheduler that sends pings every X seconds,
you don't want an application to coordinate with this in order to send
messages. The application will eventually coordinate with the onPong()
response to tune the buffer size based on the roundtrip time, or
things like that.

I see the "at most one outstanding write" at the send message level
only because it has the danger of infinite buffering.
Pings are limited at 125 bytes, so yes you can still blow up the heap
if you send a gazillion of them in a tight loop, but it's a degenerate
case.

On the other hand it would be very easy to blow up the heap reading
and sending a large file, not to mention the buffer semantic.
For example, a naive implementation to read from a huge file:

FileChannel huge = ...;
ByteBuffer buffer = ...;
long start = 0;
while (true) {
  int read = huge.read(buffer, start);
  if (read < 0) {
ws.sendBinary(ByteBuffer.allocate(0), true);
break;
  } else {
start += read;
buffer.flip();
ws.sendBinary(buffer, false);
  }
}

This code has 2 problems: allows for infinite buffering, and it's not
clear for the application when it can reuse the buffer.
An implementation *must* copy the buffer data if it allows for
multiple outstanding writes, causing even more memory pressure.

If the application solves the buffer problem (because the
implementation semantic is to *not* copy), then it has at most one
outstanding write, and as such also the infinite buffering problem is
solved.

It's all about the semantic you want to give to the API.

If you say: "Implementations *must* ensure infinite buffering and data
copy semantic" then it's ok.
If you say: "Implementations don't copy data, so applications must be
careful with buffers", it's ok too.
If you say: "Implementations are free to do whatever they want", then
it's ok too.

For each of those there are additional burdens on applications, but at
least it's clear to an application what it should do.

If you choose the last, then an application has to assume the least,
and it has to write code with only one outstanding write.
At that point you (as implementer) have the burden of putting in the
API javadocs the correct examples, and explain why the snippet above
is wrong.

Thanks !

-- 
Simone Bordet
http://bordet.blogspot.com
---
Finally, no matter how good the architecture and design are,
to deliver bug-free software with optimal performance and reliability,
the implementation technique must be flawless.   Victoria Livschitz

Re: RFR JDK-8157273: Simplify outgoing messages queueing policy in WebSocket API

[Pavel Rappo]

> On 13 Jun 2016, at 12:14, Simone Bordet  wrote:
> 
> To me, pings (let's leave out unsolicited pongs) are an application
> mean to measure roundtrip; because they can be interleaved at the
> protocol level, I may be interested in knowing when they are fully
> written, but I would not make this interfere with message sending.
> Say for example you have a scheduler that sends pings every X seconds,
> you don't want an application to coordinate with this in order to send
> messages. The application will eventually coordinate with the onPong()
> response to tune the buffer size based on the roundtrip time, or
> things like that.

If you're talking about the application scheduling these Pings every X seconds,
then I have a question. Are we talking about constant rate here or constant
delay? If it's the former, then I wonder what the application should do if the
previous Ping at time (t) still hasn't been sent by the time (t + period)?

It's related to the following questions I've asked previously. 

1. Does "sendPing" have to wait until the CF returned by the previous
"sendText|sendBinary" has completed?

2. Does "sendPing" have to wait until the CF returned by the previous sendPing
has completed?

Re: RFR JDK-8157273: Simplify outgoing messages queueing policy in WebSocket API

[Simone Bordet]

Hi,

On Mon, Jun 13, 2016 at 1:44 PM, Pavel Rappo  wrote:
> If you're talking about the application scheduling these Pings every X 
> seconds,
> then I have a question. Are we talking about constant rate here or constant
> delay? If it's the former, then I wonder what the application should do if the
> previous Ping at time (t) still hasn't been sent by the time (t + period)?

If it is a mechanism to detect the network roundtrip, constant delay is fine.

> It's related to the following questions I've asked previously.
>
> 1. Does "sendPing" have to wait until the CF returned by the previous
> "sendText|sendBinary" has completed?

sendPing() schedules a ping frame to be sent as soon as possible. When
it is sent, the CF is completed.
How you handle this internally, it's an implementation detail.
If the API semantic for sendX() is "at most one outstanding write" I
would not consider a ping an outstanding write: it can be performed at
any time.

> 2. Does "sendPing" have to wait until the CF returned by the previous sendPing
> has completed?

I'd say no.

-- 
Simone Bordet
http://bordet.blogspot.com
---
Finally, no matter how good the architecture and design are,
to deliver bug-free software with optimal performance and reliability,
the implementation technique must be flawless.   Victoria Livschitz

Re: RFR JDK-8157273: Simplify outgoing messages queueing policy in WebSocket API

[Simone Bordet]

Hi,

On Mon, Jun 13, 2016 at 3:40 PM, Pavel Rappo  wrote:
>
>> On 13 Jun 2016, at 12:14, Simone Bordet  wrote:
>>
>> Close should definitely *not* "jump ahead of the queue", it should be
>> sent after queued stuff has been sent.
>
> Why is that?

Because contrary to ping, the specification does not say "as soon as
possible" but:

"An endpoint MAY delay sending a Close frame until its current message is
sent (for instance, if the majority of a fragmented message is
already sent, an endpoint MAY send the remaining fragments before
sending a Close frame)."

> Shouldn't we treat this as an implementation specific detail? I
> mean Close, Ping and Pong are control messages. They are differ from Text and
> Binary ones.
>
> If we specify this straight away, we're closing the ability for Close to 
> behave
> like this. Are you sure this kind of Close behaviour is *never* needed?
> Meanwhile it's easy to note in the API that if one wants to *force* order then
> sendClose should be chained through CSs. It should be easy, especially taking
> into account Close is the last message sent by a client.

True :)

-- 
Simone Bordet
http://bordet.blogspot.com
---
Finally, no matter how good the architecture and design are,
to deliver bug-free software with optimal performance and reliability,
the implementation technique must be flawless.   Victoria Livschitz

Re: RFR JDK-8157273: Simplify outgoing messages queueing policy in WebSocket API

[Pavel Rappo]

> On 13 Jun 2016, at 12:14, Simone Bordet  wrote:
> 
> Close should definitely *not* "jump ahead of the queue", it should be
> sent after queued stuff has been sent.

Why is that? Shouldn't we treat this as an implementation specific detail? I
mean Close, Ping and Pong are control messages. They are differ from Text and
Binary ones.

If we specify this straight away, we're closing the ability for Close to behave
like this. Are you sure this kind of Close behaviour is *never* needed?
Meanwhile it's easy to note in the API that if one wants to *force* order then
sendClose should be chained through CSs. It should be easy, especially taking
into account Close is the last message sent by a client.

Re: RFR JDK-8157273: Simplify outgoing messages queueing policy in WebSocket API

[Pavel Rappo]

> On 13 Jun 2016, at 14:45, Simone Bordet  wrote:
> 
> Because contrary to ping, the specification does not say "as soon as
> possible" but:
> 
> "An endpoint MAY delay sending a Close frame until its current message is
> sent (for instance, if the majority of a fragmented message is
> already sent, an endpoint MAY send the remaining fragments before
> sending a Close frame)."

This should be read in context :p

   If an endpoint receives a Close frame and did not previously send a
   Close frame, the endpoint MUST send a Close frame in response.  (When
   sending a Close frame in response, the endpoint typically echos the
   status code it received.)  It SHOULD do so as soon as practical. An
   endpoint MAY delay sending a Close frame until its current message is
   sent (for instance, if the majority of a fragmented message is
   already sent, an endpoint MAY send the remaining fragments before
   sending a Close frame).

I think it's more about reciprocated Close rather than unsolicited.

Overall I'm thinking of pushing the change, with the disclaimer that in future
it doesn't stop us from accepting a custom queue in the builder. So
1-outstanding write policy could be enforced by the implementation.

Re: RFR JDK-8159039: sun/net/httpclient/hpack/HeaderTableTest.java fails on some locales

[Naoto Sato]

Hi Pavel,

Locale.setDefault() sets the JVM wide default locale, so I'd suggest add 
try/finally block to resume the locale to the previous one.


Naoto

On 6/13/16 3:29 AM, Pavel Rappo wrote:

Hi,

Could you please review the following change for JDK-8159039?

http://cr.openjdk.java.net/~prappo/8159039/webrev.00/

This change contains a fix for 8159039 as well as a number of tiny editorial
changes.

Thanks,
-Pavel

Re: RFR JDK-8159039: sun/net/httpclient/hpack/HeaderTableTest.java fails on some locales

[Chris Hegarty]

On 13/06/16 16:39, Naoto Sato wrote:

Hi Pavel,

Locale.setDefault() sets the JVM wide default locale, so I'd suggest add
try/finally block to resume the locale to the previous one.


I did suggest something like this to Pavel off-list, but then
noticed that the test runs in othervm mode, so won't affect
subsequent tests. But of course it could have other negative
affects on itself, but somewhat limited at least.

-Chris.


Naoto

On 6/13/16 3:29 AM, Pavel Rappo wrote:

Hi,

Could you please review the following change for JDK-8159039?

http://cr.openjdk.java.net/~prappo/8159039/webrev.00/

This change contains a fix for 8159039 as well as a number of tiny
editorial
changes.

Thanks,
-Pavel

Re: RFR JDK-8159039: sun/net/httpclient/hpack/HeaderTableTest.java fails on some locales

[Naoto Sato]

Does that mean each method test invocation on an separate vm, or the 
scope is .java? If it's the latter, I would still suggest doing 
try/finally, otherwise the later test invocations could be affected with 
the FRENCH locale.


Naoto

On 6/13/16 8:52 AM, Chris Hegarty wrote:

On 13/06/16 16:39, Naoto Sato wrote:

Hi Pavel,

Locale.setDefault() sets the JVM wide default locale, so I'd suggest add
try/finally block to resume the locale to the previous one.


I did suggest something like this to Pavel off-list, but then
noticed that the test runs in othervm mode, so won't affect
subsequent tests. But of course it could have other negative
affects on itself, but somewhat limited at least.

-Chris.


Naoto

On 6/13/16 3:29 AM, Pavel Rappo wrote:

Hi,

Could you please review the following change for JDK-8159039?

http://cr.openjdk.java.net/~prappo/8159039/webrev.00/

This change contains a fix for 8159039 as well as a number of tiny
editorial
changes.

Thanks,
-Pavel

RFR 8144008: Setting NO_PROXY on an URLConnection is not complied with

[Vyom Tewari]

Hi All,

Please review the below fix.
Bug   : JDK-8144008 Setting NO_PROXY on an URLConnection is not 
complied with
Webrev : 
http://cr.openjdk.java.net/~vtewari/8144008/webrev0.0/index.html 



Thanks,
Vyom