Re: RFR: 8132359 - JarURLConnection.getJarFile() resource leak when file is not found

[Rob McKenna]

I'm afraid so.

The option of changing the the JarURLConnection spec to indicate what
could happen when the class file is missing was discussed but that will
require some further thought. (and it may not necessarily result in a fix
for this problem, the assumption being that behaviour changes in this
code would be unwelcome)

-Rob

On 25/11/19 13:49, Alan Bateman wrote:
> 
> Daniel's summary is useful but changing URLClassPath doesn't feel right. Are
> you in a hurry to find a solution to this? Just asking as I think I'd prefer
> to see other options explored that fixed it in the protocol handler instead.
> 
> -Alan
> 
> On 25/11/2019 13:31, Rob McKenna wrote:
> > Thanks Daniel,
> > 
> > cc'ing core-libs-dev in case there are any objections.
> > 
> >  -Rob
> > 
> > On 25/11/19 10:47, Daniel Fuchs wrote:
> > > Hi Rob,
> > > 
> > > That looks good to me. I wonder if that should go to corelibs for
> > > review as well.
> > > 
> > > The underlying issue here is that JarURLConnection open both its
> > > jar file and its jar file entry in its connect() method.
> > > However, it delegates the opening of the jar file to the cache,
> > > when uses cache is true.
> > > In this latter case, if opening the entry throws an exception,
> > > it can't close the jar file because the file might have come from
> > > the cache and it might still be used by someone else.
> > > But because getJarFile() calls connect() then there's no way
> > > to retrieve the jar file through that (failed) connection.
> > > 
> > > I agree that fixing the issue in URLClassPath is probably the
> > > less risky.
> > > 
> > > I see that your test caters for all possible setting of uses cache
> > > and combination of success/failure when opening the jar entry,
> > > so this give me confidence that the fix is working as intended.
> > > 
> > > best regards,
> > > 
> > > -- daniel
> > > 
> > > 
> > > On 24/11/2019 15:33, Rob McKenna wrote:
> > > > Hi folks,
> > > > 
> > > > If a FileNotFoundException is thrown when attempting to load a class
> > > > from a jar file, a reference to the open JarFile remains even after the
> > > > loader is closed. The test in the webrev demonstrates the problem by
> > > > attempting to delete the JarFile after attempting a class load.
> > > > 
> > > > The deletion will fail as the JarFile is still in use (i.e. open)
> > > > despite the fact that the loader has been closed.
> > > > 
> > > > Note: the behaviour depends on the URLConnections useCaches setting so
> > > > the test cycles through the combinations. (this helpfully found a 
> > > > problem
> > > > with an earlier fix attempt)
> > > > 
> > > > bug: https://bugs.openjdk.java.net/browse/JDK-8132359
> > > > webrev: http://cr.openjdk.java.net/~robm/8132359/webrev.01/
> > > > 
> > > > Thanks,
> > > > 
> > > >   -Rob
> > > > 
> 

RFR: 8132359 - JarURLConnection.getJarFile() resource leak when file is not found

[Rob McKenna]

Thanks Daniel,

cc'ing core-libs-dev in case there are any objections.

-Rob

On 25/11/19 10:47, Daniel Fuchs wrote:
> Hi Rob,
> 
> That looks good to me. I wonder if that should go to corelibs for
> review as well.
> 
> The underlying issue here is that JarURLConnection open both its
> jar file and its jar file entry in its connect() method.
> However, it delegates the opening of the jar file to the cache,
> when uses cache is true.
> In this latter case, if opening the entry throws an exception,
> it can't close the jar file because the file might have come from
> the cache and it might still be used by someone else.
> But because getJarFile() calls connect() then there's no way
> to retrieve the jar file through that (failed) connection.
> 
> I agree that fixing the issue in URLClassPath is probably the
> less risky.
> 
> I see that your test caters for all possible setting of uses cache
> and combination of success/failure when opening the jar entry,
> so this give me confidence that the fix is working as intended.
> 
> best regards,
> 
> -- daniel
> 
> 
> On 24/11/2019 15:33, Rob McKenna wrote:
> > Hi folks,
> > 
> > If a FileNotFoundException is thrown when attempting to load a class
> > from a jar file, a reference to the open JarFile remains even after the
> > loader is closed. The test in the webrev demonstrates the problem by
> > attempting to delete the JarFile after attempting a class load.
> > 
> > The deletion will fail as the JarFile is still in use (i.e. open)
> > despite the fact that the loader has been closed.
> > 
> > Note: the behaviour depends on the URLConnections useCaches setting so
> > the test cycles through the combinations. (this helpfully found a problem
> > with an earlier fix attempt)
> > 
> > bug: https://bugs.openjdk.java.net/browse/JDK-8132359
> > webrev: http://cr.openjdk.java.net/~robm/8132359/webrev.01/
> > 
> > Thanks,
> > 
> >  -Rob
> > 
> 

RFR: 8132359 - JarURLConnection.getJarFile() resource leak when file is not found

[Rob McKenna]

Hi folks,

If a FileNotFoundException is thrown when attempting to load a class
from a jar file, a reference to the open JarFile remains even after the
loader is closed. The test in the webrev demonstrates the problem by
attempting to delete the JarFile after attempting a class load.

The deletion will fail as the JarFile is still in use (i.e. open)
despite the fact that the loader has been closed.

Note: the behaviour depends on the URLConnections useCaches setting so
the test cycles through the combinations. (this helpfully found a problem
with an earlier fix attempt)

bug: https://bugs.openjdk.java.net/browse/JDK-8132359
webrev: http://cr.openjdk.java.net/~robm/8132359/webrev.01/

Thanks,

-Rob

Re: [RFR] 8219640: UnresolvedAddressException not thrown for incorrect hostname

[Rob McKenna]

Thanks for the pointer to the passing host Alan. With that I was able to
track down:

https://sourceware.org/git/?p=glibc.git;a=commitdiff;h=108bc4049f8ae82710aec26a92ffdb4b439c83fd

So it looks like this has recently been fixed in glibc.

-Rob

On 03/09/19 17:47, Alan Bateman wrote:
> On 03/09/2019 17:35, Chris Hegarty wrote:
> > :
> > 
> >  From the same Linux system:
> > 
> > $ ldd --version
> > ldd (Ubuntu GLIBC 2.23-0ubuntu11) 2.23
> > Copyright (C) 2016 Free Software Foundation, Inc.
> > This is free software; see the source for copying conditions.  There is NO
> > warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
> > Written by Roland McGrath and Ulrich Drepper.
> > 
> I just checked a system with OEL 7.6 and glibc 2.17 and it doesn't
> duplicate. Would be interesting to know if this issue is tracked in
> somewhere so we have a more complete picture on the issue.
> 
> -Alan

[RFR] 8219640: UnresolvedAddressException not thrown for incorrect hostname

[Rob McKenna]

Hi folks,

Assuming the premise of this bug is correct, could I get a review for
the following:

https://bugs.openjdk.java.net/browse/JDK-8219640
http://cr.openjdk.java.net/~robm/8219640/webrev.01/

Windows rejects a lookup for "127.0.0.1 test" where getaddrinfo on
non-Windows platforms resolves to 127.0.0.1. Note: this only happens
when the set of characters before the space corresponds to an IP
address. Both sets of platforms will reject "somehost name".

-Rob

Re: upgrade to jdk6 com.sun.httpserver

[Rob McKenna]

W.r.t. alternatives the HTTP serving landscape on the JVM is rich and
diverse at this point. Projects worth a look include Grizzly, Netty, Jetty,
Tomcat, Undertow, Rapidoid and the many cool frameworks build on top of
these technologies. (e.g.  Jooby, SparkJava, Play to name a few)

-Rob

On 20/02/18 16:15, Ashton Hogan wrote:
>  These items are essential to keeping the server up to date and keeping the 
> code at Oracle clean and up to standard:
> 
> 1. Update to HTTP22. Remove excess threads, only one thread is needed3. 
> Replace handler with a FIFO queue4. Clean up code, ideally with 
> http://elegantobjects.org principles
> If you disagree on any of them please reply with why and what the alternative 
> should be

Re: [RFR] 8184328: JDK8u131 socketRead0 hang at SSL read

[Rob McKenna]

Thanks Xuelei, webrev below:

http://cr.openjdk.java.net/~robm/8184328/webrev.02/

Presumably this won't require a CSR?

-Rob

On 15/09/17 04:38, Xuelei Fan wrote:
> I still prefer to not-depends on socket receiving timeout.  But I'm fine if
> you want to move on with it.
> 
> As we can close the super socket in the current implementation, it implies
> that application can handle it already.  So you may not need the system
> property and 5 times retries.  I think it's fine just call fatal() for the
> first timeout.
> 
> Xuelei
> 
> On 9/15/2017 4:32 PM, Xuelei Fan wrote:
> >On 9/15/2017 8:22 AM, Rob McKenna wrote:
> >>This test calls close directly. (3rd last line in the stack)
> >>
> >>I believe this is the only possible stack (with the new parameter) once
> >>autoclose is set to false. If autoclose is true we'd skip the call to
> >>waitForClose and just go directly to Socket.close() unless I'm mistaken.
> >>
> >I did not find the call to fatal() in the current implementation.  I think
> >you mean you added the call to fatal() in your update so that when
> >timeout, a fatal() will always get called?
> >
> >Thinking about two things:
> >1. application have to set receiving timeout in order to  get receiving
> >timeout.
> >I have a concern about it, as described in other comments.
> >
> >2. can we close the super socket?
> >It is a surprise to me to close super socket even we don't allocate it. It
> >does not feel right to me, but this is the current behavior.  All right, I
> >get your point.
> >
> >Xuelei
> >
> >> -Rob
> >>
> >>On 15/09/17 07:55, Xuelei Fan wrote:
> >>>On 9/15/2017 7:41 AM, Rob McKenna wrote:
> >>>>On 15/09/17 07:07, Xuelei Fan wrote:
> >>>>>On 9/15/2017 7:00 AM, Rob McKenna wrote:
> >>>>>>When we call close() on the SSLSocket that calls close() on the
> >>>>>>underlying java Socket which closes the native socket.
> >>>>>>
> >>>>>Sorry, I did not get the point.  Please see the close()
> >>>>>implementation of
> >>>>>SSLSocket (sun.security.ssl.SSLSocketImpl.close()) about the details.
> >>>>
> >>>>Running my original test against an instrumented 8u-dev produces the
> >>>>following:
> >>>>
> >>>>java.lang.Exception: Stack trace
> >>>>at java.lang.Thread.dumpStack(Thread.java:1336)
> >>>>at java.net.Socket.close(Socket.java:1491)
> >>>>at
> >>>>sun.security.ssl.BaseSSLSocketImpl.close(BaseSSLSocketImpl.java:624)
> >>>>at
> >>>>sun.security.ssl.SSLSocketImpl.closeSocket(SSLSocketImpl.java:1579)
> >>>>at sun.security.ssl.SSLSocketImpl.fatal(SSLSocketImpl.java:1980)
> >>>>at
> >>>>sun.security.ssl.SSLSocketImpl.waitForClose(SSLSocketImpl.java:1793)
> >>>>at
> >>>>sun.security.ssl.SSLSocketImpl.closeSocket(SSLSocketImpl.java:1592)
> >>>>at
> >>>>sun.security.ssl.SSLSocketImpl.closeInternal(SSLSocketImpl.java:1726)
> >>>>at sun.security.ssl.SSLSocketImpl.close(SSLSocketImpl.java:1615)
> >>>>at ssl.SSLClient.close(SSLClient.java:143)
> >>>>at
> >>>>ssl.SocketTimeoutCloseHang.ReadHang.testSSLServer(ReadHang.java:77)
> >>>>
> >>>It is just one possible stacks of many.  There are cases where no
> >>>fatal()
> >>>get called.  For example, application call close() method directly.
> >>>
> >>>Xuelei

Re: [RFR] 8184328: JDK8u131 socketRead0 hang at SSL read

[Rob McKenna]

This test calls close directly. (3rd last line in the stack)

I believe this is the only possible stack (with the new parameter) once
autoclose is set to false. If autoclose is true we'd skip the call to
waitForClose and just go directly to Socket.close() unless I'm mistaken.

-Rob

On 15/09/17 07:55, Xuelei Fan wrote:
> On 9/15/2017 7:41 AM, Rob McKenna wrote:
> >On 15/09/17 07:07, Xuelei Fan wrote:
> >>On 9/15/2017 7:00 AM, Rob McKenna wrote:
> >>>When we call close() on the SSLSocket that calls close() on the
> >>>underlying java Socket which closes the native socket.
> >>>
> >>Sorry, I did not get the point.  Please see the close() implementation of
> >>SSLSocket (sun.security.ssl.SSLSocketImpl.close()) about the details.
> >
> >Running my original test against an instrumented 8u-dev produces the
> >following:
> >
> >java.lang.Exception: Stack trace
> > at java.lang.Thread.dumpStack(Thread.java:1336)
> > at java.net.Socket.close(Socket.java:1491)
> > at sun.security.ssl.BaseSSLSocketImpl.close(BaseSSLSocketImpl.java:624)
> > at sun.security.ssl.SSLSocketImpl.closeSocket(SSLSocketImpl.java:1579)
> > at sun.security.ssl.SSLSocketImpl.fatal(SSLSocketImpl.java:1980)
> > at sun.security.ssl.SSLSocketImpl.waitForClose(SSLSocketImpl.java:1793)
> > at sun.security.ssl.SSLSocketImpl.closeSocket(SSLSocketImpl.java:1592)
> > at sun.security.ssl.SSLSocketImpl.closeInternal(SSLSocketImpl.java:1726)
> > at sun.security.ssl.SSLSocketImpl.close(SSLSocketImpl.java:1615)
> > at ssl.SSLClient.close(SSLClient.java:143)
> > at ssl.SocketTimeoutCloseHang.ReadHang.testSSLServer(ReadHang.java:77)
> >
> It is just one possible stacks of many.  There are cases where no fatal()
> get called.  For example, application call close() method directly.
> 
> Xuelei

Re: [RFR] 8184328: JDK8u131 socketRead0 hang at SSL read

[Rob McKenna]

Ah, right! This is the part I was missing.

So my fix is intended to address this specific circumstance only (where
we get caught in the while loop in waitForClose() indefinitely despite
having set a read timeout). In this situation it would be reasonable for
somebody to set a read timeout in the hope that the close() call would
not hang indefinitely. Unfortunately due to the while loop in
waitForClose it does regardless. (hence my assertion that applications
already have to set a read timeout to attempt to avoid this situation)

So you're suggesting that we take the read timeout out of the close
mechanics completely and replace it with something more appropriate?

Given that closing an SSLSocket requires a read operation in order to
receive the close_notify though, I'm not sure how to accomplish that.
Can you go into a little bit more detail?

-Rob

On 15/09/17 07:57, Xuelei Fan wrote:
> On 9/15/2017 7:44 AM, Rob McKenna wrote:
> >Perhaps I'm misunderstanding you here. Can you illustrate this a bit
> >further?
> >
> The basic point is simple: removing the closing blocking even receiving
> timeout is not set.
> 
> >Applications already have to set a read timeout
> I did not get the point.  Applications don't have to set a read timeout.
> 
> Xuelei
> 
> >, my proposal doesn't
> >alter this fact. (i.e. if the read timeout isn't set applications which
> >call close could potentially get stuck in readReply indefinitely)
> >
> >     -Rob
> >
> >On 15/09/17 07:23, Xuelei Fan wrote:
> >>On 9/15/2017 7:07 AM, Rob McKenna wrote:
> >>>But they are inextricably linked regardless.
> >>>
> >>>When we close an SSLSocket it performs a readReply which is subject to
> >>>the read timeout. So if no read timeout is specified, the call to
> >>>readReply will hang indefinitely.
> >>That's one of what I worried about.  Applications have to set receiving
> >>timeout in your proposal.  I don't want closing timeout binding to receiving
> >>timeout.  It's doable and the impact is minimal.
> >>
> >>Xuelei
> >>
> >>>If a read timeout is specified we
> >>>would need to maintain two separate timeouts and take each into account
> >>>when polling.
> >>>
> >>>What you are suggesting would effectively necessitate a reimplementation
> >>>of the close mechanics discarding the read timeout completely. (which
> >>>would be a significant enough change in terms of compatibility)
> >>>
> >>> -Rob
> >>>
> >>>On 13/09/17 03:56, Xuelei Fan wrote:
> >>>>On 9/13/2017 8:52 AM, Rob McKenna wrote:
> >>>>>W.r.t. a separate timeout, the underlying mechanics of a close already
> >>>>>depend on the readTimeout in this situation.
> >>>>That's a concerns of mine.  In order to work for your countermeasure,
> >>>>applications have to set receiving timeout, and take care of the closing
> >>>>timeout when evaluate what's a right timeout value.  The mixing could be
> >>>>misleading and not easy to use.
> >>>>
> >>>>Xuelei

Re: [RFR] 8184328: JDK8u131 socketRead0 hang at SSL read

[Rob McKenna]

On 15/09/17 07:32, Xuelei Fan wrote:
> On 9/15/2017 7:16 AM, Rob McKenna wrote:
> >On 13/09/17 03:52, Xuelei Fan wrote:
> >>
> >>
> >>On 9/13/2017 8:52 AM, Rob McKenna wrote:
> >>>Hi Xuelei,
> >>>
> >>>This behaviour is already exposed via the autoclose boolean in:
> >>>
> >>>https://docs.oracle.com/javase/8/docs/api/javax/net/ssl/SSLSocketFactory.html#createSocket-java.net.Socket-java.io.InputStream-boolean-
> >>>
> >>I did not get the point.  What do you mean by this behavior is already
> >>exposed?
> >
> >In SSLSocketImpl.closeSocket() waitForClose is only called if autoclose
> >is true. If not the SSLSocket simply calls super.close().
> >
> Did you get something different?  I think waitForClose is only called if
> autoclose is false.
> 
> No matter the autoclose is true or false, I'm not sure what do you mean by
> this behavior is already exposed.  Can you describe more about the point.
> 

Ack, yes, sorry, I got that backwards. If you set autoclose to true
SSLSocket.close() will skip waitForClose() and simply call
super.close(). When I say this behaviour is already exposed I am
referring to the call to super.close(). (which is effectively what this
fix does after the specified number of attempts via the call to fatal)

-Rob

