Re: RFR: 8157965 update httpserver logging to use java.lang.System.Logger
> On 19 Oct 2016, at 16:29, Daniel Fuchs wrote: > > Thanks Roger! > > On 19/10/16 16:01, Roger Riggs wrote: >> Hi Daniel, >> >> looks fine. >> >> (The style differences are disconcerting but are consistent within the >> file). > > Yes. I preserved the original style though it was hurting > my eyes ;-) Having a mixed style was much worse. Looks good Daniel. Maybe worth a followup to run these files through a reasonable IDE reformatter. -Chris. > best regards, > > -- daniel > >> >> Roger >> >> >> On 10/19/2016 10:52 AM, Daniel Fuchs wrote: >>> Hi, >>> >>> Please find below a patch that updates jdk.httpserver to use >>> System.Logger. >>> >>> As a result jdk.httpserver no longer requires java.logging. >>> >>> webrev: http://cr.openjdk.java.net/~dfuchs/webrev_8157965/webrev.00 >>> issue: https://bugs.openjdk.java.net/browse/JDK-8157965 >>> >>> best regards, >>> >>> -- daniel >> >
JDK 9 RFR of JDK-8146257: sun/net/www/protocol/jar/B4957695.java fails intermittently with java.lang.RuntimeException: some jar_cache files left behind
sun/net/www/protocol/jar/B4957695.java
This test is to verify that no jar_cache tmpFile left in temp directory
when IOException occurs.
Test do stream reading and trigger IOException and compare the jar_cache
file number before and after this action (that triggered the IOException):
String tmpdir = System.getProperty("java.io.tmpdir");
String[] list1 = listTmpFiles(tmpdir);
… // do stream reading, trigger IOException
String[] list2 = listTmpFiles(tmpdir);
if (!sameList (list1, list2)) {
throw new RuntimeException ("some jar_cache files left
behind");
}
This does not work well when tests run concurrently, the jar_cache file
under tmpdir may from other tests.
Please review the patch to set "java.io.tmpdir" to current working dir
to avoid test be affected by other tests.
bug: https://bugs.openjdk.java.net/browse/JDK-8146257
webrev: http://cr.openjdk.java.net/~amlu/8146257/webrev.00/
Thanks,
Amy
--- old/test/sun/net/www/protocol/jar/B4957695.java 2016-10-20
17:04:00.0 +0800
+++ new/test/sun/net/www/protocol/jar/B4957695.java 2016-10-20
17:04:00.0 +0800
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2003, 2012, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2003, 2016, 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
@@ -24,6 +24,7 @@
/**
* @test
* @bug 4957695
+ * @run main/othervm -Djava.io.tmpdir=. B4957695
* @summary URLJarFile.retrieve does not delete tmpFile on IOException
*/
Re: JDK 9 RFR of JDK-8146257: sun/net/www/protocol/jar/B4957695.java fails intermittently with java.lang.RuntimeException: some jar_cache files left behind
> On 20 Oct 2016, at 11:45, Amy Lu wrote:
>
> sun/net/www/protocol/jar/B4957695.java
>
> This test is to verify that no jar_cache tmpFile left in temp directory when
> IOException occurs.
>
> Test do stream reading and trigger IOException and compare the jar_cache file
> number before and after this action (that triggered the IOException):
>
> String tmpdir = System.getProperty("java.io.tmpdir");
> String[] list1 = listTmpFiles(tmpdir);
> … // do stream reading, trigger IOException
> String[] list2 = listTmpFiles(tmpdir);
> if (!sameList (list1, list2)) {
> throw new RuntimeException ("some jar_cache files left behind");
> }
>
> This does not work well when tests run concurrently, the jar_cache file under
> tmpdir may from other tests.
>
> Please review the patch to set "java.io.tmpdir" to current working dir to
> avoid test be affected by other tests.
>
> bug: https://bugs.openjdk.java.net/browse/JDK-8146257
> webrev: http://cr.openjdk.java.net/~amlu/8146257/webrev.00/
Thanks Amy, I think this change should help isolate the test from
other interference in /tmp.
-Chris.
> Thanks,
> Amy
>
>
> --- old/test/sun/net/www/protocol/jar/B4957695.java 2016-10-20
> 17:04:00.0 +0800
> +++ new/test/sun/net/www/protocol/jar/B4957695.java 2016-10-20
> 17:04:00.0 +0800
> @@ -1,5 +1,5 @@
> /*
> - * Copyright (c) 2003, 2012, Oracle and/or its affiliates. All rights
> reserved.
> + * Copyright (c) 2003, 2016, 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
> @@ -24,6 +24,7 @@
> /**
> * @test
> * @bug 4957695
> + * @run main/othervm -Djava.io.tmpdir=. B4957695
> * @summary URLJarFile.retrieve does not delete tmpFile on IOException
> */
>
>
>
RFR 8168405: Pending exceptions in java.base/windows/native
Hello, Could you please review the following change for [1]? http://cr.openjdk.java.net/~prappo/8168405/webrev/ This change addresses some code paths in the native networking code for Windows operating system where thrown exceptions might be left unnoticed. Thanks, -Pavel [1] https://bugs.openjdk.java.net/browse/JDK-8168405
RE: RFR 8168405: Pending exceptions in java.base/windows/native
Hi Pavel, overall this looks good. I've got a few minor remarks: 1. What about using the macro CHECK_NULL_RETURN in NetworkInterface_winXP.c? 2. in Java_java_net_TwoStacksPlainDatagramSocketImpl_peekData: You could move 1178 /* make sure receive() picks up the right fd */ 1179 (*env)->SetIntField(env, this, pdsi_fduseID, fduse); into the else branch above and set port to -1 in the n<0 branches. That way -1 will be returned always and the free(fullPacket) is also invoked if necessary with less lines of code :) But that's probably a matter of taste ;-) Best regards Christoph > -Original Message- > From: Pavel Rappo [mailto:pavel.ra...@oracle.com] > Sent: Donnerstag, 20. Oktober 2016 14:47 > To: OpenJDK Network Dev list > Subject: RFR 8168405: Pending exceptions in java.base/windows/native > > Hello, > > Could you please review the following change for [1]? > >http://cr.openjdk.java.net/~prappo/8168405/webrev/ > > This change addresses some code paths in the native networking code for > Windows > operating system where thrown exceptions might be left unnoticed. > > Thanks, > -Pavel > > > [1] https://bugs.openjdk.java.net/browse/JDK-8168405
Re: RFR 8168405: Pending exceptions in java.base/windows/native
> On 20 Oct 2016, at 14:03, Langer, Christoph wrote:
>
> Hi Pavel,
>
> overall this looks good. I've got a few minor remarks:
>
> 1. What about using the macro CHECK_NULL_RETURN in NetworkInterface_winXP.c?
>
> 2. in Java_java_net_TwoStacksPlainDatagramSocketImpl_peekData:
>
> You could move
> 1178 /* make sure receive() picks up the right fd */
> 1179 (*env)->SetIntField(env, this, pdsi_fduseID, fduse);
> into the else branch above and set port to -1 in the n<0 branches. That way
> -1 will be returned always and the free(fullPacket) is also invoked if
> necessary with less lines of code :) But that's probably a matter of taste ;-)
>
> Best regards
> Christoph
Hi Christoph,
Thanks a lot for looking into this! I would prefer to be a bit more explicit in
both cases. And indeed I considered to do exactly what you described in the 2nd
point, but then I saw this pattern in several different places (just grep it) in
this file:
if (packetBufferLen > MAX_BUFFER_LEN) {
free(fullPacket);
}
So for consistency's sake maybe we should keep it as it is? I appreciate it
looks a bit wordy.
Thanks!
RE: RFR 8168405: Pending exceptions in java.base/windows/native
Hi Pavel,
> > On 20 Oct 2016, at 14:03, Langer, Christoph
> wrote:
> >
> > Hi Pavel,
> >
> > overall this looks good. I've got a few minor remarks:
> >
> > 1. What about using the macro CHECK_NULL_RETURN in
> NetworkInterface_winXP.c?
> >
> > 2. in Java_java_net_TwoStacksPlainDatagramSocketImpl_peekData:
> >
> > You could move
> > 1178 /* make sure receive() picks up the right fd */
> > 1179 (*env)->SetIntField(env, this, pdsi_fduseID, fduse);
> > into the else branch above and set port to -1 in the n<0 branches. That way
> > -1
> will be returned always and the free(fullPacket) is also invoked if necessary
> with less lines of code :) But that's probably a matter of taste ;-)
> >
> > Best regards
> > Christoph
>
> Hi Christoph,
>
> Thanks a lot for looking into this! I would prefer to be a bit more explicit
> in
> both cases. And indeed I considered to do exactly what you described in the
> 2nd
> point, but then I saw this pattern in several different places (just grep it)
> in
> this file:
>
> if (packetBufferLen > MAX_BUFFER_LEN) {
> free(fullPacket);
> }
>
> So for consistency's sake maybe we should keep it as it is? I appreciate it
> looks a bit wordy.
>
> Thanks!
Ok, that's fine for me then. Unfortunately I'm no reviewer...
Best regards
Christoph
RE: Ping: RFR (M): 8167481: cleanup of headers and includes for native libnet
Chris, I created a new version that addresses your points: http://cr.openjdk.java.net/~clanger/webrevs/8167481.1/ I reordered the headers according to your suggestions ( 1) system, 2) platform specific includes, 3) JNI includes, and finally, 4) generated headers ), added the @Native and removed the Windows stuff in net_util_md.h. To make reodering of system includes possible, I had to add back some system headers in a few places and could not always maintain the alphabetical order. I think you should run it through PRT and then hopefully give me your final blessing... Thanks & Best regards Christoph > -Original Message- > From: Chris Hegarty [mailto:chris.hega...@oracle.com] > Sent: Dienstag, 18. Oktober 2016 20:56 > To: Langer, Christoph ; net-dev@openjdk.java.net > Subject: Re: Ping: RFR (M): 8167481: cleanup of headers and includes for > native > libnet > > Christoph, > > > On 17 Oct 2016, at 09:40, Langer, Christoph > wrote: > > … > > Hi, > > > > as I already mentioned, here is another proposal for cleanup in libnet: > > https://bugs.openjdk.java.net/browse/JDK-8167481 > > http://cr.openjdk.java.net/~clanger/webrevs/8167481.0/ > > Overall, this looks good. Specific comments below ... > > > This time I touched the includes. Generally I reordered the includes to > > include > “net_util.h” first, then any JNI includes, such as “java_net_InetAddress.h” > and > finally all standard header includes in alphabetical order. For platform > specific > includes, I use the order Linux, AIX, Solaris, BSD. I removed all includes > that > don’t seem to be necessary to get the respective file compiled. Is that the > correct approach that is desired? > > It is good to be consistent. My personal preference is to order the includes; > 1) system, 2) platform specific includes, 3) JNI includes, and finally, 4) > generated headers ( e.g. “java_net_InetAddress.h” ). > > > To make this work, I had to remove the definitions “IPv4” and “IPv6” in > net_util.h and replace their usages with “java_net_InetAddress_IPv4” resp. > “java_net_InetAddress_IPv6” from JNI which seems to be the correct thing to > do anyway. > > Right. This is a good change. Trivially, and it is not strictly necessary but > good for readability, would be to add @Native to InetAddress.IPv4 and > InetAddress.IPv6. > > > For Windows I also cleaned up (removed) the definition of SOCKADDR_IN6 in > net_util_md.h and replaced all occurences with sockaddr_in6. It seems that the > capital letters version SOCKADDR_IN6 is a remainder of a very old Microsoft > runtime (_MSC_VER < 1310) which corresponds to MSVC 7.0, which is even > older than Visual Studio 2003. So I’m wondering if I should remove the whole > section in windows net_util_md.h now (lines 47 – 177)? > > I think it should be ok to remove this. > > > For Windows it seems that we can also completely get rid of > src/java.base/windows/native/libnet/icmp.h as the icmp stuff is all brought by > the #include . Would you agree to that? > > Ok. > > > So far I successfully ran builds in my local environments for Windows, > > Linux, > AIX, Solaris and Apple. I’ve now added the changeset to our local regression > testing environment for OpenJDK. If the change is to your like, I would like > to > ask you to also run it through JPRT to see if I missed some dependency. > > I ran your patch through our internal build and test system and there > are no surprises. > > -Chris. > > > Thanks and best regards > > Christoph
Re: RFR 8168405: Pending exceptions in java.base/windows/native
> On 20 Oct 2016, at 13:47, Pavel Rappo wrote: > > Hello, > > Could you please review the following change for [1]? > > http://cr.openjdk.java.net/~prappo/8168405/webrev/ Thank you Pavel, this looks good. -Chris. > This change addresses some code paths in the native networking code for > Windows > operating system where thrown exceptions might be left unnoticed. > > Thanks, > -Pavel > > > [1] https://bugs.openjdk.java.net/browse/JDK-8168405 >
