RFR 8006560: java/net/ipv6tests/B6521014.java fails intermittently

2013-01-18 Thread Chris Hegarty
This test can be seen to fail intermittently on a very busy system. The 
test tries to bind to a "hardcoded" (relative to another) port number. I 
see no reason for specify the port number in this testcase. The socket 
needs to be bound to a specific address, but we should be able to 
specify a port of 0 (ephemeral port).


Exception:
  Caused by: java.net.BindException: Address already in use
  at java.net.PlainSocketImpl.socketBind(Native Method)
  at 
java.net.AbstractPlainSocketImpl.bind(AbstractPlainSocketImpl.java:382)

  at java.net.Socket.bind(Socket.java:626)
  at B6521014.test2(B6521014.java:105)
  at B6521014.main(B6521014.java:123)

>: hg diff java/net/ipv6tests/B6521014.java
diff -r a546d8897e0d test/java/net/ipv6tests/B6521014.java
--- a/test/java/net/ipv6tests/B6521014.java Wed Jan 16 12:09:35 2013 
+
+++ b/test/java/net/ipv6tests/B6521014.java Fri Jan 18 14:18:44 2013 
+

@@ -95,14 +95,12 @@ public class B6521014 {
 Socket sock;
 ServerSocket ssock;
 int port;
-int localport;

 ssock = new ServerSocket(0);
 ssock.setSoTimeout(100);
 port = ssock.getLocalPort();
-localport = port + 1;
 sock = new Socket();
-sock.bind(new InetSocketAddress(sin, localport));
+sock.bind(new InetSocketAddress(sin, 0));
 try {
 sock.connect(new InetSocketAddress(sin, port), 100);
 } catch (SocketTimeoutException e) {

-Chris.


Re: Code review request 7183373: URLClassloader.close() does not close JAR files whose resources have been loaded via getResource()

2013-01-18 Thread Chris Hegarty
I haven't been able to spend as much time on this as I would like, jdk8 
M6 code freeze is approaching fast. Since this area is fraught with 
danger the safest change would be what is in the .2 version of the 
webrev [1]. I think I would be ok with this.


-Chris.

[1] http://cr.openjdk.java.net/~shirishk/7183373/webrev.2/

On 18/01/2013 06:54, Frank Ding wrote:

Hi Michael,
Could you please take a look at my comment below?

Best regards,
Frank

On 1/6/2013 4:23 PM, Frank Ding wrote:

Hi Michael,
After reading carefully discussion thread, let me elaborate my
investigation and conclusion.
The 2nd version of Shirish's patch tries to address your concern that
"Would it be possible to fix this within the context of whatever
loader is currently being invoked?". The new solution sticks to using
Loader rather than JarLoader.
The call cl.close() in the jtreg test case, according to its spec
(URLClassLoader.close) should "close any files that were opened by it
in case of jar". Its implementation code shows it closes any opened
resources through api such as getResourceAsStream invoked by client
code but doesn't take care of any resources opened by
findResource(String) or findResources(String). This implies that
findResource should return any resource found but should not leave it
in open state. The key issue for a Loader.findResource() when
searching within a jar file does not follow this rule because the code
combination "InputStream is = url.openStream(); is.close();" (in
URLClassPath.Loader.findResource()) leaves the jar file in open state.
As Shirish pointed out, if useCaches is set to true, the problem is
gone. It can be easily verified from code JarURLInputStream.close()
defined in JarURLConnection.java.
My conclusion is that Shirish's first patch is reasonable (except the
constructor change which I have not fully understood yet) because
choosing a JarLoader avoids unclosed resources after calling
URLClassLoader.getResource() and 2nd patch also makes sense as
explained above. The ramifications of these 2 patches need deliberate
considerations but we still have to fix the issue after weighing their
risks. Could you please shed your light on it?

Best regards,
Frank

On 8/25/2012 12:02 AM, Shirish Kuncolienkar wrote:

On 8/24/2012 5:39 PM, Michael McMahon wrote:

On 23/08/12 18:50, Shirish Kuncolienkar wrote:

Could I get the change reviewed please

This behavior is seen on Windows.
Logic in URLClassPath.getLoader() does not take care of an URL
which looks like "jar:file:/C:/test/xyz.jar!/". The logic ends up
choosing a FileLoader instead of a JarLoader. JarLoader has
provision for closing file handles, so choosing a JarLoader will
solve the problem. Secondly the constructor of JarLoader blindly
adds a prefix and suffix to the provided URL to make it look like a
jar URL. Changed the code here to conditionally append/prepend

The change set can be found at
http://cr.openjdk.java.net/~shirishk/7183373/webrev.0/

-Shirish


Shirish,

I have a slight concern that this would modify the Loader class to
be used in some circumstances
completely independent of the requirements of
URLClassLoader.close(). This is very sensitive code.
Would it be possible to fix this within the context of whatever
loader is currently being invoked?

- Michael


Michael,

Thanks for the review comments. The second version of the fix is
uploaded at http://cr.openjdk.java.net/~shirishk/7183373/webrev.2/
Could you please take a look at this one ?

Description of the fix:
URLClassPath.Loader.findResource() method opens a connection to the
provided URL to test whether the URL is good. Here the Jar file gets
opened but does not get closed because the created stream as
setUseCaches set to true.

Just out of curiosity I would like to know bit more on "some
circumstances completely independent of the requirements of
URLClassLoader.close()". I see that the Loader classes are private in
nature and are being used within the context of the URLClassPath.
We create an instance of JarLoader for all the jars that are on the
extension class loader path by adding "jar" , "!/" to the file url
which comes as the input. The reason behind the first fix was that if
we have a url like this why not use a JarLoader instance.

- Shirish







hg: jdk8/tl/langtools: 8006561: Langtools test failure: missing diags/examples

2013-01-18 Thread maurizio . cimadamore
Changeset: 3d84ae209919
Author:mcimadamore
Date:  2013-01-18 15:38 +
URL:   http://hg.openjdk.java.net/jdk8/tl/langtools/rev/3d84ae209919

8006561: Langtools test failure: missing diags/examples
Summary: forgot to hg add tests
Reviewed-by: jjg

+ test/tools/javac/diags/examples/UnderscoreAsIdentifier.java
+ test/tools/javac/lambda/WarnUnderscoreAsIdent.java
+ test/tools/javac/lambda/WarnUnderscoreAsIdent.out



Re: RFR 8006560: java/net/ipv6tests/B6521014.java fails intermittently

2013-01-18 Thread Kurchi Subhra Hazra


Looks good - I looked into the bug details for the test too, but could 
not find any

reason why we would need server and client to bind to consecutive ports.

Thanks,
- Kurchi

On 1/18/13 6:23 AM, Chris Hegarty wrote:
This test can be seen to fail intermittently on a very busy system. 
The test tries to bind to a "hardcoded" (relative to another) port 
number. I see no reason for specify the port number in this testcase. 
The socket needs to be bound to a specific address, but we should be 
able to specify a port of 0 (ephemeral port).


Exception:
  Caused by: java.net.BindException: Address already in use
  at java.net.PlainSocketImpl.socketBind(Native Method)
  at 
java.net.AbstractPlainSocketImpl.bind(AbstractPlainSocketImpl.java:382)

  at java.net.Socket.bind(Socket.java:626)
  at B6521014.test2(B6521014.java:105)
  at B6521014.main(B6521014.java:123)

>: hg diff java/net/ipv6tests/B6521014.java
diff -r a546d8897e0d test/java/net/ipv6tests/B6521014.java
--- a/test/java/net/ipv6tests/B6521014.java Wed Jan 16 12:09:35 
2013 +
+++ b/test/java/net/ipv6tests/B6521014.java Fri Jan 18 14:18:44 
2013 +

@@ -95,14 +95,12 @@ public class B6521014 {
 Socket sock;
 ServerSocket ssock;
 int port;
-int localport;

 ssock = new ServerSocket(0);
 ssock.setSoTimeout(100);
 port = ssock.getLocalPort();
-localport = port + 1;
 sock = new Socket();
-sock.bind(new InetSocketAddress(sin, localport));
+sock.bind(new InetSocketAddress(sin, 0));
 try {
 sock.connect(new InetSocketAddress(sin, port), 100);
 } catch (SocketTimeoutException e) {

-Chris.




RFR: JDK-8006568 HTTP protocol handler NLTM Authentication should use Base64 API

2013-01-18 Thread Mark Sheppard

Hi,
  as part of the Base64 API refactoring activity a review is requested for
JDK-8006568 changes.

Description:

jdk8 has java.util.Base64 to define standard API. As part of a refactoring 
process to migrate core-libs to use this standard API, the NTLM Authentication 
support should be refactored to use Base64 API.


files affected are:

sun/net/www/protocol/http/ntlm/NTLMAuthentication.java
sun/net/www/protocol/http/ntlm/NTLMAuthSequence.java

and test

test/sun/net/www/protocol/http/ProxyTunnelServer.java

webrev located at:

http://cr.openjdk.java.net/~chegar/8006568/webrev.00/

regards
Mark


Re: RFR: JDK-8006568 HTTP protocol handler NLTM Authentication should use Base64 API

2013-01-18 Thread Chris Hegarty

Looks fine to me Mark.

-Chris.

On 01/18/2013 05:17 PM, Mark Sheppard wrote:


Hi,
   as part of the Base64 API refactoring activity a review is requested for
JDK-8006568 changes.

Description:

jdk8 has java.util.Base64 to define standard API. As part of a refactoring 
process to migrate core-libs to use this standard API, the NTLM Authentication 
support should be refactored to use Base64 API.


files affected are:

sun/net/www/protocol/http/ntlm/NTLMAuthentication.java
sun/net/www/protocol/http/ntlm/NTLMAuthSequence.java

and test

test/sun/net/www/protocol/http/ProxyTunnelServer.java

webrev located at:

http://cr.openjdk.java.net/~chegar/8006568/webrev.00/

regards
Mark



Re: RFR: JDK-8006568 HTTP protocol handler NLTM Authentication should use Base64 API

2013-01-18 Thread Alan Bateman

On 18/01/2013 17:26, Chris Hegarty wrote:

Looks fine to me Mark.

-Chris.


Looks good to me too.

-Alan


hg: jdk8/tl/jdk: 8005120: Compiler warnings in socket transport native code

2013-01-18 Thread chris . hegarty
Changeset: f88e963960ae
Author:jzavgren
Date:  2013-01-18 17:34 +
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/f88e963960ae

8005120: Compiler warnings in socket transport native code
Reviewed-by: chegar, dsamersoff

! src/share/transport/socket/socketTransport.c
! src/share/transport/socket/sysSocket.h
! src/solaris/transport/socket/socket_md.c
! src/windows/transport/socket/socket_md.c



Re: RFR 8006560: java/net/ipv6tests/B6521014.java fails intermittently

2013-01-18 Thread Brad Wetmore
Looks good to me.  Don't know how many of those we've cleaned over the 
years!  ;)


Brad


On 1/18/2013 6:23 AM, Chris Hegarty wrote:

This test can be seen to fail intermittently on a very busy system. The
test tries to bind to a "hardcoded" (relative to another) port number. I
see no reason for specify the port number in this testcase. The socket
needs to be bound to a specific address, but we should be able to
specify a port of 0 (ephemeral port).

Exception:
   Caused by: java.net.BindException: Address already in use
   at java.net.PlainSocketImpl.socketBind(Native Method)
   at
java.net.AbstractPlainSocketImpl.bind(AbstractPlainSocketImpl.java:382)
   at java.net.Socket.bind(Socket.java:626)
   at B6521014.test2(B6521014.java:105)
   at B6521014.main(B6521014.java:123)

 >: hg diff java/net/ipv6tests/B6521014.java
diff -r a546d8897e0d test/java/net/ipv6tests/B6521014.java
--- a/test/java/net/ipv6tests/B6521014.java Wed Jan 16 12:09:35 2013
+
+++ b/test/java/net/ipv6tests/B6521014.java Fri Jan 18 14:18:44 2013
+
@@ -95,14 +95,12 @@ public class B6521014 {
  Socket sock;
  ServerSocket ssock;
  int port;
-int localport;

  ssock = new ServerSocket(0);
  ssock.setSoTimeout(100);
  port = ssock.getLocalPort();
-localport = port + 1;
  sock = new Socket();
-sock.bind(new InetSocketAddress(sin, localport));
+sock.bind(new InetSocketAddress(sin, 0));
  try {
  sock.connect(new InetSocketAddress(sin, port), 100);
  } catch (SocketTimeoutException e) {

-Chris.


hg: jdk8/tl/jdk: 6939260: (fs) BasicFileAttributes.lastModifiedTime() should return last modified time with higher precision

2013-01-18 Thread alan . bateman
Changeset: 06da8d0e
Author:alanb
Date:  2013-01-18 18:48 +
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/06da8d0e

6939260: (fs) BasicFileAttributes.lastModifiedTime() should return last 
modified time with higher precision
Reviewed-by: chegar

! src/solaris/classes/sun/nio/fs/UnixFileAttributes.java
! src/solaris/native/sun/nio/fs/UnixNativeDispatcher.c
! test/java/nio/file/attribute/BasicFileAttributeView/Basic.java