> >>
> >>>My position would be that allowing 5 retries allows us to say with some
> >>>confidence that we're not going to get a close_notify from the server.
> >>You have more chance to get the close_notify, but it does not mean you can
> >>always get the close_notify in 5 retries.  When you cannot get it, something
> >>bad happens.
> >
> >No, the property would need to be tuned to suit the networking
> >environment in which the application is deployed. Much the same as a
> >timeout would be.
> >
> >>
> >>>If this is the case I think its reasonable to close the connection.
> >>>
> >>>W.r.t. a separate timeout, the underlying mechanics of a close already
> >>>depend on the readTimeout in this situation. (waiting on a close_notify
> >>>requires performing a read so the read timeout makes sense in this
> >>>context) I'm happy to alter that but I think that the combination of
> >>>a timeout and a retry count is straightforward and lower impact.
> >>>
> >>>In my opinion the default behaviour of potentially hanging indefinitely
> >>>is worse than the alternative here. (bearing in mind that we are closing
> >>>the underlying socket)
> >>>
> >>I did not get the point, are we really closing the underlying socket (or the
> >>layered ssl connection?) for the context of you update?
> >
> >We're calling fatal which calls closeSocket which in turn calls
> >super.close(). (this calls Socket.close() via BaseSSLSocketImpl /
> >SSLSocket) As noted in an earlier reply, this will close the
> >underlying native socket. (I'll perform more testing to verify this)
> >
> When the fatal get called?  I may miss something.  Could you describe the
> scenarios in more details?
> 
> Xuelei
> 
> > -Rob
> >
> >>
> >>Xuelei
> >>
> >>>I'll file a CSR as soon as we settle on the direction this fix will
> >>>take.
> >>>
> >>> -Rob
> >>>
> >>>On 13/09/17 05:52, Xuelei Fan wrote:
> >>>>In theory, there are intermittent compatibility problems as this update 
> >>>>may
> >>>>not close the SSL connection over the existing socket layer gracefully, 
> >>>>even
> >>>>for high speed networking environments, while the underlying socket is
> >>>>alive.  The impact could be serious in some environment.
> >>>>
> >>>>For safe, I may suggest turn this countermeasure off by default.  And
> >>>>providing options to turn on this countermeasure:
> >>>>1. Close the SSL connection gracefully by default; or
> >>>>2. Close the SSL connection after a timeout.
> >>>>
> >>>>It's hardly to say 5 times receiving timeout is better/safer than timeout
> >>>>once in this context.  As you have already had a system property to 
> >>>>control,
> >>>>you may be able to use options other than the customized socket receiving
> >>>>timeout, so that the closing timeout is not mixed/confused/dependent 
> >>>>on/with
> >>>>the receiving timeout.
> >>>>
> >>>

Re: [RFR] 8184328: JDK8u131 socketRead0 hang at SSL read

[Rob McKenna]

Perhaps I'm misunderstanding you here. Can you illustrate this a bit
further?

Applications already have to set a read timeout, my proposal doesn't
alter this fact. (i.e. if the read timeout isn't set applications which
call close could potentially get stuck in readReply indefinitely)

-Rob

On 15/09/17 07:23, Xuelei Fan wrote:
> On 9/15/2017 7:07 AM, Rob McKenna wrote:
> >But they are inextricably linked regardless.
> >
> >When we close an SSLSocket it performs a readReply which is subject to
> >the read timeout. So if no read timeout is specified, the call to
> >readReply will hang indefinitely.
> That's one of what I worried about.  Applications have to set receiving
> timeout in your proposal.  I don't want closing timeout binding to receiving
> timeout.  It's doable and the impact is minimal.
> 
> Xuelei
> 
> >If a read timeout is specified we
> >would need to maintain two separate timeouts and take each into account
> >when polling.
> >
> >What you are suggesting would effectively necessitate a reimplementation
> >of the close mechanics discarding the read timeout completely. (which
> >would be a significant enough change in terms of compatibility)
> >
> > -Rob
> >
> >On 13/09/17 03:56, Xuelei Fan wrote:
> >>On 9/13/2017 8:52 AM, Rob McKenna wrote:
> >>>W.r.t. a separate timeout, the underlying mechanics of a close already
> >>>depend on the readTimeout in this situation.
> >>That's a concerns of mine.  In order to work for your countermeasure,
> >>applications have to set receiving timeout, and take care of the closing
> >>timeout when evaluate what's a right timeout value.  The mixing could be
> >>misleading and not easy to use.
> >>
> >>Xuelei

Re: [RFR] 8184328: JDK8u131 socketRead0 hang at SSL read

[Rob McKenna]

On 15/09/17 07:07, Xuelei Fan wrote:
> On 9/15/2017 7:00 AM, Rob McKenna wrote:
> >When we call close() on the SSLSocket that calls close() on the
> >underlying java Socket which closes the native socket.
> >
> Sorry, I did not get the point.  Please see the close() implementation of
> SSLSocket (sun.security.ssl.SSLSocketImpl.close()) about the details.

Running my original test against an instrumented 8u-dev produces the
following:

java.lang.Exception: Stack trace
at java.lang.Thread.dumpStack(Thread.java:1336)
at java.net.Socket.close(Socket.java:1491)
at sun.security.ssl.BaseSSLSocketImpl.close(BaseSSLSocketImpl.java:624)
at sun.security.ssl.SSLSocketImpl.closeSocket(SSLSocketImpl.java:1579)
at sun.security.ssl.SSLSocketImpl.fatal(SSLSocketImpl.java:1980)
at sun.security.ssl.SSLSocketImpl.waitForClose(SSLSocketImpl.java:1793)
at sun.security.ssl.SSLSocketImpl.closeSocket(SSLSocketImpl.java:1592)
at sun.security.ssl.SSLSocketImpl.closeInternal(SSLSocketImpl.java:1726)
at sun.security.ssl.SSLSocketImpl.close(SSLSocketImpl.java:1615)
at ssl.SSLClient.close(SSLClient.java:143)
at ssl.SocketTimeoutCloseHang.ReadHang.testSSLServer(ReadHang.java:77)

-Rob

> 
> Xuelei
> 
> > -Rob
> >
> >On 13/09/17 04:09, Xuelei Fan wrote:
> >>It's a little bit complicated for layered SSL connections.  Application can
> >>build a SSL connection on existing socket (we call it layered SSL
> >>connections).  The problem scenarios make look like:
> >>1. open a socket for applications.
> >>2. established a SSL connection on the existing socket.
> >>3. close the SSL connection, but leaving data in the socket.
> >>4. establish another SSL connection on the socket, as the existing data in
> >>the socket, the connection cannot be established.
> >>5. establish another app connection on the socket, as the existing data in
> >>the socket, the connection cannot be established.
> >>
> >>
> >>Timeout happens even on very high speed network. If a timeout happens and
> >>the SSL connection is not closed gracefully, and then the following
> >>applications breaks.  IMHO, we need to take care of the case.
> >>
> >>Xuelei
> >>
> >>On 9/13/2017 1:06 PM, Chris Hegarty wrote:
> >>>Xuelei,
> >>>
> >>>Without diving deeper into this issue, Rob’s suggested approach seems 
> >>>reasonable to me, and better than existing out-of-the-box behaviour. I’m 
> >>>not sure what issues you are thinking of, with using the read timeout in 
> >>>combination with a retry mechanism, in this manner? If the network is so 
> >>>slow, surely there will be other issues with connecting and reading, why 
> >>>is closing any different.
> >>>
> >>>-Chris.
> >>>
> >>>>On 13 Sep 2017, at 16:52, Rob McKenna <rob.mcke...@oracle.com> wrote:
> >>>>
> >>>>Hi Xuelei,
> >>>>
> >>>>This behaviour is already exposed via the autoclose boolean in:
> >>>>
> >>>>https://docs.oracle.com/javase/8/docs/api/javax/net/ssl/SSLSocketFactory.html#createSocket-java.net.Socket-java.io.InputStream-boolean-
> >>>>
> >>>>My position would be that allowing 5 retries allows us to say with some
> >>>>confidence that we're not going to get a close_notify from the server.
> >>>>If this is the case I think its reasonable to close the connection.
> >>>>
> >>>>W.r.t. a separate timeout, the underlying mechanics of a close already
> >>>>depend on the readTimeout in this situation. (waiting on a close_notify
> >>>>requires performing a read so the read timeout makes sense in this
> >>>>context) I'm happy to alter that but I think that the combination of
> >>>>a timeout and a retry count is straightforward and lower impact.
> >>>>
> >>>>In my opinion the default behaviour of potentially hanging indefinitely
> >>>>is worse than the alternative here. (bearing in mind that we are closing
> >>>>the underlying socket)
> >>>>
> >>>>I'll file a CSR as soon as we settle on the direction this fix will
> >>>>take.
> >>>>
> >>>>-Rob
> >>>>
> >>>>On 13/09/17 05:52, Xuelei Fan wrote:
> >>>>>In theory, there are intermittent compatibility problems as this update 
> >>>>>may
> >>>>>not close the SSL c

Re: [RFR] 8184328: JDK8u131 socketRead0 hang at SSL read

[Rob McKenna]

On 13/09/17 03:52, Xuelei Fan wrote:
> 
> 
> On 9/13/2017 8:52 AM, Rob McKenna wrote:
> >Hi Xuelei,
> >
> >This behaviour is already exposed via the autoclose boolean in:
> >
> >https://docs.oracle.com/javase/8/docs/api/javax/net/ssl/SSLSocketFactory.html#createSocket-java.net.Socket-java.io.InputStream-boolean-
> >
> I did not get the point.  What do you mean by this behavior is already
> exposed?

In SSLSocketImpl.closeSocket() waitForClose is only called if autoclose
is true. If not the SSLSocket simply calls super.close().

> 
> >My position would be that allowing 5 retries allows us to say with some
> >confidence that we're not going to get a close_notify from the server.
> You have more chance to get the close_notify, but it does not mean you can
> always get the close_notify in 5 retries.  When you cannot get it, something
> bad happens.

No, the property would need to be tuned to suit the networking
environment in which the application is deployed. Much the same as a
timeout would be.

> 
> >If this is the case I think its reasonable to close the connection.
> >
> >W.r.t. a separate timeout, the underlying mechanics of a close already
> >depend on the readTimeout in this situation. (waiting on a close_notify
> >requires performing a read so the read timeout makes sense in this
> >context) I'm happy to alter that but I think that the combination of
> >a timeout and a retry count is straightforward and lower impact.
> >
> >In my opinion the default behaviour of potentially hanging indefinitely
> >is worse than the alternative here. (bearing in mind that we are closing
> >the underlying socket)
> >
> I did not get the point, are we really closing the underlying socket (or the
> layered ssl connection?) for the context of you update?

We're calling fatal which calls closeSocket which in turn calls
super.close(). (this calls Socket.close() via BaseSSLSocketImpl /
SSLSocket) As noted in an earlier reply, this will close the
underlying native socket. (I'll perform more testing to verify this)

-Rob

> 
> Xuelei
> 
> >I'll file a CSR as soon as we settle on the direction this fix will
> >take.
> >
> > -Rob
> >
> >On 13/09/17 05:52, Xuelei Fan wrote:
> >>In theory, there are intermittent compatibility problems as this update may
> >>not close the SSL connection over the existing socket layer gracefully, even
> >>for high speed networking environments, while the underlying socket is
> >>alive.  The impact could be serious in some environment.
> >>
> >>For safe, I may suggest turn this countermeasure off by default.  And
> >>providing options to turn on this countermeasure:
> >>1. Close the SSL connection gracefully by default; or
> >>2. Close the SSL connection after a timeout.
> >>
> >>It's hardly to say 5 times receiving timeout is better/safer than timeout
> >>once in this context.  As you have already had a system property to control,
> >>you may be able to use options other than the customized socket receiving
> >>timeout, so that the closing timeout is not mixed/confused/dependent on/with
> >>the receiving timeout.
> >>
> >>Put all together:
> >>1. define a closing timeout, for example "jdk.tls.waitForClose".
> >>2. the property default value is zero, no behavior changes.
> >>3. applications can set positive milliseconds value for the property. The
> >>SSL connection will be closed in the set milliseconds (or about the maximum
> >>value between SO_TIMEOUT and closing timeout), the connection is not grant
> >>to be gracefully.
> >>
> >>What do you think?
> >>
> >>BTW, please file a CSR as this update is introducing an external system
> >>property.
> >>
> >>Thanks,
> >>Xuelei
> >>
> >>On 9/11/2017 3:29 PM, Rob McKenna wrote:
> >>>Hi folks,
> >>>
> >>>In high latency environments a client SSLSocket with autoClose set to false
> >>>can hang indefinitely if it does not correctly recieve a close_notify
> >>>from the server.
> >>>
> >>>In order to rectify this situation I would like to suggest that we
> >>>implement an integer JDK property (jdk.tls.closeRetries) which instructs
> >>>waitForClose to attempt the close no more times than the value of the
> >>>property. I would also suggest that 5 is a reasonable default.
> >>>
> >>>Note: each attempt times out based on the value of
> >>>Socket.setSoTimeout(int timeout).
> >>>
> >>>Also, the behaviour here is similar to that of waitForClose() when
> >>>autoClose is set to true, less the retries.
> >>>
> >>>http://cr.openjdk.java.net/~robm/8184328/webrev.01/
> >>>
> >>> -Rob
> >>>

Re: [RFR] 8184328: JDK8u131 socketRead0 hang at SSL read

[Rob McKenna]

But they are inextricably linked regardless.

When we close an SSLSocket it performs a readReply which is subject to
the read timeout. So if no read timeout is specified, the call to
readReply will hang indefinitely. If a read timeout is specified we
would need to maintain two separate timeouts and take each into account
when polling.

What you are suggesting would effectively necessitate a reimplementation
of the close mechanics discarding the read timeout completely. (which
would be a significant enough change in terms of compatibility)

-Rob

On 13/09/17 03:56, Xuelei Fan wrote:
> On 9/13/2017 8:52 AM, Rob McKenna wrote:
> >W.r.t. a separate timeout, the underlying mechanics of a close already
> >depend on the readTimeout in this situation.
> That's a concerns of mine.  In order to work for your countermeasure,
> applications have to set receiving timeout, and take care of the closing
> timeout when evaluate what's a right timeout value.  The mixing could be
> misleading and not easy to use.
> 
> Xuelei

Re: [RFR] 8184328: JDK8u131 socketRead0 hang at SSL read

[Rob McKenna]

When we call close() on the SSLSocket that calls close() on the
underlying java Socket which closes the native socket.

-Rob

On 13/09/17 04:09, Xuelei Fan wrote:
> It's a little bit complicated for layered SSL connections.  Application can
> build a SSL connection on existing socket (we call it layered SSL
> connections).  The problem scenarios make look like:
> 1. open a socket for applications.
> 2. established a SSL connection on the existing socket.
> 3. close the SSL connection, but leaving data in the socket.
> 4. establish another SSL connection on the socket, as the existing data in
> the socket, the connection cannot be established.
> 5. establish another app connection on the socket, as the existing data in
> the socket, the connection cannot be established.
> 
> 
> Timeout happens even on very high speed network. If a timeout happens and
> the SSL connection is not closed gracefully, and then the following
> applications breaks.  IMHO, we need to take care of the case.
> 
> Xuelei
> 
> On 9/13/2017 1:06 PM, Chris Hegarty wrote:
> >Xuelei,
> >
> >Without diving deeper into this issue, Rob’s suggested approach seems 
> >reasonable to me, and better than existing out-of-the-box behaviour. I’m not 
> >sure what issues you are thinking of, with using the read timeout in 
> >combination with a retry mechanism, in this manner? If the network is so 
> >slow, surely there will be other issues with connecting and reading, why is 
> >closing any different.
> >
> >-Chris.
> >
> >>On 13 Sep 2017, at 16:52, Rob McKenna <rob.mcke...@oracle.com> wrote:
> >>
> >>Hi Xuelei,
> >>
> >>This behaviour is already exposed via the autoclose boolean in:
> >>
> >>https://docs.oracle.com/javase/8/docs/api/javax/net/ssl/SSLSocketFactory.html#createSocket-java.net.Socket-java.io.InputStream-boolean-
> >>
> >>My position would be that allowing 5 retries allows us to say with some
> >>confidence that we're not going to get a close_notify from the server.
> >>If this is the case I think its reasonable to close the connection.
> >>
> >>W.r.t. a separate timeout, the underlying mechanics of a close already
> >>depend on the readTimeout in this situation. (waiting on a close_notify
> >>requires performing a read so the read timeout makes sense in this
> >>context) I'm happy to alter that but I think that the combination of
> >>a timeout and a retry count is straightforward and lower impact.
> >>
> >>In my opinion the default behaviour of potentially hanging indefinitely
> >>is worse than the alternative here. (bearing in mind that we are closing
> >>the underlying socket)
> >>
> >>I'll file a CSR as soon as we settle on the direction this fix will
> >>take.
> >>
> >>-Rob
> >>
> >>On 13/09/17 05:52, Xuelei Fan wrote:
> >>>In theory, there are intermittent compatibility problems as this update may
> >>>not close the SSL connection over the existing socket layer gracefully, 
> >>>even
> >>>for high speed networking environments, while the underlying socket is
> >>>alive.  The impact could be serious in some environment.
> >>>
> >>>For safe, I may suggest turn this countermeasure off by default.  And
> >>>providing options to turn on this countermeasure:
> >>>1. Close the SSL connection gracefully by default; or
> >>>2. Close the SSL connection after a timeout.
> >>>
> >>>It's hardly to say 5 times receiving timeout is better/safer than timeout
> >>>once in this context.  As you have already had a system property to 
> >>>control,
> >>>you may be able to use options other than the customized socket receiving
> >>>timeout, so that the closing timeout is not mixed/confused/dependent 
> >>>on/with
> >>>the receiving timeout.
> >>>
> >>>Put all together:
> >>>1. define a closing timeout, for example "jdk.tls.waitForClose".
> >>>2. the property default value is zero, no behavior changes.
> >>>3. applications can set positive milliseconds value for the property. The
> >>>SSL connection will be closed in the set milliseconds (or about the maximum
> >>>value between SO_TIMEOUT and closing timeout), the connection is not grant
> >>>to be gracefully.
> >>>
> >>>What do you think?
> >>>
> >>>BTW, please file a CSR as this update is introducing an external system
> >>&

Re: [RFR] 8184328: JDK8u131 socketRead0 hang at SSL read

[Rob McKenna]

Hi Xuelei,

This behaviour is already exposed via the autoclose boolean in:

https://docs.oracle.com/javase/8/docs/api/javax/net/ssl/SSLSocketFactory.html#createSocket-java.net.Socket-java.io.InputStream-boolean-

My position would be that allowing 5 retries allows us to say with some
confidence that we're not going to get a close_notify from the server.
If this is the case I think its reasonable to close the connection.

W.r.t. a separate timeout, the underlying mechanics of a close already
depend on the readTimeout in this situation. (waiting on a close_notify
requires performing a read so the read timeout makes sense in this
context) I'm happy to alter that but I think that the combination of
a timeout and a retry count is straightforward and lower impact.

In my opinion the default behaviour of potentially hanging indefinitely
is worse than the alternative here. (bearing in mind that we are closing
the underlying socket)

I'll file a CSR as soon as we settle on the direction this fix will
take.

-Rob

On 13/09/17 05:52, Xuelei Fan wrote:
> In theory, there are intermittent compatibility problems as this update may
> not close the SSL connection over the existing socket layer gracefully, even
> for high speed networking environments, while the underlying socket is
> alive.  The impact could be serious in some environment.
> 
> For safe, I may suggest turn this countermeasure off by default.  And
> providing options to turn on this countermeasure:
> 1. Close the SSL connection gracefully by default; or
> 2. Close the SSL connection after a timeout.
> 
> It's hardly to say 5 times receiving timeout is better/safer than timeout
> once in this context.  As you have already had a system property to control,
> you may be able to use options other than the customized socket receiving
> timeout, so that the closing timeout is not mixed/confused/dependent on/with
> the receiving timeout.
> 
> Put all together:
> 1. define a closing timeout, for example "jdk.tls.waitForClose".
> 2. the property default value is zero, no behavior changes.
> 3. applications can set positive milliseconds value for the property. The
> SSL connection will be closed in the set milliseconds (or about the maximum
> value between SO_TIMEOUT and closing timeout), the connection is not grant
> to be gracefully.
> 
> What do you think?
> 
> BTW, please file a CSR as this update is introducing an external system
> property.
> 
> Thanks,
> Xuelei
> 
> On 9/11/2017 3:29 PM, Rob McKenna wrote:
> >Hi folks,
> >
> >In high latency environments a client SSLSocket with autoClose set to false
> >can hang indefinitely if it does not correctly recieve a close_notify
> >from the server.
> >
> >In order to rectify this situation I would like to suggest that we
> >implement an integer JDK property (jdk.tls.closeRetries) which instructs
> >waitForClose to attempt the close no more times than the value of the
> >property. I would also suggest that 5 is a reasonable default.
> >
> >Note: each attempt times out based on the value of
> >Socket.setSoTimeout(int timeout).
> >
> >Also, the behaviour here is similar to that of waitForClose() when
> >autoClose is set to true, less the retries.
> >
> >http://cr.openjdk.java.net/~robm/8184328/webrev.01/
> >
> > -Rob
> >

[RFR] 8184328: JDK8u131 socketRead0 hang at SSL read

[Rob McKenna]

Hi folks,

In high latency environments a client SSLSocket with autoClose set to false
can hang indefinitely if it does not correctly recieve a close_notify
from the server.

In order to rectify this situation I would like to suggest that we
implement an integer JDK property (jdk.tls.closeRetries) which instructs
waitForClose to attempt the close no more times than the value of the
property. I would also suggest that 5 is a reasonable default.

Note: each attempt times out based on the value of
Socket.setSoTimeout(int timeout).

Also, the behaviour here is similar to that of waitForClose() when
autoClose is set to true, less the retries.

http://cr.openjdk.java.net/~robm/8184328/webrev.01/

-Rob

RFR - 8166747: Add invalid network / computer name cases to isReachable known failure switch

[Rob McKenna]

Hi folks,

Looking for a review of this simple addition to Inet4AddressImpl.c on Windows. 
As per the bug report:

In the ping4() call in Inet4AddressImpl.c on Windows there is a switch 
statement containing failure codes for IcmpSendEcho which correspond to well 
known and expected failures for this call when a host is not reachable. In 
these cases ping4() simply returns false as opposed to throwing an exception.

Prior releases of the JDK would return false when using the tcp ping method 
where we currently throw an exception with the ERROR_INVALID_COMPUTERNAME 
(Windows error code 1210) or ERROR_INVALID_NETNAME (1214) errors. We should add 
these cases to the switch statement for compatibility purposes.

http://cr.openjdk.java.net/~robm/8166747/webrev.01/

-Rob

Re: RFR - 8159410: InetAddress.isReachable returns true for non existing IP addresses

[Rob McKenna]

Thanks folks,

I've been running some testing here and noticed that IP_REQ_TIMED_OUT can also 
be returned from IcmpSendEcho (as opposed to only being an error code in an 
ICMP_ECHO_REPLY)

Updated webrev here:

http://cr.openjdk.java.net/~robm/8159410/webrev.03/

-Rob

On 22/09/16 06:12, Chris Hegarty wrote:
> 
> > On 22 Sep 2016, at 18:04, Mark Sheppard <mark.shepp...@oracle.com> wrote:
> > 
> > 
> > it is good that you added  the additional error code, "cover all bases", as 
> > they say.
> > In any case your exception handling will inform if  something has been 
> > missed, should it occur.
> > So at the risk of triggering another MS curiosity, the changes look fine
> 
> +1 
> 
> -Chris.
> 
> > regards
> > Mark
> > 
> > On 21/09/2016 19:45, Rob McKenna wrote:
> >> The link would be handy:
> >> 
> >> http://cr.openjdk.java.net/~robm/8159410/webrev.02/
> >> 
> >>-Rob
> >> 
> >> On 21/09/16 07:44, Rob McKenna wrote:
> >>> I've updated the webrev here with the copyright year (thanks Christoph) 
> >>> and extra error codes. I overlooked the codes from the old implementation 
> >>> of tcp_ping4 above this code. These are winsock error codes which I would 
> >>> expect IcmpSendEcho to use, but in our testing it actually returned the 
> >>> system error codes in at least one situation:
> >>> 
> >>> https://msdn.microsoft.com/en-gb/library/windows/desktop/ms740668%28v=vs.85%29.aspx
> >>> https://msdn.microsoft.com/en-us/library/windows/desktop/ms681383%28v=vs.85%29.aspx
> >>> 
> >>>   -Rob
> >>> 
> >>> On 21/09/16 06:32, Seán Coffey wrote:
> >>>> spotted an interesting blog on the MSDN timeout issue here :
> >>>> https://www.frameflow.com/ping-utility-flaw-in-windows-api-creating-false-timeouts/
> >>>> 
> >>>> Regards,
> >>>> Sean.
> >>>> 
> >>>> On 21/09/16 17:42, Mark Sheppard wrote:
> >>>>> the IcmpSendEcho series of calls come with some idiosyncrasies in that
> >>>>> there is a minimum timeout that they can handle
> >>>>> think it is about 1000msecs. isReachable can specify a finer grained
> >>>>> timeout hence the need for timeout check
> >>>>> 
> >>>>> regards
> >>>>> Mark
> >>>>> 
> >>>>> On 21/09/2016 17:18, Vyom Tewari wrote:
> >>>>>> Hi Rob,
> >>>>>> 
> >>>>>> Do you really think this extra check is required ?
> >>>>>> 
> >>>>>> if (pEchoReply->Status == IP_SUCCESS
> >>>>>> + && (int)pEchoReply->RoundTripTime <= timeout) I did not found any
> >>>>>> doc(MSDN) which explains this. I think in case of IP_SUCCESS
> >>>>>> "RoundTripTime" is always less than timeout. I think similar changes is
> >>>>>> required in Inet6Address.c as well ? Thanks, Vyom
> >>>>>> 
> >>>>>> On Wednesday 21 September 2016 08:46 PM, Rob McKenna wrote:
> >>>>>>> Hi folks,
> >>>>>>> 
> >>>>>>> I'd like to fix a bug caused by an incorrect assumption. The 
> >>>>>>> IcmpSendEcho* calls can actually return a similar set of errors 
> >>>>>>> regardless of whether the call itself failed or succeeded. This 
> >>>>>>> change checks that both the call and the ping were successful. In 
> >>>>>>> addition to that it takes a number of common failure causes into 
> >>>>>>> account before deciding to throw an exception.
> >>>>>>> 
> >>>>>>> http://cr.openjdk.java.net/~robm/8159410/webrev.01/
> >>>>>>> 
> >>>>>>>   -Rob
> > 
> 

Re: RFR - 8159410: InetAddress.isReachable returns true for non existing IP addresses

[Rob McKenna]

The link would be handy:

http://cr.openjdk.java.net/~robm/8159410/webrev.02/

-Rob

On 21/09/16 07:44, Rob McKenna wrote:
> I've updated the webrev here with the copyright year (thanks Christoph) and 
> extra error codes. I overlooked the codes from the old implementation of 
> tcp_ping4 above this code. These are winsock error codes which I would expect 
> IcmpSendEcho to use, but in our testing it actually returned the system error 
> codes in at least one situation:
> 
> https://msdn.microsoft.com/en-gb/library/windows/desktop/ms740668%28v=vs.85%29.aspx
> https://msdn.microsoft.com/en-us/library/windows/desktop/ms681383%28v=vs.85%29.aspx
> 
>   -Rob
> 
> On 21/09/16 06:32, Seán Coffey wrote:
> > spotted an interesting blog on the MSDN timeout issue here :
> > https://www.frameflow.com/ping-utility-flaw-in-windows-api-creating-false-timeouts/
> > 
> > Regards,
> > Sean.
> > 
> > On 21/09/16 17:42, Mark Sheppard wrote:
> > >
> > >the IcmpSendEcho series of calls come with some idiosyncrasies in that
> > >there is a minimum timeout that they can handle
> > >think it is about 1000msecs. isReachable can specify a finer grained
> > >timeout hence the need for timeout check
> > >
> > >regards
> > >Mark
> > >
> > >On 21/09/2016 17:18, Vyom Tewari wrote:
> > >>
> > >>Hi Rob,
> > >>
> > >>Do you really think this extra check is required ?
> > >>
> > >>if (pEchoReply->Status == IP_SUCCESS
> > >>+ && (int)pEchoReply->RoundTripTime <= timeout) I did not found any
> > >>doc(MSDN) which explains this. I think in case of IP_SUCCESS
> > >>"RoundTripTime" is always less than timeout. I think similar changes is
> > >>required in Inet6Address.c as well ? Thanks, Vyom
> > >>
> > >>On Wednesday 21 September 2016 08:46 PM, Rob McKenna wrote:
> > >>>Hi folks,
> > >>>
> > >>>I'd like to fix a bug caused by an incorrect assumption. The 
> > >>>IcmpSendEcho* calls can actually return a similar set of errors 
> > >>>regardless of whether the call itself failed or succeeded. This change 
> > >>>checks that both the call and the ping were successful. In addition to 
> > >>>that it takes a number of common failure causes into account before 
> > >>>deciding to throw an exception.
> > >>>
> > >>>http://cr.openjdk.java.net/~robm/8159410/webrev.01/
> > >>>
> > >>> -Rob
> > >>
> > >
> > 

Re: RFR - 8159410: InetAddress.isReachable returns true for non existing IP addresses

[Rob McKenna]

I've updated the webrev here with the copyright year (thanks Christoph) and 
extra error codes. I overlooked the codes from the old implementation of 
tcp_ping4 above this code. These are winsock error codes which I would expect 
IcmpSendEcho to use, but in our testing it actually returned the system error 
codes in at least one situation:

https://msdn.microsoft.com/en-gb/library/windows/desktop/ms740668%28v=vs.85%29.aspx
https://msdn.microsoft.com/en-us/library/windows/desktop/ms681383%28v=vs.85%29.aspx

-Rob

On 21/09/16 06:32, Seán Coffey wrote:
> spotted an interesting blog on the MSDN timeout issue here :
> https://www.frameflow.com/ping-utility-flaw-in-windows-api-creating-false-timeouts/
> 
> Regards,
> Sean.
> 
> On 21/09/16 17:42, Mark Sheppard wrote:
> >
> >the IcmpSendEcho series of calls come with some idiosyncrasies in that
> >there is a minimum timeout that they can handle
> >think it is about 1000msecs. isReachable can specify a finer grained
> >timeout hence the need for timeout check
> >
> >regards
> >Mark
> >
> >On 21/09/2016 17:18, Vyom Tewari wrote:
> >>
> >>Hi Rob,
> >>
> >>Do you really think this extra check is required ?
> >>
> >>if (pEchoReply->Status == IP_SUCCESS
> >>+ && (int)pEchoReply->RoundTripTime <= timeout) I did not found any
> >>doc(MSDN) which explains this. I think in case of IP_SUCCESS
> >>"RoundTripTime" is always less than timeout. I think similar changes is
> >>required in Inet6Address.c as well ? Thanks, Vyom
> >>
> >>On Wednesday 21 September 2016 08:46 PM, Rob McKenna wrote:
> >>>Hi folks,
> >>>
> >>>I'd like to fix a bug caused by an incorrect assumption. The IcmpSendEcho* 
> >>>calls can actually return a similar set of errors regardless of whether 
> >>>the call itself failed or succeeded. This change checks that both the call 
> >>>and the ping were successful. In addition to that it takes a number of 
> >>>common failure causes into account before deciding to throw an exception.
> >>>
> >>>http://cr.openjdk.java.net/~robm/8159410/webrev.01/
> >>>
> >>>   -Rob
> >>
> >
> 

Re: RFR - 8159410: InetAddress.isReachable returns true for non existing IP addresses

[Rob McKenna]

Yup. To elabourate:

If we set a small timeout for a faraway host with a high ping, IcmpSendEcho can 
report success even if the rtt exceeded the timeout, hence the need for this 
explicit check.

-Rob

On 21/09/16 11:07, Vyom Tewari wrote:
> So InetAddress.isReachable() will return false if the underline API
> IcmpSendEcho return with Status== IP_SUCESS and RoundTripTime > timeout.
> 
> Vyom
> 
> 
> On Wednesday 21 September 2016 10:39 PM, Rob McKenna wrote:
> >Unfortunately the behaviour described is undocumented and was found the hard 
> >way. This part of the code is a necessity though.
> >
> > -Rob
> >
> >On 21/09/16 09:48, Vyom Tewari wrote:
> >>Hi Rob,
> >>
> >>Do you really think this extra check is required ?
> >>
> >>if (pEchoReply->Status == IP_SUCCESS
> >>+ && (int)pEchoReply->RoundTripTime <= timeout) I did not found any
> >>doc(MSDN) which explains this. I think in case of IP_SUCCESS "RoundTripTime"
> >>is always less than timeout. I think similar changes is required in
> >>Inet6Address.c as well ? Thanks, Vyom
> >>
> >>
> >>On Wednesday 21 September 2016 08:46 PM, Rob McKenna wrote:
> >>>Hi folks,
> >>>
> >>>I'd like to fix a bug caused by an incorrect assumption. The IcmpSendEcho* 
> >>>calls can actually return a similar set of errors regardless of whether 
> >>>the call itself failed or succeeded. This change checks that both the call 
> >>>and the ping were successful. In addition to that it takes a number of 
> >>>common failure causes into account before deciding to throw an exception.
> >>>
> >>>http://cr.openjdk.java.net/~robm/8159410/webrev.01/
> >>>
> >>>   -Rob
> 

Re: RFR - 8159410: InetAddress.isReachable returns true for non existing IP addresses

[Rob McKenna]

Unfortunately the behaviour described is undocumented and was found the hard 
way. This part of the code is a necessity though.

-Rob

On 21/09/16 09:48, Vyom Tewari wrote:
> Hi Rob,
> 
> Do you really think this extra check is required ?
> 
> if (pEchoReply->Status == IP_SUCCESS
> + && (int)pEchoReply->RoundTripTime <= timeout) I did not found any
> doc(MSDN) which explains this. I think in case of IP_SUCCESS "RoundTripTime"
> is always less than timeout. I think similar changes is required in
> Inet6Address.c as well ? Thanks, Vyom
> 
> 
> On Wednesday 21 September 2016 08:46 PM, Rob McKenna wrote:
> >Hi folks,
> >
> >I'd like to fix a bug caused by an incorrect assumption. The IcmpSendEcho* 
> >calls can actually return a similar set of errors regardless of whether the 
> >call itself failed or succeeded. This change checks that both the call and 
> >the ping were successful. In addition to that it takes a number of common 
> >failure causes into account before deciding to throw an exception.
> >
> >http://cr.openjdk.java.net/~robm/8159410/webrev.01/
> >
> > -Rob
> 

Re: RFR - 8159410: InetAddress.isReachable returns true for non existing IP addresses

[Rob McKenna]

It's absolutely worth looking into and I'll get going on that, but I'd rather 
deal with it separately to the ipv4 stuff. IcmpSendEcho already appears to 
behave somewhat strangely, so I wouldn't necessarily assume that the ipv4 and 
ipv6 code will end up being identical.

-Rob

On 21/09/16 05:35, Mark Sheppard wrote:
> Hi Rob,
>  this looks good ...
> 
> do you think there is any need to replicate these changes in
> Inet6AddressImpl.c ? (or leave it alone and don't trouble trouble until
> trouble troubles you :-)
> 
> regards
> Mark
> 
> regards
> Mark
> On 21/09/2016 16:16, Rob McKenna wrote:
> >Hi folks,
> >
> >I'd like to fix a bug caused by an incorrect assumption. The IcmpSendEcho* 
> >calls can actually return a similar set of errors regardless of whether the 
> >call itself failed or succeeded. This change checks that both the call and 
> >the ping were successful. In addition to that it takes a number of common 
> >failure causes into account before deciding to throw an exception.
> >
> >http://cr.openjdk.java.net/~robm/8159410/webrev.01/
> >
> > -Rob
> 

RFR - 8159410: InetAddress.isReachable returns true for non existing IP addresses

[Rob McKenna]

Hi folks,

I'd like to fix a bug caused by an incorrect assumption. The IcmpSendEcho* 
calls can actually return a similar set of errors regardless of whether the 
call itself failed or succeeded. This change checks that both the call and the 
ping were successful. In addition to that it takes a number of common failure 
causes into account before deciding to throw an exception.

http://cr.openjdk.java.net/~robm/8159410/webrev.01/

-Rob

RFR - 8165988: Test JarURLConnectionUseCaches.java fails at windows: failed to clean up files after test

[Rob McKenna]

Hi folks,

A resource cleanup issue can cause this test to fail on windows, fix is to run 
in othervm:

http://cr.openjdk.java.net/~robm/8165988/webrev.01/

-Rob

Re: RFR: 6947916: JarURLConnection does not handle useCaches correctly

[Rob McKenna]

To be explicit, new webrev here:

http://cr.openjdk.java.net/~robm/6947916/webrev.03/

-Rob

On 09/09/16 07:03, Rob McKenna wrote:
> Chris just pointed out to me that the test.classes prefix on the jar path is 
> unnecessary. He also mentioned that jtreg would clear up the scratch 
> directory so the deleteOnExit wouldn't be needed either.
> 
>   -Rob
> 
> On 09/09/16 05:02, Rob McKenna wrote:
> > Will do
> > 
> > -Rob
> > 
> > On 09/09/16 11:00, Roger Riggs wrote:
> > > Hi Rob,
> > > 
> > > Looks ok.
> > > 
> > > Its also a good practice to cleanup the file.  (File.deleteOnExit).
> > > 
> > > Roger
> > > 
> > > 
> > > On 9/9/2016 9:23 AM, Rob McKenna wrote:
> > > >Hey Chris,
> > > >
> > > >Apologies, I'm guilty of "just doing what adjacent tests do" here.
> > > >
> > > >That shell script is actually there in the test source already, but 
> > > >generating the jar from the test means theres no need to use it or to 
> > > >check in a binary. Thanks for picking me up!
> > > >
> > > >http://cr.openjdk.java.net/~robm/6947916/webrev.02/
> > > >
> > > > -Rob
> > > >
> > > >On 08/09/16 08:40, Chris Hegarty wrote:
> > > >>>On 7 Sep 2016, at 14:17, Rob McKenna <rob.mcke...@oracle.com> wrote:
> > > >>>
> > > >>>Hi folks,
> > > >>>
> > > >>>Looking for a review of this simple enough fix:
> > > >>>
> > > >>>http://cr.openjdk.java.net/~robm/6947916/webrev.01/
> > > >>>https://bugs.openjdk.java.net/browse/JDK-6947916
> > > >>I think that the source changes are good, but the tests has a
> > > >>reference to a shell script that is absent. Also, could the test just
> > > >>create a simple jar file, rather than checking in a binary artifact.
> > > >>
> > > >>-Chris.
> > > >>
> > > >>>In a nutshell, if multiple URLConnections are made to several files in 
> > > >>>a single jar, each will use the same backing JarFile object. 
> > > >>>Unfortunately JarURLConnections connect() method recreates the 
> > > >>>jarFileURLConnection for a given JarURLConnection using the default 
> > > >>>value for getUseCaches instead of the *current* value.
> > > >>>
> > > >>>In effect this means that jarURLConnection.getUseCaches() may return 
> > > >>>true before calling connect() and false after. This in turn means that 
> > > >>>when a JarURLConnection's inputstream is closed, it will believe that 
> > > >>>caching has been turned off and the underlying jarFile will be closed 
> > > >>>out from under all other JarURLConnection inputstreams.
> > > >>>
> > > >>>   -Rob
> > > 

Re: RFR: 6947916: JarURLConnection does not handle useCaches correctly

[Rob McKenna]

Chris just pointed out to me that the test.classes prefix on the jar path is 
unnecessary. He also mentioned that jtreg would clear up the scratch directory 
so the deleteOnExit wouldn't be needed either.

-Rob

On 09/09/16 05:02, Rob McKenna wrote:
> Will do
> 
>   -Rob
> 
> On 09/09/16 11:00, Roger Riggs wrote:
> > Hi Rob,
> > 
> > Looks ok.
> > 
> > Its also a good practice to cleanup the file.  (File.deleteOnExit).
> > 
> > Roger
> > 
> > 
> > On 9/9/2016 9:23 AM, Rob McKenna wrote:
> > >Hey Chris,
> > >
> > >Apologies, I'm guilty of "just doing what adjacent tests do" here.
> > >
> > >That shell script is actually there in the test source already, but 
> > >generating the jar from the test means theres no need to use it or to 
> > >check in a binary. Thanks for picking me up!
> > >
> > >http://cr.openjdk.java.net/~robm/6947916/webrev.02/
> > >
> > >   -Rob
> > >
> > >On 08/09/16 08:40, Chris Hegarty wrote:
> > >>>On 7 Sep 2016, at 14:17, Rob McKenna <rob.mcke...@oracle.com> wrote:
> > >>>
> > >>>Hi folks,
> > >>>
> > >>>Looking for a review of this simple enough fix:
> > >>>
> > >>>http://cr.openjdk.java.net/~robm/6947916/webrev.01/
> > >>>https://bugs.openjdk.java.net/browse/JDK-6947916
> > >>I think that the source changes are good, but the tests has a
> > >>reference to a shell script that is absent. Also, could the test just
> > >>create a simple jar file, rather than checking in a binary artifact.
> > >>
> > >>-Chris.
> > >>
> > >>>In a nutshell, if multiple URLConnections are made to several files in a 
> > >>>single jar, each will use the same backing JarFile object. Unfortunately 
> > >>>JarURLConnections connect() method recreates the jarFileURLConnection 
> > >>>for a given JarURLConnection using the default value for getUseCaches 
> > >>>instead of the *current* value.
> > >>>
> > >>>In effect this means that jarURLConnection.getUseCaches() may return 
> > >>>true before calling connect() and false after. This in turn means that 
> > >>>when a JarURLConnection's inputstream is closed, it will believe that 
> > >>>caching has been turned off and the underlying jarFile will be closed 
> > >>>out from under all other JarURLConnection inputstreams.
> > >>>
> > >>> -Rob
> > 

Re: RFR: 6947916: JarURLConnection does not handle useCaches correctly

[Rob McKenna]

Will do

-Rob

On 09/09/16 11:00, Roger Riggs wrote:
> Hi Rob,
> 
> Looks ok.
> 
> Its also a good practice to cleanup the file.  (File.deleteOnExit).
> 
> Roger
> 
> 
> On 9/9/2016 9:23 AM, Rob McKenna wrote:
> >Hey Chris,
> >
> >Apologies, I'm guilty of "just doing what adjacent tests do" here.
> >
> >That shell script is actually there in the test source already, but 
> >generating the jar from the test means theres no need to use it or to check 
> >in a binary. Thanks for picking me up!
> >
> >http://cr.openjdk.java.net/~robm/6947916/webrev.02/
> >
> > -Rob
> >
> >On 08/09/16 08:40, Chris Hegarty wrote:
> >>>On 7 Sep 2016, at 14:17, Rob McKenna <rob.mcke...@oracle.com> wrote:
> >>>
> >>>Hi folks,
> >>>
> >>>Looking for a review of this simple enough fix:
> >>>
> >>>http://cr.openjdk.java.net/~robm/6947916/webrev.01/
> >>>https://bugs.openjdk.java.net/browse/JDK-6947916
> >>I think that the source changes are good, but the tests has a
> >>reference to a shell script that is absent. Also, could the test just
> >>create a simple jar file, rather than checking in a binary artifact.
> >>
> >>-Chris.
> >>
> >>>In a nutshell, if multiple URLConnections are made to several files in a 
> >>>single jar, each will use the same backing JarFile object. Unfortunately 
> >>>JarURLConnections connect() method recreates the jarFileURLConnection for 
> >>>a given JarURLConnection using the default value for getUseCaches instead 
> >>>of the *current* value.
> >>>
> >>>In effect this means that jarURLConnection.getUseCaches() may return true 
> >>>before calling connect() and false after. This in turn means that when a 
> >>>JarURLConnection's inputstream is closed, it will believe that caching has 
> >>>been turned off and the underlying jarFile will be closed out from under 
> >>>all other JarURLConnection inputstreams.
> >>>
> >>>   -Rob
> 

RFR: 6947916: JarURLConnection does not handle useCaches correctly

[Rob McKenna]

Hi folks,

Looking for a review of this simple enough fix:

http://cr.openjdk.java.net/~robm/6947916/webrev.01/
https://bugs.openjdk.java.net/browse/JDK-6947916

In a nutshell, if multiple URLConnections are made to several files in a single 
jar, each will use the same backing JarFile object. Unfortunately 
JarURLConnections connect() method recreates the jarFileURLConnection for a 
given JarURLConnection using the default value for getUseCaches instead of the 
*current* value.

In effect this means that jarURLConnection.getUseCaches() may return true 
before calling connect() and false after. This in turn means that when a 
JarURLConnection's inputstream is closed, it will believe that caching has been 
turned off and the underlying jarFile will be closed out from under all other 
JarURLConnection inputstreams.

-Rob

8150234: Windows 10 App Containers disallow access to ICMP calls

[Rob McKenna]

Hi folks,

Looking for a review for this change.

Basically https://bugs.openjdk.java.net/browse/JDK-8135305 abandoned the old 
TCP echo isReachable check in favour of Windows' ICMP calls on supported 
platforms. Unfortunately it turns out that Windows 10's new App Containers 
don't actually allow access to these calls. This fix adds a check to see if 
access to those calls is allowed on those supported platforms and if not, falls 
back to the old TCP method.

http://cr.openjdk.java.net/~robm/8150234/webrev.01/

-Rob

Re: 8143397: It looks like InetAddress.isReachable(timeout) works incorrectly

[Rob McKenna]

Testing has shown that when a timeout < 1000ms is specified the 
IcmpSendEcho calls fail (apparently) randomly. Once the timeout is 
1000ms or greater it works as expected. Therefore I've updated the fix 
to use 1000ms as a minimum. The existing logic ensures that the ttl is 
less than the specified timeout in any case:


http://cr.openjdk.java.net/~robm/8143397/webrev.02/

-Rob

On 01/12/15 14:59, Rob McKenna wrote:

It appears that there is an undocumented minimum timeout in the
IcmpSendEcho* functions. If the timeout parameter is set to a number
below this minimum timeout it is effectively ignored. Thus if you wanted
to ensure that you could ping a particular host within a certain timeout
less than the undocumented minimum, you could potentially receive a
false positive. (i.e. if you set the timeout to 20ms but the ping takes
30ms, IcmpSendEcho will still succeed)

The following fix checks the icmp reply packet and compares the round
trip time to the requested timeout parameter before deciding whether the
call was successful or not:

http://cr.openjdk.java.net/~robm/8143397/webrev.01/

 -Rob

Re: 8143397: It looks like InetAddress.isReachable(timeout) works incorrectly

[Rob McKenna]

The intention of the 2nd revision of the fix is to make the undocumented 
1000ms problem a non issue.


If a user calls this function with a timeout of 200ms that timeout is 
automatically substituted for 1000ms in the IcmpSendEcho call. Once the 
response is received its RTT is checked to make sure it is lower than 
200ms. If not the method returns false.


That being the case though the call could potentially take up to 1000ms 
to complete where a host is not reachable even though a smaller timeout 
is specified. The return value of isReachable() will correctly say 
whether the host was reachable or not within the actual timeout 
specified by the user however.


In order to avoid the older (and less reliable) way of testing 
reachability I feel that this small corner case is a worthwhile tradeoff.


-Rob

On 09/12/15 01:31, Xuelei Fan wrote:

Is it nice to say in the spec that it is not reliable if the timeout is
too small?  At least 1000ms timeout by default may be not acceptable in
some circumstances.

Xuelei

On 12/9/2015 12:31 AM, Rob McKenna wrote:

Testing has shown that when a timeout < 1000ms is specified the
IcmpSendEcho calls fail (apparently) randomly. Once the timeout is
1000ms or greater it works as expected. Therefore I've updated the fix
to use 1000ms as a minimum. The existing logic ensures that the ttl is
less than the specified timeout in any case:

http://cr.openjdk.java.net/~robm/8143397/webrev.02/

 -Rob

On 01/12/15 14:59, Rob McKenna wrote:

It appears that there is an undocumented minimum timeout in the
IcmpSendEcho* functions. If the timeout parameter is set to a number
below this minimum timeout it is effectively ignored. Thus if you wanted
to ensure that you could ping a particular host within a certain timeout
less than the undocumented minimum, you could potentially receive a
false positive. (i.e. if you set the timeout to 20ms but the ping takes
30ms, IcmpSendEcho will still succeed)

The following fix checks the icmp reply packet and compares the round
trip time to the requested timeout parameter before deciding whether the
call was successful or not:

http://cr.openjdk.java.net/~robm/8143397/webrev.01/

  -Rob

8143397: It looks like InetAddress.isReachable(timeout) works incorrectly

[Rob McKenna]

It appears that there is an undocumented minimum timeout in the 
IcmpSendEcho* functions. If the timeout parameter is set to a number 
below this minimum timeout it is effectively ignored. Thus if you wanted 
to ensure that you could ping a particular host within a certain timeout 
less than the undocumented minimum, you could potentially receive a 
false positive. (i.e. if you set the timeout to 20ms but the ping takes 
30ms, IcmpSendEcho will still succeed)


The following fix checks the icmp reply packet and compares the round 
trip time to the requested timeout parameter before deciding whether the 
call was successful or not:


http://cr.openjdk.java.net/~robm/8143397/webrev.01/

-Rob

Re: Request for review & approval: 8141260: isReachable crash in windows xp

[Rob McKenna]

Sounds good Mark, will do pre-push, cheers.

-Rob

On 25/11/15 18:27, Mark Sheppard wrote:

yes, the fix looks fine, and I verified the individual isReachable test
on the test host where it failed.

it is possible to refactor the function

Java_java_net_Inet4AddressImpl_isReachable0

a little,  by extracting the newly re-introduced "else" block into its
own function
e.g. wxp_ping4

such that

if (isVistaSP1OrGreater()) {
 jint src_addr = 0;
 jint dest_addr = 0;
 jbyte caddr[4];
 int sz;

 /**
  * Convert IP address from byte array to integer
  */
 sz = (*env)->GetArrayLength(env, addrArray);
 if (sz != 4) {
   return JNI_FALSE;
 }
 memset((char *) caddr, 0, sizeof(caddr));
 (*env)->GetByteArrayRegion(env, addrArray, 0, 4, caddr);
 dest_addr = ((caddr[0]<<24) & 0xff00);
 dest_addr |= ((caddr[1] <<16) & 0xff);
 dest_addr |= ((caddr[2] <<8) & 0xff00);
 dest_addr |= (caddr[3] & 0xff);
 dest_addr = htonl(dest_addr);

 /**
  * If a network interface was specified, let's convert its address
  * as well.
  */
 if (!(IS_NULL(ifArray))) {
 memset((char *) caddr, 0, sizeof(caddr));
 (*env)->GetByteArrayRegion(env, ifArray, 0, 4, caddr);
 src_addr = ((caddr[0]<<24) & 0xff00);
 src_addr |= ((caddr[1] <<16) & 0xff);
 src_addr |= ((caddr[2] <<8) & 0xff00);
 src_addr |= (caddr[3] & 0xff);
 src_addr = htonl(src_addr);
 }

 return ping4(env, src_addr, dest_addr, timeout);
 } else {
 wxp_ping4(env, this, addrArray, timeout, ifArray, ttl)
 }



regards
Mark

On 25/11/2015 15:32, Seán Coffey wrote:

Looks ok to me Rob and provides a re-introduction of the old
Java_java_net_Inet4AddressImpl_isReachable0 function for XP systems
where necessary. Reviewed.

Approved for jdk8u-dev also.

Regards,
Sean.

On 25/11/15 14:00, Rob McKenna wrote:

forgot to cc net-dev

-Rob

On 24/11/15 16:37, Rob McKenna wrote:

Hi folks,

The recently updated ICMP (8133015) code fails on Windows XP due to a
missing api. This fix allows XP to fall back to the old tcp based
method:

https://bugs.openjdk.java.net/browse/JDK-8141260
http://cr.openjdk.java.net/~robm/8141260/webrev.01/

 -Rob

Re: Request for review & approval: 8141260: isReachable crash in windows xp

[Rob McKenna]

forgot to cc net-dev

-Rob

On 24/11/15 16:37, Rob McKenna wrote:

Hi folks,

The recently updated ICMP (8133015) code fails on Windows XP due to a
missing api. This fix allows XP to fall back to the old tcp based method:

https://bugs.openjdk.java.net/browse/JDK-8141260
http://cr.openjdk.java.net/~robm/8141260/webrev.01/

 -Rob

Re: RFR: 8135305: InetAddress.isReachable reports true when loopback interface is specified

[Rob McKenna]

Good point. I'll remove that pre-push.

-Rob

On 25/09/15 11:57, Mark Sheppard wrote:


Hi Rob,
looks fine ...  the "him" variable in the

Java_java_net_Inet4AddressImpl_isReachable0

would appear not to be used now, so could probably be removed with its
call on memset ?

regards
Mark

On 24/09/2015 15:45, Rob McKenna wrote:

Please ignore the formatting errors. Thats either a problem with hg
diff or webrev, but its fixed in the repo.

-Rob

On 24/09/15 15:43, Rob McKenna wrote:

Hi folks,

The recent change to isReachable (8133015:
InetAddress.isReachable(tmout) returning wrong value on Windows for IPv6
- http://hg.openjdk.java.net/jdk9/jdk9/jdk/rev/59ff6cd9535d) neglected
to take the specified interface into account for ipv4.

This change ensures that isReachable returns false for external hosts
when the loopback interface is specified.

http://cr.openjdk.java.net/~robm/8135305/webrev.01/

 -Rob

RFR: 8135305: InetAddress.isReachable reports true when loopback interface is specified

[Rob McKenna]

Hi folks,

The recent change to isReachable (8133015: 
InetAddress.isReachable(tmout) returning wrong value on Windows for IPv6 
- http://hg.openjdk.java.net/jdk9/jdk9/jdk/rev/59ff6cd9535d) neglected 
to take the specified interface into account for ipv4.


This change ensures that isReachable returns false for external hosts 
when the loopback interface is specified.


http://cr.openjdk.java.net/~robm/8135305/webrev.01/

-Rob

Re: RFR: 8135305: InetAddress.isReachable reports true when loopback interface is specified

[Rob McKenna]

Please ignore the formatting errors. Thats either a problem with hg diff 
or webrev, but its fixed in the repo.


-Rob

On 24/09/15 15:43, Rob McKenna wrote:

Hi folks,

The recent change to isReachable (8133015:
InetAddress.isReachable(tmout) returning wrong value on Windows for IPv6
- http://hg.openjdk.java.net/jdk9/jdk9/jdk/rev/59ff6cd9535d) neglected
to take the specified interface into account for ipv4.

This change ensures that isReachable returns false for external hosts
when the loopback interface is specified.

http://cr.openjdk.java.net/~robm/8135305/webrev.01/

 -Rob

RFR - 8087190: Regression in sun.net.util.IPAddressUtil.isIPv4LiteralAddress(String)

[Rob McKenna]

Hi folks, looking for a review for this simple change.

The change for https://bugs.openjdk.java.net/browse/JDK-8040747 
initialised each octet to 0. This meant that ... was translated to 
0.0.0.0 before checking the validity of the address.


This small change remedies that.

http://cr.openjdk.java.net/~robm/8087190/webrev.01/

I just noticed that I neglected to add 8087190 to the @bug tag of 
textToNumericFormat.java. I'll do that pre-push.


-Rob

RFR: 8080819: Inet4AddressImpl regression caused by JDK-7180557

[Rob McKenna]

Hi folks,

7180557 used getifaddrs as a way of determining the local hosts ip 
address on Mac OSX in order to fix a problem with OSX's naming system. 
When fixing this we decided to place that call before the call to 
getaddrinfo thus taking the naming system out of the equation.


Unfortunately when running over VPN we return the wrong ifaddr causing a 
failure.


The solution is to place the getifaddrs call below getaddrinfo and to 
only call it if there is an error with getaddrinfo as there was in the 
original (7180557) case.


http://cr.openjdk.java.net/~robm/8080819/webrev.01/

-Rob

Re: 8080819: [PATCH] fix Inet4AddressImpl.lookupAllHostAddr returning unroutable addresses

[Rob McKenna]

Adding bug id to subject for reference.

-Rob

On 22/05/15 16:05, Rob McKenna wrote:

Hi Brian,

By coincidence I just started looking at this today myself.

We have an existing bug filed for this issue:
https://bugs.openjdk.java.net/browse/JDK-8080819

The diagnosis looks good.

 -Rob

On 22/05/15 07:56, Brian Toal wrote:

The fix for https://bugs.openjdk.java.net/browse/JDK-7180557 causes a
regression for Mac OSX hosts connected to a VPN (or otherwise with more
than network
interface associated with the hostname).

For example, when on Cisco VPN, the ifconfig is as follows:

lo0: flags=8049UP,LOOPBACK,RUNNING,MULTICAST mtu 16384
 options=3RXCSUM,TXCSUM
 inet6 fe80::1%lo0 prefixlen 64 scopeid 0x1
 inet 127.0.0.1 netmask 0xff00
 inet6 ::1 prefixlen 128
gif0: flags=8010POINTOPOINT,MULTICAST mtu 1280
stf0: flags=0 mtu 1280
en0: flags=8863UP,BROADCAST,SMART,RUNNING,SIMPLEX,MULTICAST mtu 1500
 ether 28:cf:e9:1a:eb:6b
 inet6 fe80::2acf:e9ff:fe1a:eb6b%en0 prefixlen 64 scopeid 0x4
 inet 192.168.0.106 netmask 0xff00 broadcast 192.168.0.255
 media: autoselect
 status: active
p2p0: flags=8843UP,BROADCAST,RUNNING,SIMPLEX,MULTICAST mtu 2304
 ether 0a:cf:e9:1a:eb:6b
 media: autoselect
 status: inactive
utun0: flags=80d1UP,POINTOPOINT,RUNNING,NOARP,MULTICAST mtu 1320
 inet 10.3.23.199 -- 10.3.23.199 netmask 0xc000
 inet6 fe80::2acf:e9ff:fe1a:eb6b%utun0 prefixlen 64 scopeid 0x6
 inet6 fe80::49cd:21e1:4c11:721d%utun0 prefixlen 128 scopeid 0x6

The hostname entry in DNS is associated with the utun0 interface, and
10.3.23.199 and all routes go through that interface. The 192.168.0.106
IP is my local wifi address, and is not used (other than to tunnel the
VPN traffic when VPN is connected). Any connections to the 192.168
address, even from localhost, will fail (hang indefinitely, as all
packets are dropped).

The OS getaddrinfo calls, when passed the FQDN hostname only return the
10.3 address (getaddrinfo.c attached).

Prior to the fix for JDK-7180557, getaddrinfo was being used and
everything worked fine. As part of the fix, the following block was
added for OSX only in the IPv4 path:

+ #ifdef MACOSX
+ /* If we're looking up the local machine, bypass DNS lookups and
get
+  * address from getifaddrs.
+  */
+ ret = lookupIfLocalhost(env, hostname, JNI_FALSE);
+ if (ret != NULL) {
+ JNU_ReleaseStringPlatformChars(env, host, hostname);
+ return ret;
+ }
+ #endif

This code, which occurs before the usual path and applies only to
localhost lookups, tries to guess the localaddress by interface
enumeration. In the case of a VPN configuration, both en0 (192.168) and
utun0 will match as they are active. So, for example,
InetAddress.getAllByName(localhost FQDN), will return both addresses.
However, the getByName() call and most other code which implicitly uses
the localhost will return only the first entry, which is the unroutable
en0 address.

The host is configured correctly and other processes all resolve the
hostname correctly. The issue is specific to this OSX workaround.

On the IPv6 path for the same fix, the lookupIfLocalhost workaround was
only applied *if* the original lookup via the usual routines failed. The
attached patch follows the same approach in the IPv4 path, if the
orginal lookup fails then call lookupIfLocalhost If this was applied to
the IPv4 path.  The patch fixes the issue without regressing the hosts
that triggered this fix.

# HG changeset patch
# User btoal
# Date 1432276472 25200
#  Thu May 21 23:34:32 2015 -0700
# Node ID 24cf7a8a8bf4f489da6b2506bcb57cb329a93593
# Parent  20e6cadfac43717a81d99daff5e769de695992cd
Only call lookupIfLocalhost if getaddrinfo call errors to avoid
resolving to a incorrect address.

diff -r 20e6cadfac43 -r 24cf7a8a8bf4
src/solaris/native/java/net/Inet4AddressImpl.c
--- a/src/solaris/native/java/net/Inet4AddressImpl.cThu Feb 05
13:00:26 2015 +0100
+++ b/src/solaris/native/java/net/Inet4AddressImpl.cThu May 21
23:34:32 2015 -0700
@@ -172,20 +172,19 @@
  return NULL;
  }

-#ifdef MACOSX
-/* If we're looking up the local machine, bypass DNS lookups and get
- * address from getifaddrs.
- */
-ret = lookupIfLocalhost(env, hostname, JNI_FALSE);
-if (ret != NULL || (*env)-ExceptionCheck(env)) {
-JNU_ReleaseStringPlatformChars(env, host, hostname);
-return ret;
-}
-#endif
-
  error = getaddrinfo(hostname, NULL, hints, res);

  if (error) {
+#ifdef MACOSX
+/* If we're looking up the local machine, bypass DNS lookups
and get
+ * address from getifaddrs.
+ */
+ret = lookupIfLocalhost(env, hostname, JNI_FALSE);
+if (ret != NULL || (*env)-ExceptionCheck(env)) {
+JNU_ReleaseStringPlatformChars(env, host, hostname);
+return ret;
+}
+#endif
  /* report error */
  ThrowUnknownHostExceptionWithGaiError(env, hostname

Re: [PATCH] fix Inet4AddressImpl.lookupAllHostAddr returning unroutable addresses

[Rob McKenna]

Hi Brian,

By coincidence I just started looking at this today myself.

We have an existing bug filed for this issue: 
https://bugs.openjdk.java.net/browse/JDK-8080819


The diagnosis looks good.

-Rob

On 22/05/15 07:56, Brian Toal wrote:

The fix for https://bugs.openjdk.java.net/browse/JDK-7180557 causes a
regression for Mac OSX hosts connected to a VPN (or otherwise with more
than network
interface associated with the hostname).

For example, when on Cisco VPN, the ifconfig is as follows:

lo0: flags=8049UP,LOOPBACK,RUNNING,MULTICAST mtu 16384
 options=3RXCSUM,TXCSUM
 inet6 fe80::1%lo0 prefixlen 64 scopeid 0x1
 inet 127.0.0.1 netmask 0xff00
 inet6 ::1 prefixlen 128
gif0: flags=8010POINTOPOINT,MULTICAST mtu 1280
stf0: flags=0 mtu 1280
en0: flags=8863UP,BROADCAST,SMART,RUNNING,SIMPLEX,MULTICAST mtu 1500
 ether 28:cf:e9:1a:eb:6b
 inet6 fe80::2acf:e9ff:fe1a:eb6b%en0 prefixlen 64 scopeid 0x4
 inet 192.168.0.106 netmask 0xff00 broadcast 192.168.0.255
 media: autoselect
 status: active
p2p0: flags=8843UP,BROADCAST,RUNNING,SIMPLEX,MULTICAST mtu 2304
 ether 0a:cf:e9:1a:eb:6b
 media: autoselect
 status: inactive
utun0: flags=80d1UP,POINTOPOINT,RUNNING,NOARP,MULTICAST mtu 1320
 inet 10.3.23.199 -- 10.3.23.199 netmask 0xc000
 inet6 fe80::2acf:e9ff:fe1a:eb6b%utun0 prefixlen 64 scopeid 0x6
 inet6 fe80::49cd:21e1:4c11:721d%utun0 prefixlen 128 scopeid 0x6

The hostname entry in DNS is associated with the utun0 interface, and
10.3.23.199 and all routes go through that interface. The 192.168.0.106
IP is my local wifi address, and is not used (other than to tunnel the
VPN traffic when VPN is connected). Any connections to the 192.168
address, even from localhost, will fail (hang indefinitely, as all
packets are dropped).

The OS getaddrinfo calls, when passed the FQDN hostname only return the
10.3 address (getaddrinfo.c attached).

Prior to the fix for JDK-7180557, getaddrinfo was being used and
everything worked fine. As part of the fix, the following block was
added for OSX only in the IPv4 path:

+ #ifdef MACOSX
+ /* If we're looking up the local machine, bypass DNS lookups and get
+  * address from getifaddrs.
+  */
+ ret = lookupIfLocalhost(env, hostname, JNI_FALSE);
+ if (ret != NULL) {
+ JNU_ReleaseStringPlatformChars(env, host, hostname);
+ return ret;
+ }
+ #endif

This code, which occurs before the usual path and applies only to
localhost lookups, tries to guess the localaddress by interface
enumeration. In the case of a VPN configuration, both en0 (192.168) and
utun0 will match as they are active. So, for example,
InetAddress.getAllByName(localhost FQDN), will return both addresses.
However, the getByName() call and most other code which implicitly uses
the localhost will return only the first entry, which is the unroutable
en0 address.

The host is configured correctly and other processes all resolve the
hostname correctly. The issue is specific to this OSX workaround.

On the IPv6 path for the same fix, the lookupIfLocalhost workaround was
only applied *if* the original lookup via the usual routines failed. The
attached patch follows the same approach in the IPv4 path, if the
orginal lookup fails then call lookupIfLocalhost If this was applied to
the IPv4 path.  The patch fixes the issue without regressing the hosts
that triggered this fix.

# HG changeset patch
# User btoal
# Date 1432276472 25200
#  Thu May 21 23:34:32 2015 -0700
# Node ID 24cf7a8a8bf4f489da6b2506bcb57cb329a93593
# Parent  20e6cadfac43717a81d99daff5e769de695992cd
Only call lookupIfLocalhost if getaddrinfo call errors to avoid
resolving to a incorrect address.

diff -r 20e6cadfac43 -r 24cf7a8a8bf4
src/solaris/native/java/net/Inet4AddressImpl.c
--- a/src/solaris/native/java/net/Inet4AddressImpl.cThu Feb 05
13:00:26 2015 +0100
+++ b/src/solaris/native/java/net/Inet4AddressImpl.cThu May 21
23:34:32 2015 -0700
@@ -172,20 +172,19 @@
  return NULL;
  }

-#ifdef MACOSX
-/* If we're looking up the local machine, bypass DNS lookups and get
- * address from getifaddrs.
- */
-ret = lookupIfLocalhost(env, hostname, JNI_FALSE);
-if (ret != NULL || (*env)-ExceptionCheck(env)) {
-JNU_ReleaseStringPlatformChars(env, host, hostname);
-return ret;
-}
-#endif
-
  error = getaddrinfo(hostname, NULL, hints, res);

  if (error) {
+#ifdef MACOSX
+/* If we're looking up the local machine, bypass DNS lookups
and get
+ * address from getifaddrs.
+ */
+ret = lookupIfLocalhost(env, hostname, JNI_FALSE);
+if (ret != NULL || (*env)-ExceptionCheck(env)) {
+JNU_ReleaseStringPlatformChars(env, host, hostname);
+return ret;
+}
+#endif
  /* report error */
  ThrowUnknownHostExceptionWithGaiError(env, hostname, error);
  JNU_ReleaseStringPlatformChars(env, host, hostname);

RFR: 8077155: LoginContext Subject ignored by jdk8 sun.net.www.protocol.http.HttpURLConnection

[Rob McKenna]

Hi folks,

Looking for a review of this webrev:

http://cr.openjdk.java.net/~robm/8077155/webrev.01/

Basically the subjects credentials are not available to 
HttpURLConnection.getInputStream0 since it now runs in a doPrivileged 
block. Changing that to doPrivilegedWithCombiner allows getInputStream0 
access to the AccessControlContext's DomainCombiner and the subjects 
associated credentials.


-Rob

Re: RFR: 8075039: (sctp) com/sun/nio/sctp/SctpMultiChannel/SendFailed.java fails on Solaris only

[Rob McKenna]

Pressed send a little too early. This test was added by:

8067846: (sctp) InternalError when receiving SendFailedNotification

http://hg.openjdk.java.net/jdk9/dev/jdk/rev/b10dc4dc6903

-Rob

On 18/03/15 23:27, Rob McKenna wrote:

Hi folks,

Mistakenly neglected to exclude Solaris from testing:

http://cr.openjdk.java.net/~robm/8075039/webrev.01/

-Rob

RFR: 8075039: (sctp) com/sun/nio/sctp/SctpMultiChannel/SendFailed.java fails on Solaris only

[Rob McKenna]

Hi folks,

Mistakenly neglected to exclude Solaris from testing:

http://cr.openjdk.java.net/~robm/8075039/webrev.01/

-Rob

RFR: 8067680: (sctp) Possible race initializing native IDs

[Rob McKenna]

This is a fix to a possible race in the current sctp code. In a nutshell 
the conditional only checks that isaCls is not null. However if isaCls 
is set by one thread and that thread is interrupted before setting 
isaCtrID, the interrupting thread will falsely assume that isaCtrID has 
been set.


http://cr.openjdk.java.net/~robm/8067680/webrev.01/

-Rob

RFR: 8031435: Ftp download does not work properly for ftp user without password

[Rob McKenna]

Hi folks,

Very simple fix to allow FtpURLConnection connect without a password in 
the url. (i.e. ftp://user@server/ as opposed to the current 
ftp://user:@server)


http://cr.openjdk.java.net/~robm/8031435/webrev.01/

-Rob

hg: jdk8/tl/jdk: 7152892: some jtreg tests fail with permission denied

[rob . mckenna]

Changeset: da4b0962ad11
Author:robm
Date:  2014-02-10 14:35 +
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/da4b0962ad11

7152892: some jtreg tests fail with permission denied
Reviewed-by: coffeys

! test/java/lang/ClassLoader/Assert.sh
! test/java/rmi/registry/readTest/readTest.sh
! test/java/util/zip/ZipFile/ReadZip.java
! test/sun/net/www/protocol/jar/jarbug/run.sh

hg: jdk8/tl/jdk: 8029525: java/lang/ProcessBuilder/Basic.java fails intermittently

[rob . mckenna]

Changeset: 72ea199e3e1b
Author:robm
Date:  2013-12-05 16:19 +
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/72ea199e3e1b

8029525: java/lang/ProcessBuilder/Basic.java fails intermittently
Reviewed-by: alanb, chegar
Contributed-by: roger.ri...@oracle.com

! test/java/lang/ProcessBuilder/Basic.java

hg: jdk8/tl/jdk: 6703075: (process) java/lang/ProcessBuilder/Basic.java fails with fastdebug

[rob . mckenna]

Changeset: 89fccc5a7469
Author:martin
Date:  2013-11-21 16:06 +
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/89fccc5a7469

6703075: (process) java/lang/ProcessBuilder/Basic.java fails with fastdebug
Reviewed-by: alanb

! test/java/lang/ProcessBuilder/Basic.java

hg: jdk8/tl/jdk: 8022206: Intermittent test failures in java/lang/ProcessBuilder/Basic.java

[rob . mckenna]

Changeset: 63b696dafc8a
Author:robm
Date:  2013-11-19 15:36 +
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/63b696dafc8a

8022206: Intermittent test failures in java/lang/ProcessBuilder/Basic.java
Reviewed-by: chegar, alanb

! test/java/lang/ProcessBuilder/Basic.java

hg: jdk8/tl/jdk: 8024521: (process) Async close issues with Process InputStream

[rob . mckenna]

Changeset: e2ec05b2ed94
Author:igerasim
Date:  2013-10-23 15:37 +0400
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/e2ec05b2ed94

8024521: (process) Async close issues with Process InputStream
Reviewed-by: psandoz, martin, alanb, robm

! src/solaris/classes/java/lang/UNIXProcess.java.bsd
! src/solaris/classes/java/lang/UNIXProcess.java.linux
+ test/java/lang/Runtime/exec/CloseRace.java

Re: RFR: 8005068 - HttpCookie does not correctly handle negative maxAge values

[Rob McKenna]

Thanks Chris,

I don't mind deferring. Nobody is shouting at me to get this fixed at 
the moment.


-Rob

On 21/10/13 16:21, Chris Hegarty wrote:

Hi Rob,

The changes look fine. ( Trivially the copyright year on the test 
should be 2013 ).


Coming so late in the JDK8 development cycle, only P1-3 bugs are being 
accepted [1]. I see this is a P4. Should it be deferred to the next 
available minor update, or do you think it warrants being fixed in jdk8.


-Chris.

[1] http://openjdk.java.net/projects/jdk8/milestones#Rampdown_start

On 18/10/2013 18:36, Rob McKenna wrote:

Hi folks,

Simple enough change here. As per the description HttpCookie.setMaxAge
will set any arbitrary negative value, while we only check for
MAX_AGE_UNSPECIFIED to determine whether a cookies max age has been
specified or not. This fix sets maxAge to MAX_AGE_UNSPECIFIED if the
setMaxAge(expiry) parameter is  0.

In addition to that HttpCookie.parse(header) incorrectly sets the maxAge
to a negative value if the expires attribute is in the past. This
effectively means it is unspecified instead of expired. This fix sets
such maxAge values to 0 (expire immediately) instead.

https://bugs.openjdk.java.net/browse/JDK-8005068
http://cr.openjdk.java.net/~robm/8005068/webrev.01/

-Rob

hg: jdk8/tl/jdk: 8024660: TEST_BUG: java/lang/ProcessBuilder/*IOHandle.java leaving hotspot.log open in fastdebug builds

[rob . mckenna]

Changeset: 8d1d5a5aeb41
Author:robm
Date:  2013-10-18 16:28 +0100
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/8d1d5a5aeb41

8024660: TEST_BUG: java/lang/ProcessBuilder/*IOHandle.java leaving hotspot.log 
open in fastdebug builds
Reviewed-by: alanb
Contributed-by: pavel.pune...@oracle.com

! test/java/lang/ProcessBuilder/InheritIOEHandle.java
! test/java/lang/ProcessBuilder/SiblingIOEHandle.java

RFR: 8005068 - HttpCookie does not correctly handle negative maxAge values

[Rob McKenna]

Hi folks,

Simple enough change here. As per the description HttpCookie.setMaxAge 
will set any arbitrary negative value, while we only check for 
MAX_AGE_UNSPECIFIED to determine whether a cookies max age has been 
specified or not. This fix sets maxAge to MAX_AGE_UNSPECIFIED if the 
setMaxAge(expiry) parameter is  0.


In addition to that HttpCookie.parse(header) incorrectly sets the maxAge 
to a negative value if the expires attribute is in the past. This 
effectively means it is unspecified instead of expired. This fix sets 
such maxAge values to 0 (expire immediately) instead.


https://bugs.openjdk.java.net/browse/JDK-8005068
http://cr.openjdk.java.net/~robm/8005068/webrev.01/

-Rob

Re: RFR: 7180557 - InetAddress.getLocalHost throws UnknownHostException on java7u5 on OSX

[Rob McKenna]

Thanks Dmitry! I'll correct that nipick pre-push.

-Rob

On 07/10/13 16:47, Dmitry Samersoff wrote:

Rob,

This version of your fix looks good for me.


Inet4AddressImpl.c:

   Thumbs up.

Inet6AddressImpl.c:

   Thumbs up.

173 if (JVM_GetHostName(myhostname, NI_MAXHOST)) {

Nitpicking, explicit == -1 would be better here.



Actually, can you tell me why you'd rather not
include ipv6 loopbacks at all?

If interface don't have ipv6 address but we have ipv6 loopback it most
likely means that ipv6 is not functioning properly.

-Dmitry


On 2013-10-04 19:03, Rob McKenna wrote:

Hi Dmitry,

Thanks a lot for the comprehensive review. W.r.t. line 210, I agree
there is a problem with the logic, but I'd like to suggest an
alternative solution:

- If addrs4 = 1, then we will always be including loopback addresses in
the list. Since the idea of this extra condition is to exclude loopback
interfaces from the list unless they're the only available addresses, I
would suggest (addrs4 == numV4Loopback  addrs6 == numV6Loopback)
instead.

- On the very limited chance that a user has a machine with only an ipv6
loopback configured (unlikely, I'd agree) it probably makes sense to
leave it in there. Actually, can you tell me why you'd rather not
include ipv6 loopbacks at all?

New webrev at:

http://cr.openjdk.java.net/~robm/7180557/webrev.04/

 -Rob

On 21/09/13 12:20, Dmitry Samersoff wrote:

Rob,

See below -
ll. 210 have to be fixed, other comments is just an opinion.


Inet4AddressImpl.c:

ll.132 - it might be better to move initialization to a separate
function, the same way as in Inet6 -  I really like this idea.

Inet6AddressImpl.c


ll. 126.

it's better to move static int initialized into initializeInetClasses()
and don't check it ll. 282.


ll. 170

it looks like rest of the code uses NI_MAXHOST also we have to check
results of JVM_GetHostName if it returns -1 it's probably better to
bailout immediately.


ll. 193 and below - wrong indent

4)

ll. 210

we can have more than one v4 address so

if (addrs4 = 1)

have to be here.

*.

Your include ipv6 loopback in the list if interface has no ipv4
addresses, I'm not sure this logic is correct. On my opinion it's better
to never include ipv6 loopbacks.

*.

Is it better to rename:
includeLoopback = includeLoopbacks


ll. 218

It might be better to calculate arraySize under if at ll. 210 to make
code better readable

ll. 236

It might be better to split if statement to make it more readable at the
cost of duplicating  iter = iter-ifa_next; line.

e.g.

while (iter != NULL) {
...

if (family != AF_INET6 and family != AF_INET) {
  iter = iter-ifa_next;
  continue;
}

if ((!includeV6 and family == AF_INET6)) {
  iter = iter-ifa_next;
  continue;
}

if (!includeLoopback and isLoopback) {
  iter = iter-ifa_next;
  continue;
}

if (iter-ifa_name[0] != '\0'iter-ifa_addr) {
 // Copy address to Java array
  
  iter = iter-ifa_next;
  continue; // redundant just for readability
}
}

ll.244

I'm not sure it's good to return partially complete array in case of
object allocation fail. Probably you should throw

JNU_ThrowOutOfMemoryError(env, Object allocation failed);

-Dmitry


On 2013-09-20 18:58, Rob McKenna wrote:

After a brief discussion with Chris, we decided to revert the position
of the call to lookupIfLocalAddrs so as to give the local host primacy
over DNS.

Latest (and hopefully last) webrev here:

http://cr.openjdk.java.net/~robm/7180557/webrev.03/

  -Rob

On 14/09/13 00:00, Rob McKenna wrote:

Hi Bernd,

I should have said in the context of this bug. What I meant was
removing AI_CANONNAME doesn't resolve the issue as far as Mac is
concerned. I.e. I still see the UnknownHostException. In this
particular case the hostname is not set via the hosts file.

In the latest webrev the call to getifaddrs only occurs if getaddrinfo
fails.

  -Rob

On 13/09/13 20:28, Bernd Eckenfels wrote:

Am 13.09.2013, 19:32 Uhr, schrieb Rob McKenna
rob.mcke...@oracle.com:

W.r.t. the use of AI_CANONNAME, this doesn't actually make a
difference in the context of this fix, but is definitely something
that should be looked at. I'll put it on the todo list.

I think it does make a difference: If you remove the CANON flag
getaddrinfo() will not do DNS lookups when the host is configured to
prefer the hosts file (which it should do on Linux and OSX). And so
the platform specific lookupIfLocalhost() can be put after the
getaddrinfo() (again).

I actually think the bug exists on all platforms. If getaddrinfo()
fails because neighter hosts nor DNS file finds the name this can
happen on all platforms. I dont think it helps to add a fallback only
on MACOSX (and there is certainly no need to prefer the fallback
then).

Gruss
Bernd

Re: RFR: 7180557 - InetAddress.getLocalHost throws UnknownHostException on java7u5 on OSX

[Rob McKenna]

Heh, you just beat me to the punch.

-Rob

On 07/10/13 17:34, Dmitry Samersoff wrote:

Rob,

Existing code uses if (JVM_GetHostName(myhostname, NI_MAXHOST)) so I
withdraw my last comments. Please, don't change it!!!

-Dmitry

On 2013-10-07 20:30, Rob McKenna wrote:

Thanks Dmitry! I'll correct that nipick pre-push.

 -Rob

On 07/10/13 16:47, Dmitry Samersoff wrote:

Rob,

This version of your fix looks good for me.


Inet4AddressImpl.c:

Thumbs up.

Inet6AddressImpl.c:

Thumbs up.

173 if (JVM_GetHostName(myhostname, NI_MAXHOST)) {

Nitpicking, explicit == -1 would be better here.



Actually, can you tell me why you'd rather not
include ipv6 loopbacks at all?

If interface don't have ipv6 address but we have ipv6 loopback it most
likely means that ipv6 is not functioning properly.

-Dmitry


On 2013-10-04 19:03, Rob McKenna wrote:

Hi Dmitry,

Thanks a lot for the comprehensive review. W.r.t. line 210, I agree
there is a problem with the logic, but I'd like to suggest an
alternative solution:

- If addrs4 = 1, then we will always be including loopback addresses in
the list. Since the idea of this extra condition is to exclude loopback
interfaces from the list unless they're the only available addresses, I
would suggest (addrs4 == numV4Loopback  addrs6 == numV6Loopback)
instead.

- On the very limited chance that a user has a machine with only an ipv6
loopback configured (unlikely, I'd agree) it probably makes sense to
leave it in there. Actually, can you tell me why you'd rather not
include ipv6 loopbacks at all?

New webrev at:

http://cr.openjdk.java.net/~robm/7180557/webrev.04/

  -Rob

On 21/09/13 12:20, Dmitry Samersoff wrote:

Rob,

See below -
ll. 210 have to be fixed, other comments is just an opinion.


Inet4AddressImpl.c:

ll.132 - it might be better to move initialization to a separate
function, the same way as in Inet6 -  I really like this idea.

Inet6AddressImpl.c


ll. 126.

it's better to move static int initialized into initializeInetClasses()
and don't check it ll. 282.


ll. 170

it looks like rest of the code uses NI_MAXHOST also we have to check
results of JVM_GetHostName if it returns -1 it's probably better to
bailout immediately.


ll. 193 and below - wrong indent

4)

ll. 210

we can have more than one v4 address so

if (addrs4 = 1)

have to be here.

*.

Your include ipv6 loopback in the list if interface has no ipv4
addresses, I'm not sure this logic is correct. On my opinion it's
better
to never include ipv6 loopbacks.

*.

Is it better to rename:
includeLoopback = includeLoopbacks


ll. 218

It might be better to calculate arraySize under if at ll. 210 to make
code better readable

ll. 236

It might be better to split if statement to make it more readable at
the
cost of duplicating  iter = iter-ifa_next; line.

e.g.

while (iter != NULL) {
...

 if (family != AF_INET6 and family != AF_INET) {
   iter = iter-ifa_next;
   continue;
 }

 if ((!includeV6 and family == AF_INET6)) {
   iter = iter-ifa_next;
   continue;
 }

 if (!includeLoopback and isLoopback) {
   iter = iter-ifa_next;
   continue;
 }

 if (iter-ifa_name[0] != '\0'iter-ifa_addr) {
  // Copy address to Java array
   
   iter = iter-ifa_next;
   continue; // redundant just for readability
 }
}

ll.244

I'm not sure it's good to return partially complete array in case of
object allocation fail. Probably you should throw

JNU_ThrowOutOfMemoryError(env, Object allocation failed);

-Dmitry


On 2013-09-20 18:58, Rob McKenna wrote:

After a brief discussion with Chris, we decided to revert the position
of the call to lookupIfLocalAddrs so as to give the local host primacy
over DNS.

Latest (and hopefully last) webrev here:

http://cr.openjdk.java.net/~robm/7180557/webrev.03/

   -Rob

On 14/09/13 00:00, Rob McKenna wrote:

Hi Bernd,

I should have said in the context of this bug. What I meant was
removing AI_CANONNAME doesn't resolve the issue as far as Mac is
concerned. I.e. I still see the UnknownHostException. In this
particular case the hostname is not set via the hosts file.

In the latest webrev the call to getifaddrs only occurs if
getaddrinfo
fails.

   -Rob

On 13/09/13 20:28, Bernd Eckenfels wrote:

Am 13.09.2013, 19:32 Uhr, schrieb Rob McKenna
rob.mcke...@oracle.com:

W.r.t. the use of AI_CANONNAME, this doesn't actually make a
difference in the context of this fix, but is definitely something
that should be looked at. I'll put it on the todo list.

I think it does make a difference: If you remove the CANON flag
getaddrinfo() will not do DNS lookups when the host is configured to
prefer the hosts file (which it should do on Linux and OSX). And so
the platform specific lookupIfLocalhost() can be put after the
getaddrinfo() (again).

I actually think the bug exists on all platforms. If getaddrinfo()
fails because neighter hosts nor DNS file finds the name this can
happen on all platforms. I dont

Re: RFR: 7180557 - InetAddress.getLocalHost throws UnknownHostException on java7u5 on OSX

[Rob McKenna]

Hi Dmitry,

Thanks a lot for the comprehensive review. W.r.t. line 210, I agree 
there is a problem with the logic, but I'd like to suggest an 
alternative solution:


- If addrs4 = 1, then we will always be including loopback addresses in 
the list. Since the idea of this extra condition is to exclude loopback 
interfaces from the list unless they're the only available addresses, I 
would suggest (addrs4 == numV4Loopback  addrs6 == numV6Loopback) 
instead.


- On the very limited chance that a user has a machine with only an ipv6 
loopback configured (unlikely, I'd agree) it probably makes sense to 
leave it in there. Actually, can you tell me why you'd rather not 
include ipv6 loopbacks at all?


New webrev at:

http://cr.openjdk.java.net/~robm/7180557/webrev.04/

-Rob

On 21/09/13 12:20, Dmitry Samersoff wrote:

Rob,

See below -
ll. 210 have to be fixed, other comments is just an opinion.


Inet4AddressImpl.c:

ll.132 - it might be better to move initialization to a separate
function, the same way as in Inet6 -  I really like this idea.

Inet6AddressImpl.c


ll. 126.

it's better to move static int initialized into initializeInetClasses()
and don't check it ll. 282.


ll. 170

it looks like rest of the code uses NI_MAXHOST also we have to check
results of JVM_GetHostName if it returns -1 it's probably better to
bailout immediately.


ll. 193 and below - wrong indent

4)

ll. 210

we can have more than one v4 address so

if (addrs4 = 1)

have to be here.

*.

Your include ipv6 loopback in the list if interface has no ipv4
addresses, I'm not sure this logic is correct. On my opinion it's better
to never include ipv6 loopbacks.

*.

Is it better to rename:
includeLoopback = includeLoopbacks


ll. 218

It might be better to calculate arraySize under if at ll. 210 to make
code better readable

ll. 236

It might be better to split if statement to make it more readable at the
cost of duplicating  iter = iter-ifa_next; line.

e.g.

while (iter != NULL) {
...

   if (family != AF_INET6 and family != AF_INET) {
 iter = iter-ifa_next;
 continue;
   }

   if ((!includeV6 and family == AF_INET6)) {
 iter = iter-ifa_next;
 continue;
   }

   if (!includeLoopback and isLoopback) {
 iter = iter-ifa_next;
 continue;
   }

   if (iter-ifa_name[0] != '\0'iter-ifa_addr) {
// Copy address to Java array
 
 iter = iter-ifa_next;
 continue; // redundant just for readability
   }
}

ll.244

I'm not sure it's good to return partially complete array in case of
object allocation fail. Probably you should throw

JNU_ThrowOutOfMemoryError(env, Object allocation failed);

-Dmitry


On 2013-09-20 18:58, Rob McKenna wrote:

After a brief discussion with Chris, we decided to revert the position
of the call to lookupIfLocalAddrs so as to give the local host primacy
over DNS.

Latest (and hopefully last) webrev here:

http://cr.openjdk.java.net/~robm/7180557/webrev.03/

 -Rob

On 14/09/13 00:00, Rob McKenna wrote:

Hi Bernd,

I should have said in the context of this bug. What I meant was
removing AI_CANONNAME doesn't resolve the issue as far as Mac is
concerned. I.e. I still see the UnknownHostException. In this
particular case the hostname is not set via the hosts file.

In the latest webrev the call to getifaddrs only occurs if getaddrinfo
fails.

 -Rob

On 13/09/13 20:28, Bernd Eckenfels wrote:

Am 13.09.2013, 19:32 Uhr, schrieb Rob McKenna rob.mcke...@oracle.com:

W.r.t. the use of AI_CANONNAME, this doesn't actually make a
difference in the context of this fix, but is definitely something
that should be looked at. I'll put it on the todo list.

I think it does make a difference: If you remove the CANON flag
getaddrinfo() will not do DNS lookups when the host is configured to
prefer the hosts file (which it should do on Linux and OSX). And so
the platform specific lookupIfLocalhost() can be put after the
getaddrinfo() (again).

I actually think the bug exists on all platforms. If getaddrinfo()
fails because neighter hosts nor DNS file finds the name this can
happen on all platforms. I dont think it helps to add a fallback only
on MACOSX (and there is certainly no need to prefer the fallback then).

Gruss
Bernd

Re: RFR: 7180557 - InetAddress.getLocalHost throws UnknownHostException on java7u5 on OSX

[Rob McKenna]

After a brief discussion with Chris, we decided to revert the position 
of the call to lookupIfLocalAddrs so as to give the local host primacy 
over DNS.


Latest (and hopefully last) webrev here:

http://cr.openjdk.java.net/~robm/7180557/webrev.03/

-Rob

On 14/09/13 00:00, Rob McKenna wrote:

Hi Bernd,

I should have said in the context of this bug. What I meant was 
removing AI_CANONNAME doesn't resolve the issue as far as Mac is 
concerned. I.e. I still see the UnknownHostException. In this 
particular case the hostname is not set via the hosts file.


In the latest webrev the call to getifaddrs only occurs if getaddrinfo 
fails.


-Rob

On 13/09/13 20:28, Bernd Eckenfels wrote:

Am 13.09.2013, 19:32 Uhr, schrieb Rob McKenna rob.mcke...@oracle.com:
W.r.t. the use of AI_CANONNAME, this doesn't actually make a 
difference in the context of this fix, but is definitely something 
that should be looked at. I'll put it on the todo list.


I think it does make a difference: If you remove the CANON flag 
getaddrinfo() will not do DNS lookups when the host is configured to 
prefer the hosts file (which it should do on Linux and OSX). And so 
the platform specific lookupIfLocalhost() can be put after the 
getaddrinfo() (again).


I actually think the bug exists on all platforms. If getaddrinfo() 
fails because neighter hosts nor DNS file finds the name this can 
happen on all platforms. I dont think it helps to add a fallback only 
on MACOSX (and there is certainly no need to prefer the fallback then).


Gruss
Bernd

Re: RFR: 7180557 - InetAddress.getLocalHost throws UnknownHostException on java7u5 on OSX

[Rob McKenna]

Hi folks, updated webrev at:

http://cr.openjdk.java.net/~robm/7180557/webrev.02/

Hopefully all of your concerns have been addressed.

W.r.t. the use of AI_CANONNAME, this doesn't actually make a difference 
in the context of this fix, but is definitely something that should be 
looked at. I'll put it on the todo list.


-Rob

On 05/09/13 21:33, Dmitry Samersoff wrote:

Rob,

Did you try to remove

hints.ai_flags = AI_CANONNAME

this flag asks getaddreinfo to return FQDN, but the function behavior is
not clearly defined for the case where FQDN is not available.

-Dmitry

On 2013-09-03 19:18, Rob McKenna wrote:

Hi folks,

Mac seems to have trouble looking up local hostnames using getaddrinfo
unless a domain is set. The solution is to add a check with getifaddrs .

This fix replaces a usage of _ALLBSD_SOURCE with MACOSX. I haven't seen
a canonical answer on whether this is the way to go so I figured trial
by fire might get the discussion going.

http://cr.openjdk.java.net/~robm/7180557/webrev.01/

 -Rob

Re: RFR: 7180557 - InetAddress.getLocalHost throws UnknownHostException on java7u5 on OSX

[Rob McKenna]

Hi Bernd,

I should have said in the context of this bug. What I meant was removing 
AI_CANONNAME doesn't resolve the issue as far as Mac is concerned. I.e. 
I still see the UnknownHostException. In this particular case the 
hostname is not set via the hosts file.


In the latest webrev the call to getifaddrs only occurs if getaddrinfo 
fails.


-Rob

On 13/09/13 20:28, Bernd Eckenfels wrote:

Am 13.09.2013, 19:32 Uhr, schrieb Rob McKenna rob.mcke...@oracle.com:
W.r.t. the use of AI_CANONNAME, this doesn't actually make a 
difference in the context of this fix, but is definitely something 
that should be looked at. I'll put it on the todo list.


I think it does make a difference: If you remove the CANON flag 
getaddrinfo() will not do DNS lookups when the host is configured to 
prefer the hosts file (which it should do on Linux and OSX). And so 
the platform specific lookupIfLocalhost() can be put after the 
getaddrinfo() (again).


I actually think the bug exists on all platforms. If getaddrinfo() 
fails because neighter hosts nor DNS file finds the name this can 
happen on all platforms. I dont think it helps to add a fallback only 
on MACOSX (and there is certainly no need to prefer the fallback then).


Gruss
Bernd

Re: RFR: 7180557 - InetAddress.getLocalHost throws UnknownHostException on java7u5 on OSX

[Rob McKenna]

Thanks for the comments folks. Chris, I like the idea of moving this 
check below the GAI call. Dmitry, that loop is indeed a bit of a code 
smell. I'll take care of it. Bernd / Dmitry, thanks for the notes on 
AI_CANONNAME. I'll adjust the code and get some testing done then report 
back!


I've kept this code as close to the apple version as sensible but the 
feedback received so far trumps that.


-Rob

On 05/09/13 22:30, Bernd Eckenfels wrote:

Hello,

I reported before, AI_CANONNAME is used in different places with no 
good reason. If you use the flag, the result would be in 
res[0].ai_canonname, which is not used. So you can remove it and safe 
the elaborate resolving which comes with it.


And I also think the comment skip DNS lookup is wrong, as GAI 
typically (also) looks in /etc/hosts - and this is also documented in 
the mac os man page.


So I think if you remove the flag you dont need the lookupIflocalhost 
shortcut at all (or you need it on all systems).


And if you have a look there, I think the AI_ADDRCONFIG should be 
considered, instead.


Gruss
Bernd

Am 05.09.2013, 22:33 Uhr, schrieb Dmitry Samersoff 
dmitry.samers...@oracle.com:



Rob,

Did you try to remove

hints.ai_flags = AI_CANONNAME

this flag asks getaddreinfo to return FQDN, but the function behavior is
not clearly defined for the case where FQDN is not available.

Re: RFR: 7180557 - InetAddress.getLocalHost throws UnknownHostException on java7u5 on OSX

[Rob McKenna]

Indeed it did.

-Rob

On 04/09/13 10:36, Chris Hegarty wrote:

Rob,

I haven't looked at the changes yet, but I'm curious about the 
decision to use getifaddrs. I know that there was a discussion started 
at one point to determine what Apple's JDK6 is doing to retrieve the 
local machines IP information. Did that discussion lead to getifaddrs?


-Chris.

On 03/09/2013 16:18, Rob McKenna wrote:

Hi folks,

Mac seems to have trouble looking up local hostnames using getaddrinfo
unless a domain is set. The solution is to add a check with getifaddrs .

This fix replaces a usage of _ALLBSD_SOURCE with MACOSX. I haven't seen
a canonical answer on whether this is the way to go so I figured trial
by fire might get the discussion going.

http://cr.openjdk.java.net/~robm/7180557/webrev.01/

-Rob

RFR: 7180557 - InetAddress.getLocalHost throws UnknownHostException on java7u5 on OSX

[Rob McKenna]

Hi folks,

Mac seems to have trouble looking up local hostnames using getaddrinfo 
unless a domain is set. The solution is to add a check with getifaddrs .


This fix replaces a usage of _ALLBSD_SOURCE with MACOSX. I haven't seen 
a canonical answer on whether this is the way to go so I figured trial 
by fire might get the discussion going.


http://cr.openjdk.java.net/~robm/7180557/webrev.01/

-Rob

hg: jdk8/tl/jdk: 5049299: (process) Use posix_spawn, not fork, on S10 to avoid swap exhaustion

[rob . mckenna]

Changeset: a4b0be7341ef
Author:robm
Date:  2013-08-13 19:10 +0100
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/a4b0be7341ef

5049299: (process) Use posix_spawn, not fork, on S10 to avoid swap exhaustion
Reviewed-by: alanb, dholmes, martin, erikj, coffeys

! make/java/java/Exportedfiles.gmk
! make/java/java/Makefile
! makefiles/CompileLaunchers.gmk
! makefiles/CompileNativeLibraries.gmk
! src/solaris/classes/java/lang/UNIXProcess.java.bsd
! src/solaris/classes/java/lang/UNIXProcess.java.linux
! src/solaris/classes/java/lang/UNIXProcess.java.solaris
! src/solaris/native/java/lang/UNIXProcess_md.c
+ src/solaris/native/java/lang/childproc.c
+ src/solaris/native/java/lang/childproc.h
+ src/solaris/native/java/lang/jspawnhelper.c
! test/java/lang/ProcessBuilder/Basic.java

hg: jdk8/tl/jdk: 2 new changesets

[rob . mckenna]

Changeset: 607fa1ff3de2
Author:bpb
Date:  2013-07-09 11:26 -0700
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/607fa1ff3de2

6178739: (fmt) Formatter.format(%0.4f\n, 56789.456789) generates 
MissingFormatWidthException
Summary: Change the field width specification to require a positive value. The 
exception is still thrown but that is now explicitly consistent with the 
specification.
Reviewed-by: darcy
Contributed-by: Brian Burkhalter brian.burkhal...@oracle.com

! src/share/classes/java/util/Formatter.java

Changeset: 2ee772cda1d6
Author:bpb
Date:  2013-07-09 12:47 -0700
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/2ee772cda1d6

6480539: BigDecimal.stripTrailingZeros() has no effect on zero itself (0.0)
Summary: Make stripTrailingZeros() return BigDecimal.ZERO if the BigDecimal is 
numerically equal to zero.
Reviewed-by: darcy
Contributed-by: Brian Burkhalter brian.burkhal...@oracle.com

! src/share/classes/java/math/BigDecimal.java
! test/java/math/BigDecimal/StrippingZerosTest.java

Re: possible NIO selector leak in 7u25

[Rob McKenna]

Glad to hear you got to the bottom of it.

-Rob

On 08/07/13 23:22, Bernd Eckenfels wrote:

Hello,

we found a cause for the leak, we did not use the latest xnio-nio 
release. Looking at the NioTcpChannel code I guess that for example 
this commit could fix a potential problem (we shutdown r+w before we 
close, which would not be passed on by the older xnio-nio):


https://github.com/xnio/xnio/commit/71ebef70d11eedce9a0eb7e5de4d37ab22648b73 



When we update the implementation to 3.0.8.GA we dont see the handles 
accumulate anymore. So sorry for blaming NIO (I am still not sure why 
this does not show up in the heapdumps. I have done some test programs 
where I havent closed() the channels or havent selected on the 
canceled keys. And in all those conditions I see Java heap objects 
representing the (open) FDs. But I dont see them in our test 
environment).


Oh well... thanks for listening, it always helps to explain your 
problems to others, especially if they are home made :)


Bernd

hg: jdk8/tl/jdk: 8016063: getFinalAttributes should use FindClose

[rob . mckenna]

Changeset: d28f802ce914
Author:robm
Date:  2013-06-06 22:22 +0100
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/d28f802ce914

8016063: getFinalAttributes should use FindClose
Reviewed-by: alanb

! src/windows/native/java/io/WinNTFileSystem_md.c

hg: jdk8/tl/jdk: 7038105: File.isHidden() should return true for pagefile.sys and hiberfil.sys

[rob . mckenna]

Changeset: b16a8b4ae6b4
Author:robm
Date:  2013-05-28 16:35 +0100
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/b16a8b4ae6b4

7038105: File.isHidden() should return true for pagefile.sys and hiberfil.sys
Reviewed-by: alanb

! src/windows/native/java/io/WinNTFileSystem_md.c
! test/java/io/File/IsHidden.java

Re: Problem with fix B6369510 for HttpURLConnection Content-Type

[Rob McKenna]

HI Matthew,

On the face of it this makes sense. I don't have time to dig into it 
this week, but I'll get stuck into it next week and get a fix together.


-Rob

On 27/03/13 00:42, Matthew Hall wrote:

Forgot to include, offending code in HttpURLConnection:

if (!method.equals(PUT)  (poster != null || streaming()))
 requests.setIfNotSet (Content-type, application/x-www-form-urlencoded);

Format adjusted a bit for readability.

Matthew.

On Tue, Mar 26, 2013 at 05:33:15PM -0700, Matthew Hall wrote:

Hello,

I was working on a situation which was similar to the situation described in
this bug which was supposedly fixed in Java 5 and Java 6:

http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6369510

The bug described how Content-Type was being auto-set to
application/x-www-form-urlencoded in cases where it should not be.

I was seeing this problem, where the open-source JSCEP library was creating a
request to a Tomcat servlet I am implementing, which Tomcat was rejecting due
to encoding issues in the POST body.

When I looked at the traffic using Wireshark TCP Stream reassembly I
discovered that the request had the URL-encoded content type even though no
code in JSCEP requested this.

Upon reading through the unit test,
openjdk-7/jdk/test/sun/net/www/protocol/http/B6369510.java, I found a couple
of issues:

1) The test fails if you have an IPv6 address configured on the system,
because the code doesn't enclose the IPv6 literal with '[]':

URL url = new URL(http://; + address.getHostName() + : + address.getPort() + 
/test/);

java.net.MalformedURLException: For input string: 0:0:0:0:0:0:0:40392
 at java.net.URL.init(URL.java:601)
 at java.net.URL.init(URL.java:464)
 at java.net.URL.init(URL.java:413)
 at B6369510.doClient(B6369510.java:63)
 at B6369510.init(B6369510.java:52)
 at B6369510.main(B6369510.java:45)

2) There appears to be a logic error in the test, or the fix and the bug
description do not match:

if (requestMethod.equalsIgnoreCase(GET)  ct != null 
 ct.get(0).equals(application/x-www-form-urlencoded))
 t.sendResponseHeaders(400, -1);

else if (requestMethod.equalsIgnoreCase(POST)  ct != null 
  !ct.get(0).equals(application/x-www-form-urlencoded))
 t.sendResponseHeaders(400, -1);

This code is saying, the unit test will fail if the method is GET, and the
content-type is equal to url-encoded, and the unit test will fail if the
method is POST, and the content-type is *NOT* equal to url-encoded.

But, in the evaluation, the bug states, Content-Type is being set to
application/x-www-form-urlencoded for all HttpURLConnections requests other
than PUT requests. This is not necessary and could even cause problems for
some servers. We do not need to set this content-type for GET requests.

To me this means, the code should not be setting the Content-Type to anything,
on any type of request, because it will cause problems across the board.

So I think that the test and the bug fix do not actually fix the original bug
correctly, and the test needs to be fixed so it will work right on an IPv6
based system.

Thoughts?
Matthew Hall.

hg: jdk8/tl/jdk: 8009251: Add proxy handling and keep-alive fixes to jsse

[rob . mckenna]

Changeset: 3f8fbb0ab155
Author:robm
Date:  2013-03-21 17:33 +
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/3f8fbb0ab155

8009251: Add proxy handling and keep-alive fixes to jsse
Reviewed-by: chegar

! src/share/classes/sun/net/www/http/HttpClient.java
! 
src/share/classes/sun/net/www/protocol/https/AbstractDelegateHttpsURLConnection.java
! src/share/classes/sun/net/www/protocol/https/HttpsClient.java

hg: jdk8/tl/jdk: 8009650: HttpClient available() check throws SocketException when connection has been closed

[rob . mckenna]

Changeset: f5c85c0a9af0
Author:robm
Date:  2013-03-14 00:21 +
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/f5c85c0a9af0

8009650: HttpClient available() check throws SocketException when connection 
has been closed
Reviewed-by: chegar, khazra, dsamersoff
Contributed-by: sdoug...@redhat.com

! src/share/classes/sun/net/www/http/HttpClient.java
+ test/sun/net/www/http/HttpClient/IsAvailable.java

Re: Request for review: 8009650 - HttpClient available() check throws SocketException when connection has been closed

[Rob McKenna]

The outer try/catch is meant to catch potential exceptions originating 
from the inner try/finally. (from setSoTimeout)


-Rob

On 07/03/13 16:51, Kurchi Subhra Hazra wrote:

I am wondering why do you need two try-catch blocks here.

- Kurchi

On 3/7/13 8:18 AM, Rob McKenna wrote:

Hi folks,

This is a slight alteration of the fix contributed by Stuart Douglas. 
This fix deals with a SocketException caused by getSoTimeout() on a 
closed connection.


http://cr.openjdk.java.net/~robm/8009650/webrev.01/

-Rob

Re: Request for review: 8009650 - HttpClient available() check throws SocketException when connection has been closed

[Rob McKenna]

I've fleshed out the bug report a little to make that clearer, sorry Kurchi!

Also, I'll add a testcase to this review soon.

-Rob

On 07/03/13 16:51, Kurchi Subhra Hazra wrote:

I am wondering why do you need two try-catch blocks here.

- Kurchi

On 3/7/13 8:18 AM, Rob McKenna wrote:

Hi folks,

This is a slight alteration of the fix contributed by Stuart Douglas. 
This fix deals with a SocketException caused by getSoTimeout() on a 
closed connection.


http://cr.openjdk.java.net/~robm/8009650/webrev.01/

-Rob

Re: Request for review: 8009650 - HttpClient available() check throws SocketException when connection has been closed

[Rob McKenna]

Hi Dmitry,

I'm not 100% sure what you mean by duplication, the exceptions and their 
messages are distinct. I think it would be best to keep it that way.


-Rob

On 07/03/13 22:00, Dmitry Samersoff wrote:

Rob,

Is it possible to avoid code duplication?

i.e. do something like this:

int r;

try {
   ...
} catch (SocketException e) {
 // Comments goes here
 r = -1
}

if (r == -1){
   if (logger. ...
   available = false;
}

   return available;

-Dmitry


On 2013-03-07 20:18, Rob McKenna wrote:

Hi folks,

This is a slight alteration of the fix contributed by Stuart Douglas.
This fix deals with a SocketException caused by getSoTimeout() on a
closed connection.

http://cr.openjdk.java.net/~robm/8009650/webrev.01/

 -Rob

Re: Request for review: 8009650 - HttpClient available() check throws SocketException when connection has been closed

[Rob McKenna]

Ah, I see what you mean. Can do.

-Rob

On 07/03/13 23:13, Dmitry Samersoff wrote:

Rob,

Sorry for not being clean enough. We have repeated pattern:

  if (logger.isLoggable(PlatformLogger.FINEST)) {
  logger.finest(HttpClient.available():  + msg
  }

so it makes code better readable if we can put it to some common place.

-Dmitry

On 2013-03-08 02:31, Rob McKenna wrote:

Hi Dmitry,

I'm not 100% sure what you mean by duplication, the exceptions and their
messages are distinct. I think it would be best to keep it that way.

 -Rob

On 07/03/13 22:00, Dmitry Samersoff wrote:

Rob,

Is it possible to avoid code duplication?

i.e. do something like this:

 int r;

 try {
...
 } catch (SocketException e) {
  // Comments goes here
  r = -1
 }

 if (r == -1){
if (logger. ...
available = false;
 }

return available;

-Dmitry


On 2013-03-07 20:18, Rob McKenna wrote:

Hi folks,

This is a slight alteration of the fix contributed by Stuart Douglas.
This fix deals with a SocketException caused by getSoTimeout() on a
closed connection.

http://cr.openjdk.java.net/~robm/8009650/webrev.01/

  -Rob

Re: OpenJDK HTTP Client Bug

[Rob McKenna]

Thanks again, I'll get this sorted as soon as I can.

-Rob

On 02/03/13 20:59, Stuart Douglas wrote:
The review ID it was assigned was 2436558, however I did not hear 
anything more.


(Also I am not sure if my last post actually made it through to the 
net-dev list, as I got a bounce message saying it was awaiting 
moderator approval as I am not a member).


Stuart

Rob McKenna wrote:

Thanks for this Stuart, do you happen to have a reference to the bug you
filed on the Oracle site?

-Rob

On 02/03/13 08:55, Stuart Douglas wrote:

Seeing as the patch appears to have been stripped here it is inline:

diff -r 2eb3ac105b7f src/share/classes/sun/net/www/http/HttpClient.java
--- a/src/share/classes/sun/net/www/http/HttpClient.java Wed Feb 27
21:05:09 2013 -0800
+++ b/src/share/classes/sun/net/www/http/HttpClient.java Sat Mar 02
19:06:26 2013 +1100
@@ -362,13 +362,15 @@
protected synchronized boolean available() throws IOException {
boolean available = true;
- int old = serverSocket.getSoTimeout();
- serverSocket.setSoTimeout(1);
- BufferedInputStream tmpbuf =
- new BufferedInputStream(serverSocket.getInputStream());
+ int old = -1;
PlatformLogger logger = HttpURLConnection.getHttpLogger();
try {
+ old = serverSocket.getSoTimeout();
+ serverSocket.setSoTimeout(1);
+
+ BufferedInputStream tmpbuf =
+ new BufferedInputStream(serverSocket.getInputStream());
int r = tmpbuf.read();
if (r == -1) {
if (logger.isLoggable(PlatformLogger.FINEST)) {
@@ -382,8 +384,17 @@
logger.finest(HttpClient.available():  +
SocketTimeout: its available);
}
+ } catch (SocketException e) {
+ if (logger.isLoggable(PlatformLogger.FINEST)) {
+ logger.finest(HttpClient.available():  +
+ SocketException: not available);
+ }
+ available = false;
+ old = -1; //we don't want to call setSoTimeout, as that will throw
an exception
} finally {
- serverSocket.setSoTimeout(old);
+ if(old != -1) {
+ serverSocket.setSoTimeout(old);
+ }
}
return available;
}


- Original Message -

From: Chris Hegarty chris.hega...@oracle.com
To: Stuart Douglas sdoug...@redhat.com
Cc: Alessio Soldano asold...@redhat.com, OpenJDK Network Dev
list net-dev@openjdk.java.net, Rob McKenna
rob.mcke...@oracle.com
Sent: Saturday, 2 March, 2013 7:37:30 PM
Subject: Re: OpenJDK HTTP Client Bug

[bcc'ing off jdk7-...@openjdk.java.net, and including
net-dev@openjdk.java.net]

Hi Stuart,

Thanks for reporting this issue.

It does indeed look like a regression/side-effect of that recent
change.
There is no sign of your attached patch, maybe it got stripped by
mailman. You can probably just post it inline.

Thanks,
-Chris.

On 03/02/2013 08:18 AM, Stuart Douglas wrote:

Hello everyone,

(This is my first post to this list, so I am not 100% sure if this
is
the right place).

I have run into a bug in the JDK HttpClient that was caused by a
recent
commit:

http://hg.openjdk.java.net/jdk7u/jdk7u/jdk/diff/e6dc1d9bc70b/src/share/classes/sun/net/www/http/HttpClient.java 





The method sun.net.www.http.HttpClient#available() can throw
java.net.SocketException, which can cause the creation of the
HttpClient
to fail. This will happen if there is a connection in the cache
that has
timed out and the socket has been closed.

I have attached a small patch that should resolve the problem,
although
due to its intermittent nature I have not been able to verify with
100%
certainty.


The exception is:

Caused by: java.net.SocketException: Socket Closed
at
java.net.PlainSocketImpl.getOption(PlainSocketImpl.java:286)
at java.net.Socket.getSoTimeout(Socket.java:1032)
at sun.net.www.http.HttpClient.available(HttpClient.java:356)
at sun.net.www.http.HttpClient.New(HttpClient.java:273)
at sun.net.www.http.HttpClient.New(HttpClient.java:310)
at
sun.net.www.protocol.http.HttpURLConnection.getNewHttpClient(HttpURLConnection.java:987) 




at
sun.net.www.protocol.http.HttpURLConnection.plainConnect(HttpURLConnection.java:923) 




at
sun.net.www.protocol.http.HttpURLConnection.connect(HttpURLConnection.java:841) 




at
sun.net.www.protocol.http.HttpURLConnection.getOutputStream(HttpURLConnection.java:1031) 




at
org.apache.cxf.transport.http.HTTPConduit$WrappedOutputStream.handleHeadersTrustCaching(HTTPConduit.java:1410) 






This issue also seems to affect JDK 6, and it appears that other
people
have also run into this:
http://stackoverflow.com/questions/14270311/rather-mysterious-socketexception-with-java-1-6-on-centos-4 





I attempted to file a bug for this on the Oracle website, however
as far
as I can tell it was ignored.

Stuart

RFR: 8009251 - Add proxy handling and keep-alive fixes to jsse

[Rob McKenna]

Hi folks,

I'd like to port some changes around proxy handling, keep alive and 
tunnelling to the HttpsClient. These should have been done along with 
the HttpClient. Webrev is at:


http://cr.openjdk.java.net/~robm/8009251/webrev.01/

related issues:

https://jbs.oracle.com/bugs/browse/JDK-7199219
https://jbs.oracle.com/bugs/browse/JDK-7199862

Thanks,

-Rob

hg: jdk8/tl/jdk: 8005618: TEST_BUG: java/lang/ProcessBuilder/Basic.java failing intermittently

[rob . mckenna]

Changeset: 44d6cabc9a3f
Author:robm
Date:  2013-01-15 19:58 +
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/44d6cabc9a3f

8005618: TEST_BUG: java/lang/ProcessBuilder/Basic.java failing intermittently
Reviewed-by: alanb, martin, dholmes

! test/java/lang/ProcessBuilder/Basic.java

Re: Request for review: 8000525: Java.net.httpcookie api does not support 2-digit year format

[Rob McKenna]

Hi Chris,

I guess I figured if the parse failed the cal.set wouldn't happen. 
Still, no harm in moving the 1970 into the loop on the off chance 
something else goes wrong.


I've updated the test too. Thanks for spotting that.

-Rob

On 10/12/12 16:59, Chris Hegarty wrote:

Shouldn't 'cal.set(1970, 0, 1, 0, 0, 0)' be inside the for loop?

The test can simply throw Exception, rather can catching.

Otherwise, looks fine to me.

-Crhis.

On 06/12/2012 21:19, Rob McKenna wrote:

Hi folks,

According to HttpCookie.java:


There are 3 http cookie specifications:

Netscape draft
RFC 2109 -/http://www.ietf.org/rfc/rfc2109.txt/
RFC 2965 -/http://www.ietf.org/rfc/rfc2965.txt/

HttpCookie class can accept all these 3 forms of syntax.


According to http://www.ietf.org/rfc/rfc2109.txt section 10.1.2:


Netscape's original proposal defined an Expires header that took a date
value in a fixed-length variant format in place of Max-Age: Wdy,
DD-Mon-YY HH:MM:SS GMT


Thats in the Historical section provided to allow for compatibility
with Netscape's implementation, which we also support: (as per
http://docs.oracle.com/javase/6/docs/api/java/net/HttpCookie.html )

While we don't support the rfc explicitly, this change follows the
format specified in section 5.1.1 of rfc 6265:


3. If the year-value is greater than or equal to 70 and less than or
equal to 99, increment the year-value by 1900.
4. If the year-value is greater than or equal to 0 and less than or
equal to 69, increment the year-value by 2000.
1. NOTE: Some existing user agents interpret two-digit years 
differently.



The webrev is at:

http://cr.openjdk.java.net/~robm/8000525/webrev.01/
http://cr.openjdk.java.net/%7Erobm/8000525/webrev.01/

Note: The addition of setLenient(false) has required changes to two
existing testcases.

-Rob

Re: Request for review: 8000525: Java.net.httpcookie api does not support 2-digit year format

[Rob McKenna]

..the url would be helpful:
http://cr.openjdk.java.net/~robm/8000525/webrev.02/

-Rob
On 12/12/12 15:43, Rob McKenna wrote:

Hi Chris,

I guess I figured if the parse failed the cal.set wouldn't happen. 
Still, no harm in moving the 1970 into the loop on the off chance 
something else goes wrong.


I've updated the test too. Thanks for spotting that.

-Rob

On 10/12/12 16:59, Chris Hegarty wrote:

Shouldn't 'cal.set(1970, 0, 1, 0, 0, 0)' be inside the for loop?

The test can simply throw Exception, rather can catching.

Otherwise, looks fine to me.

-Crhis.

On 06/12/2012 21:19, Rob McKenna wrote:

Hi folks,

According to HttpCookie.java:


There are 3 http cookie specifications:

Netscape draft
RFC 2109 -/http://www.ietf.org/rfc/rfc2109.txt/
RFC 2965 -/http://www.ietf.org/rfc/rfc2965.txt/

HttpCookie class can accept all these 3 forms of syntax.


According to http://www.ietf.org/rfc/rfc2109.txt section 10.1.2:


Netscape's original proposal defined an Expires header that took a date
value in a fixed-length variant format in place of Max-Age: Wdy,
DD-Mon-YY HH:MM:SS GMT


Thats in the Historical section provided to allow for compatibility
with Netscape's implementation, which we also support: (as per
http://docs.oracle.com/javase/6/docs/api/java/net/HttpCookie.html )

While we don't support the rfc explicitly, this change follows the
format specified in section 5.1.1 of rfc 6265:


3. If the year-value is greater than or equal to 70 and less than or
equal to 99, increment the year-value by 1900.
4. If the year-value is greater than or equal to 0 and less than or
equal to 69, increment the year-value by 2000.
1. NOTE: Some existing user agents interpret two-digit years 
differently.



The webrev is at:

http://cr.openjdk.java.net/~robm/8000525/webrev.01/
http://cr.openjdk.java.net/%7Erobm/8000525/webrev.01/

Note: The addition of setLenient(false) has required changes to two
existing testcases.

-Rob

hg: jdk8/tl/jdk: 8004337: java/sql tests aren't run in test/Makefile

[rob . mckenna]

Changeset: 68374c6e65c1
Author:robm
Date:  2012-12-12 15:57 +
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/68374c6e65c1

8004337: java/sql tests aren't run in test/Makefile
Reviewed-by: lancea, alanb

! test/Makefile

hg: jdk8/tl/jdk: 8003597: TEST_BUG: Eliminate dependency on javaweb from closed net tests

[rob . mckenna]

Changeset: 40311b5f478f
Author:robm
Date:  2012-11-28 00:47 +
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/40311b5f478f

8003597: TEST_BUG: Eliminate dependency on javaweb from closed net tests
Reviewed-by: chegar

+ test/java/net/ResponseCache/Test.java
+ test/java/net/Socket/B6210227.java

request for review: 8003597: TEST_BUG: Eliminate dependency on javaweb from closed net tests

[Rob McKenna]

Hi folks,

Looking to move a pair of tests into the open repo:

http://cr.openjdk.java.net/~robm/8003597/webrev.01/

Thanks,

-Rob

hg: jdk8/tl/jdk: 8000487: Java JNDI connection library on ldap conn is not honoring configured timeout

[rob . mckenna]

Changeset: c0736b62160e
Author:robm
Date:  2012-10-15 22:34 +0100
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/c0736b62160e

8000487: Java JNDI connection library on ldap conn is not honoring configured 
timeout
Reviewed-by: vinnie

! src/share/classes/com/sun/jndi/ldap/Connection.java
! src/share/classes/com/sun/jndi/ldap/LdapClient.java
+ test/com/sun/jndi/ldap/LdapTimeoutTest.java
- test/com/sun/jndi/ldap/LdapsReadTimeoutTest.java
- test/com/sun/jndi/ldap/ReadTimeoutTest.java

hg: jdk8/tl/jdk: 7152183: TEST_BUG: java/lang/ProcessBuilder/Basic.java failing intermittently [sol]

[rob . mckenna]

Changeset: 7c2f5e52863c
Author:robm
Date:  2012-10-11 18:24 +0100
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/7c2f5e52863c

7152183: TEST_BUG: java/lang/ProcessBuilder/Basic.java failing intermittently 
[sol]
Reviewed-by: alanb, martin, dholmes

! test/java/lang/ProcessBuilder/Basic.java

hg: jdk8/tl/jdk: 7184932: Remove the temporary Selector usage in the NIO socket adapters

[rob . mckenna]

Changeset: bba370caafad
Author:robm
Date:  2012-10-04 19:53 +0100
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/bba370caafad

7184932: Remove the temporary Selector usage in the NIO socket adapters
Reviewed-by: alanb

! make/java/nio/mapfile-bsd
! make/java/nio/mapfile-linux
! make/java/nio/mapfile-solaris
! src/share/classes/sun/nio/ch/DatagramChannelImpl.java
! src/share/classes/sun/nio/ch/DatagramSocketAdaptor.java
! src/share/classes/sun/nio/ch/Net.java
! src/share/classes/sun/nio/ch/ServerSocketAdaptor.java
! src/share/classes/sun/nio/ch/ServerSocketChannelImpl.java
! src/share/classes/sun/nio/ch/SocketAdaptor.java
! src/share/classes/sun/nio/ch/SocketChannelImpl.java
! src/share/classes/sun/nio/ch/Util.java
! src/solaris/native/sun/nio/ch/Net.c
! src/windows/native/sun/nio/ch/Net.c
+ test/java/nio/channels/etc/AdaptorCloseAndInterrupt.java

RFR: 7199862 7199219 - jdk7u

[Rob McKenna]

Hi folks,

Lumping these backports together.

7199219: Proxy-Connection headers set incorrectly when a HttpClient is 
retrieved from the Keep Alive Cache
7199862: Make sure that a connection is still alive when retrieved from 
KeepAliveCache in certain cases


Webrevs at:
http://cr.openjdk.java.net/~robm/7199862/7/webrev.01/ 
http://cr.openjdk.java.net/%7Erobm/7199862/7/webrev.01/
http://cr.openjdk.java.net/~robm/7199219/7/webrev.01/ 
http://cr.openjdk.java.net/%7Erobm/7199219/7/webrev.01/


Cheers,

-Rob

hg: jdk8/tl/jdk: 7199862: Make sure that a connection is still alive when retrieved from KeepAliveCache in certain cases

[rob . mckenna]

Changeset: 11a5da68673c
Author:robm
Date:  2012-09-27 22:35 +0100
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/11a5da68673c

7199862: Make sure that a connection is still alive when retrieved from 
KeepAliveCache in certain cases
Reviewed-by: chegar

! src/share/classes/sun/net/www/http/HttpClient.java
! src/share/classes/sun/net/www/protocol/http/HttpURLConnection.java

hg: jdk8/tl/jdk: 7199219: Proxy-Connection headers set incorrectly when a HttpClient is retrieved from the Keep Alive Cache

[rob . mckenna]

Changeset: b3c7a3138c5d
Author:robm
Date:  2012-09-28 04:39 +0100
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/b3c7a3138c5d

7199219: Proxy-Connection headers set incorrectly when a HttpClient is 
retrieved from the Keep Alive Cache
Reviewed-by: chegar

! src/share/classes/sun/net/www/http/HttpClient.java
! src/share/classes/sun/net/www/protocol/http/HttpURLConnection.java

request for review: 7199219: Proxy-Connection headers set incorrectly when a HttpClient is retrieved from the Keep Alive Cache

[Rob McKenna]

Hi folks,

Looking for a review for:

7199219: Proxy-Connection headers set incorrectly when a HttpClient is 
retrieved from the Keep Alive Cache


http://cr.openjdk.java.net/~robm/7199219/webrev.01/ 
http://cr.openjdk.java.net/%7Erobm/7199219/webrev.01/


Basically we never set the correct TunnelState after retrieving a 
connection from the KeepAliveCache. This leads to incorrect connection 
headers being set in the request. (in HttpURLConnection.writeRequests())


-Rob

RFR: 7199862: Make sure that a connection is still alive when retrieved from KeepAliveCache in certain cases

[Rob McKenna]

Hi folks,

This one is connected to 7199219 and has an intersecting change in that 
we're similarly passing the httpuc from HttpURLConnection. Despite the 
intersection I've filed these as two separate issues because a) they are 
and b) I wouldn't like one to be held up by the other.


The problem here is that for a specific type of request (streaming POST) 
we don't have the option of retrying the request if there is an issue 
with the connection. A customer has run into a problem where their proxy 
was unilaterally deciding when to close connections (without paying 
attention to the servers keep-alive timeout) earlier than our 
KeepAliveCache expected. In this case our POST would fail with odd http 
response codes.


http://cr.openjdk.java.net/~robm/7199862/8/webrev.01/ 
http://cr.openjdk.java.net/%7Erobm/7199862/8/webrev.01/


-Rob

hg: jdk8/tl/jdk: 6931128: (spec) File attribute tests fail when run as root.

[rob . mckenna]

Changeset: da14e2c90bcb
Author:robm
Date:  2012-08-15 22:46 +0100
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/da14e2c90bcb

6931128: (spec) File attribute tests fail when run as root.
Reviewed-by: alanb

! src/share/classes/java/io/File.java
! test/java/io/File/Basic.java
! test/java/io/File/SetAccess.java
! test/java/io/File/SetReadOnly.java
! test/java/io/File/SymLinks.java
+ test/java/io/File/Util.java

hg: jdk8/tl/jdk: 7179305: (fs) Method name sun.nio.fs.UnixPath.getPathForExecptionMessage is misspelled

[rob . mckenna]

Changeset: 516e0c884af2
Author:robm
Date:  2012-07-09 22:26 +0100
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/516e0c884af2

7179305: (fs) Method name sun.nio.fs.UnixPath.getPathForExecptionMessage is 
misspelled
Reviewed-by: dholmes, chegar

! src/solaris/classes/sun/nio/fs/LinuxUserDefinedFileAttributeView.java
! src/solaris/classes/sun/nio/fs/LinuxWatchService.java
! src/solaris/classes/sun/nio/fs/SolarisAclFileAttributeView.java
! src/solaris/classes/sun/nio/fs/SolarisUserDefinedFileAttributeView.java
! src/solaris/classes/sun/nio/fs/SolarisWatchService.java
! src/solaris/classes/sun/nio/fs/UnixCopyFile.java
! src/solaris/classes/sun/nio/fs/UnixException.java
! src/solaris/classes/sun/nio/fs/UnixFileSystemProvider.java
! src/solaris/classes/sun/nio/fs/UnixPath.java

hg: jdk8/tl/jdk: 7174887: Deadlock in jndi ldap connection cleanup

[rob . mckenna]

Changeset: ecc5dd3790a1
Author:robm
Date:  2012-07-02 19:32 +0100
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/ecc5dd3790a1

7174887: Deadlock in jndi ldap connection cleanup
Reviewed-by: xuelei

! src/share/classes/com/sun/jndi/ldap/Connection.java
! src/share/classes/com/sun/jndi/ldap/LdapClient.java

hg: jdk8/tl/jdk: 4244896: (process) Provide System.getPid(), System.killProcess(String pid)

[rob . mckenna]

Changeset: ff0da4ea08a2
Author:robm
Date:  2012-06-26 13:27 +0100
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/ff0da4ea08a2

4244896: (process) Provide System.getPid(), System.killProcess(String pid)
Reviewed-by: alanb

! src/share/classes/java/lang/Process.java
! src/solaris/classes/java/lang/UNIXProcess.java.bsd
! src/solaris/classes/java/lang/UNIXProcess.java.linux
! src/solaris/classes/java/lang/UNIXProcess.java.solaris
! src/solaris/native/java/lang/UNIXProcess_md.c
! src/windows/classes/java/lang/ProcessImpl.java
! src/windows/native/java/lang/ProcessImpl_md.c
! test/java/lang/ProcessBuilder/Basic.java
+ test/java/lang/ProcessBuilder/DestroyTest.java

hg: jdk8/tl/jdk: 7161881: (dc) DatagramChannel.bind(null) fails if IPv4 socket and running with preferIPv6Addresses=true

[rob . mckenna]

Changeset: a7895dc61088
Author:robm
Date:  2012-06-08 18:23 +0100
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/a7895dc61088

7161881: (dc) DatagramChannel.bind(null) fails if IPv4 socket and running with 
preferIPv6Addresses=true
Reviewed-by: alanb, chegar

! src/share/classes/sun/nio/ch/DatagramChannelImpl.java
+ test/java/nio/channels/DatagramChannel/BindNull.java

hg: jdk8/tl/jdk: 7168110: Misleading jstack error message

[rob . mckenna]

Changeset: 178c480998b1
Author:robm
Date:  2012-05-17 22:42 +0100
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/178c480998b1

7168110: Misleading jstack error message
Reviewed-by: alanb, dsamersoff

! src/windows/native/sun/tools/attach/WindowsVirtualMachine.c

hg: jdk8/tl/jdk: 7166687: InetAddress.getLocalHost().getHostName() returns FQDN

[rob . mckenna]

Changeset: b26c04717735
Author:robm
Date:  2012-05-07 13:34 +0100
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/b26c04717735

7166687: InetAddress.getLocalHost().getHostName() returns FQDN
Reviewed-by: chegar

! src/solaris/native/java/net/Inet6AddressImpl.c