Re: RFR 8004675: Inet6Address.getHostAddress should use string scope identifier where available
On 12/10/2012 09:06 PM, Dmitry Samersoff wrote: Chris, Looks good for me. Thank you Dmitry, PS: Inet6Address.java: It's not necessary to remove explicit initializations - compiler do it perfectly for you. This is a pet peeve of mine. I don't like to see them in code. javac generates extra bytecodes to initialize these fields, and in most cases it is completely unnecessary. Just take a look at the output of a simple test. -Chris. -Dmitry On 2012-12-10 20:01, Chris Hegarty wrote: Inet6Address.getHostAddress() is specified to return the IP address string in textual presentation, followed by a '%' character and the scope identifier. This scope identifier can be either a numeric value or a string, depending on how the instance was created (if it was created with a scoped interface). This change proposes to remove the boolean field, 'scope_ifname_set', since it is not always correctly set when the instance contains a valid scoped interface. For example, when iterating over the NetworkInterface's on the system. 'scope_ifname_set' was never accessed from native code, so it can simply be removed. http://cr.openjdk.java.net/~chegar/8004675/webrev.00/webrev/ -Chris.
Re: reviews for 7200720 and 8003948 [7u-dev]
On 10/12/12 20:48, Dmitry Samersoff wrote: Michael, On 2012-12-10 23:35, Michael McMahon wrote: Could I get the following webrevs reviewed please? They are identical changes (except for one small change suggested by Dmitry) to what was done in 8 for the same issues http://cr.openjdk.java.net/~michaelm/7200720.7u-dev/webrev.1/ I see a comment // MMM 7200720 ?? at ll: 202 of NTLMAuthentication.java - Does it make sense to change it to something more verbose? Otherwise looks good for me. Yes, I'll change that Dmitry. Thanks! Michael http://cr.openjdk.java.net/~michaelm/8003948.7u-dev/webrev.1/ Looks good for me. -Dmitry
Re: RFR 8004675: Inet6Address.getHostAddress should use string scope identifier where available
On 10/12/12 16:01, Chris Hegarty wrote: Inet6Address.getHostAddress() is specified to return the IP address string in textual presentation, followed by a '%' character and the scope identifier. This scope identifier can be either a numeric value or a string, depending on how the instance was created (if it was created with a scoped interface). This change proposes to remove the boolean field, 'scope_ifname_set', since it is not always correctly set when the instance contains a valid scoped interface. For example, when iterating over the NetworkInterface's on the system. 'scope_ifname_set' was never accessed from native code, so it can simply be removed. http://cr.openjdk.java.net/~chegar/8004675/webrev.00/webrev/ -Chris. Looks fine. Michael
Re: Infinite Loop in KeepAliveStream
Hi Martin,
Thank you for reporting this issue. I filed 8004863: "Infinite Loop in
KeepAliveStream", to track it.
I put together a small test to reproduce the problem (inline below). It
is racey, but shows the problem most of the time on my machine.
I tried your suggested patch, but found that there were cases where not
enough data was being read off the stream, when it was being closed.
This causes a problem for subsequent requests on that stream. The
suggested patch below resolves this, and should also resolve the problem
you are seeing ( patch against jdk8 ).
Is this something that you want to run with, or would you prefer for me
to get it into jdk8?
diff -r fda257689786 src/share/classes/sun/net/www/http/KeepAliveStream.java
--- a/src/share/classes/sun/net/www/http/KeepAliveStream.java Mon Dec 10
10:52:11 2012 +0900
+++ b/src/share/classes/sun/net/www/http/KeepAliveStream.java Tue Dec 11
15:30:50 2012 +
@@ -83,10 +83,9 @@ class KeepAliveStream extends MeteredStr
if (expected > count) {
long nskip = expected - count;
if (nskip <= available()) {
-long n = 0;
-while (n < nskip) {
-nskip = nskip - n;
-n = skip(nskip);
+while (nskip > 0) {
+skip(nskip);
+nskip = expected - count;
}
---
/*
* Copyright (c) 2012, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License version 2 only, as
* published by the Free Software Foundation.
*
* This code is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
* version 2 for more details (a copy is included in the LICENSE file that
* accompanied this code).
*
* You should have received a copy of the GNU General Public License
version
* 2 along with this work; if not, write to the Free Software Foundation,
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
*
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
* or visit www.oracle.com if you need additional information or have any
* questions.
*/
/*
* @test
* @bug XX
* @summary
*/
import com.sun.net.httpserver.HttpExchange;
import com.sun.net.httpserver.HttpHandler;
import com.sun.net.httpserver.HttpServer;
import java.io.*;
import java.net.*;
import java.util.concurrent.Phaser;
// Racey test, will not always fail, but if it does then we have a problem.
public class InfiniteLoop {
public static void main(String[] args) throws Exception {
HttpServer server = HttpServer.create(new InetSocketAddress(0), 0);
server.createContext("/test/InfiniteLoop", new RespHandler());
server.start();
try {
InetSocketAddress address = server.getAddress();
URL url = new URL("http://localhost:"; + address.getPort()
+ "/test/InfiniteLoop");
final Phaser phaser = new Phaser(2);
for (int i=0; i<10; i++) {
HttpURLConnection uc =
(HttpURLConnection)url.openConnection();
final InputStream is = uc.getInputStream();
final Thread thread = new Thread() {
public void run() {
try {
phaser.arriveAndAwaitAdvance();
while (is.read() != -1)
Thread.sleep(50);
} catch (Exception x) { x.printStackTrace(); }
}};
thread.start();
phaser.arriveAndAwaitAdvance();
is.close();
System.out.println("returned from close");
thread.join();
}
} finally {
server.stop(0);
}
}
static class RespHandler implements HttpHandler {
static final int RESP_LENGTH = 32 * 1024;
@Override
public void handle(HttpExchange t) throws IOException {
InputStream is = t.getRequestBody();
byte[] ba = new byte[8192];
while(is.read(ba) != -1);
t.sendResponseHeaders(200, RESP_LENGTH);
try (OutputStream os = t.getResponseBody()) {
os.write(new byte[RESP_LENGTH]);
}
t.close();
}
}
}
---
-Chris.
On 12/11/2012 01:21 AM, Martin Buchholz wrote:
Hi sun/net/www maintainers,
Here at Google we have observed an infinite loop
in jdk/src/share/classes/sun/net/www/http/KeepAliveStream.java
85: if (nskip <= available()) {
Re: RFR 8004675: Inet6Address.getHostAddress should use string scope identifier where available
On 12/10/2012 07:37 PM, Kurchi Hazra wrote: Looks good to me. Thanks Kurchi. Not related to this bug, but do we need scope_id_set then? From what I infer, scope_id_set is being set in native code, only when scope_id is not 0, and so a check with scope_id == 0 can serve the purpose of scope_id_set too. You are right. 'scope_id_set' seems to be used in a similar manner. Unfortunately, there seems to be at least on path where a scope_id of 0 is allowable, the package private construction "Inet6Address(String,byte[],int). For now, I'd like to leave this. It will require further careful analysis to determine if there is a possible visible impact of making such a change. Thanks for the detailed review. -Chris. Thanks, Kurchi On 10.12.2012 08:01, Chris Hegarty wrote: Inet6Address.getHostAddress() is specified to return the IP address string in textual presentation, followed by a '%' character and the scope identifier. This scope identifier can be either a numeric value or a string, depending on how the instance was created (if it was created with a scoped interface). This change proposes to remove the boolean field, 'scope_ifname_set', since it is not always correctly set when the instance contains a valid scoped interface. For example, when iterating over the NetworkInterface's on the system. 'scope_ifname_set' was never accessed from native code, so it can simply be removed. http://cr.openjdk.java.net/~chegar/8004675/webrev.00/webrev/ -Chris.
hg: jdk8/tl/langtools: 8004828: refactor init of *DocImpl classes
Changeset: cfde9737131e Author:jjg Date: 2012-12-11 15:05 -0800 URL: http://hg.openjdk.java.net/jdk8/tl/langtools/rev/cfde9737131e 8004828: refactor init of *DocImpl classes Reviewed-by: darcy ! src/share/classes/com/sun/tools/javadoc/AnnotationTypeDocImpl.java ! src/share/classes/com/sun/tools/javadoc/AnnotationTypeElementDocImpl.java ! src/share/classes/com/sun/tools/javadoc/ClassDocImpl.java ! src/share/classes/com/sun/tools/javadoc/ConstructorDocImpl.java ! src/share/classes/com/sun/tools/javadoc/DocEnv.java ! src/share/classes/com/sun/tools/javadoc/DocImpl.java ! src/share/classes/com/sun/tools/javadoc/ExecutableMemberDocImpl.java ! src/share/classes/com/sun/tools/javadoc/FieldDocImpl.java ! src/share/classes/com/sun/tools/javadoc/JavadocEnter.java ! src/share/classes/com/sun/tools/javadoc/JavadocMemberEnter.java ! src/share/classes/com/sun/tools/javadoc/MemberDocImpl.java ! src/share/classes/com/sun/tools/javadoc/MethodDocImpl.java ! src/share/classes/com/sun/tools/javadoc/PackageDocImpl.java ! src/share/classes/com/sun/tools/javadoc/ProgramElementDocImpl.java ! src/share/classes/com/sun/tools/javadoc/RootDocImpl.java
hg: jdk8/tl/jdk: 8003246: Add InitialValue Supplier to ThreadLocal
Changeset: c4bd81de2868 Author:akhil Date: 2012-12-11 15:33 -0800 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/c4bd81de2868 8003246: Add InitialValue Supplier to ThreadLocal Reviewed-by: mduigou, forax, dl, chegar, briangoetz ! src/share/classes/java/lang/ThreadLocal.java + test/java/lang/ThreadLocal/ThreadLocalSupplierTest.java
Code review: 8004904: Makefile for ntlm (was Re: com/sun/security/ntlm)
Hi Chris/Mark I've added a makefile, please take a look http://cr.openjdk.java.net/~weijun/8004904/webrev.00/ *build-dev* guys, I guess this won't affect the new build style. Thanks Max On 12/12/2012 09:28 AM, Weijun Wang wrote: On 12/12/2012 01:50 AM, Chris Hegarty wrote: Max, Mark is looking to recompile com/sun/security/ntlm/Client.java during an incremental build. I cannot find explicit targets for the com/sun/security/ntlm files, are these compiled implicitly by reference from other source? Is there a way of forcing these to be recompiled? I admit, it is strange to me to find a complete package without some kind of explicit reference in the makefiles, but maybe I just cannot find it! No, you cannot find it. There was no Makefile when those files were added: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/9be643e70f42 Thanks Max -Chris. On 12/11/2012 05:36 PM, Mark Sheppard wrote: Client.java I modified files in other parts of jdk under sun.net and these were rebuilt regards Mark - Original Message - From: chris.hega...@oracle.com To: mark.shepp...@oracle.com Sent: Tuesday, December 11, 2012 5:32:32 PM GMT +00:00 GMT Britain, Ireland, Portugal Subject: Re: com/sun/security/ntlm What's the actual filename? -Chris. On 12/11/2012 05:15 PM, Mark Sheppard wrote: Hi Chris, I need some extra debug and modified a few files e.g. com.sun.security.ntlm.Client.java. But the modified files do not seem to have been rebuilt. Can you think of anything obvious that I may have overlooked. How can I force a rebuild of this component - partial build regards Mark
hg: jdk8/tl/jdk: 8004905: Correct license of test to remove classpath exception
Changeset: 6c795437f212 Author:mduigou Date: 2012-12-11 20:49 -0800 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/6c795437f212 8004905: Correct license of test to remove classpath exception Reviewed-by: akhil ! test/java/lang/ThreadLocal/ThreadLocalSupplierTest.java
