RFR 8006560: java/net/ipv6tests/B6521014.java fails intermittently
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()
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
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
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
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
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
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
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
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
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