October 2017 - core-libs-dev

Review Request JDK-8164512: Replace ClassLoader use of finalizer with phantom reference to unload native library
by mandy chung 10 Oct '17

10 Oct '17

This patch proposes to replace the ClassLoader use of finalizer with phantom reference, specifically Cleaner, for unloading native libraries. It registers the class loader for cleanup only if it's not a builtin class loader which will never be unloaded. The spec for JNI_OnUnload [1] specifies that this function is called in an unknown context whereas the hotspot implementation uses the class loader being unloaded as the context, which I consider a bug. It should not load a class defined to that class loader. The proposed spec change for JNI_FindClass if called from JNI_OnUnload to use system class loader (consistent with no caller context case). Webrev: http://cr.openjdk.java.net/~mchung/jdk10/webrevs/8164512/webrev.00/ CSR: https://bugs.openjdk.java.net/browse/JDK-8187882 Please see the spec change an behavioral change in this CSR. A native library may fail to be reloaded if it is loaded immediately or soon after a class loader is GC'ed but the cleaner thread doesn't yet have the chance to handle the unloading of the native library. Likewise, there is no guarantee regarding the timing of finalization and a native library may fail to reload. It's believed that the compatibility risk should be low. I'm looking into adding a native jtreg test that will be added in the next revision. Mandy [1] http://docs.oracle.com/javase/9/docs/specs/jni/invocation.html#jni_onunload

6 31

[10] RFR(S) 8188775: Module jdk.internal.vm.compiler.management has not been granted accessClassInPackage.org.graalvm.compiler.hotspot
by Vladimir Kozlov 10 Oct '17

10 Oct '17

https://bugs.openjdk.java.net/browse/JDK-8188775 Changes for 8182701[1] missed changes in default.policy for new module jdk.internal.vm.compiler.management. Add missing code: src/java.base/share/lib/security/default.policy @@ -154,6 +154,10 @@ permission java.security.AllPermission; }; +grant codeBase "jrt:/jdk.internal.vm.compiler.management" { + permission java.security.AllPermission; +}; + grant codeBase "jrt:/jdk.jsobject" { permission java.security.AllPermission; }; Verified with failed test. Thanks, Vladimir [1] http://hg.openjdk.java.net/jdk10/hs/rev/8b2054b7d02c

5 7

RE: jdk9/10 reject zip/jar files where seconds value of timestamp is out of supported range 0 - 59
by Baesken, Matthias 09 Oct '17

09 Oct '17

Hi Claes, thanks for your feedback. Hello, I opened a bug : JDK-8188901 jdk9/10 reject zip/jar files where seconds value of timestamp is out of supported range 0 - 59 https://bugs.openjdk.java.net/browse/JDK-8188901 In case the dosToJavaTime change was to use java.time was really just a cleanup, I would like to submit a change and go back to Date, is that ok ? The suggested workaround by Xueming Shen >really wanted to suggest to just unzip/jar & jar those jar files again, >as a workaround ... Is a good idea for some people, however in complex sceanarios people do not even currently get the name of the archive in the exception . So it is an issue to identify what to unpack/pack for them . Another issue might be the usage of signed archives . Best regards, Matthias > > Message: 2 > Date: Fri, 6 Oct 2017 16:02:19 +0200 > From: Claes Redestad <claes.redestad(a)oracle.com> > To: core-libs-dev(a)openjdk.java.net > Subject: Re: jdk9/10 reject zip/jar files where seconds value of > timestamp is out of supported range 0 - 59 > Message-ID: <e915d444-5f5f-0b88-649c-ef3c567d346c(a)oracle.com> > Content-Type: text/plain; charset=utf-8; format=flowed > > Hi, > > it appears this is a difference where java.util.Date is carrying the > overflow(s), while the > java.time-based implementation is more strict.? Same issue assumedly > exist for most > fields in the dos format (minutes can be 0-63, hours 0-31, days 0-31...). > > The dosToJavaTime implementation was changed to use java.time mostly as > a cleanup > (https://bugs.openjdk.java.net/browse/JDK-8066644) but this difference > in strictness was > overlooked.. > > Most compelling option for now might be to revert to the code in 8: > > ??? public static long dosToJavaTime8(long dtime) { > ??????? @SuppressWarnings("deprecation") // Use of date constructor. > ??????? Date d = new Date((int)(((dtime >> 25) & 0x7f) + 80), > ????????????????????????? (int)(((dtime >> 21) & 0x0f) - 1), > ????????????????????????? (int)((dtime >> 16) & 0x1f), > ????????????????????????? (int)((dtime >> 11) & 0x1f), > ????????????????????????? (int)((dtime >> 5) & 0x3f), > ????????????????????????? (int)((dtime << 1) & 0x3e)); > ??????? return d.getTime(); > ??? } > > /Claes > > On 2017-10-06 14:24, Baesken, Matthias wrote: > > Hello, > > > > When iterating on the zip entries (and printing the time stamps) of an old > sqljdbc.jar the exception > > > > "java.time.DateTimeException: Invalid value for SecondOfMinute (valid > values 0 - 59): 60" > > occured (issue has been seen on Windows). > > > > Looks like the dosToJavaTime function in ZipUtils.java of jdk9/jdk10 is a bit > more "sensitive" > > about the input it gets, compared to jdk8. > > In both implementations : > > > > java.base/share/classes/java/util/zip/ZipUtils.java > > jdk.zipfs/share/classes/jdk/nio/zipfs/ZipUtils.java > > > > > > the computation of the "seconds" values is done by > > > > public static long dosToJavaTime(long dtime) { > > ... > > (int) ((dtime << 1) & 0x3e)); > > ... > > } > > > > 0x3e is binary 111110 or decimal 62, so larger values than the supported > maximum 59 can occur > > (maybe only with old/problematic archives but still we had such an > example). > > > > When looking a bit more closely into it, we found that the "second = 60" > value was coming from a long value 930973758 > > that was passed to ZipUtils.dosToJavaTime as dtime argument. > > > > (found this out by adding an enhanced chained exception to jdk9 showing > the dtime value passed to dosToJavaTime). > > > > Here is an example of the iteration on entries of such a "bad" zip file : > > > > C:\JVM\openjdk10\bin\java ZipIterate > > > > Iterate von ZipFile: > > -------------------------------------- > > name of > ZipEntry:com/microsoft/sqlserver/jdbc/AppDTVImpl$SetValueOp.class > > Exception in thread "main" java.time.DateTimeException: Invalid value for > SecondOfMinute (valid values 0 - 59): 60 > > at > java.base/java.time.temporal.ValueRange.checkValidValue(ValueRange.java > :311) > > at > java.base/java.time.temporal.ChronoField.checkValidValue(ChronoField.jav > a:714) > > at java.base/java.time.LocalTime.of(LocalTime.java:322) > > at java.base/java.time.LocalDateTime.of(LocalDateTime.java:337) > > at java.base/java.util.zip.ZipUtils.dosToJavaTime(ZipUtils.java:101) > > at > java.base/java.util.zip.ZipUtils.extendedDosToJavaTime(ZipUtils.java:114) > > at java.base/java.util.zip.ZipEntry.getTime(ZipEntry.java:198) > > at ZipIterate.main(ZipIterate.java:37) > > > > > > > > A similar problem (dealing with days/months not seconds out of supported > range) is described here : > > https://bugs.openjdk.java.net/browse/JDK-8184940 > > "JDK 9 rejects zip files where the modified day or month is 0" > > > > > > Compared to jdk8, this is a regression so I think it would be good to have a > better handling of problematic out of range seconds-values. > > > > Do you think it would be sufficient to adjust the out of range seconds to > the supported max-value 59 ? > > Or is something more sophisticated prefered ? > > I found the enhanced chained exception showing the dtime long values > helpful too, should I submit it ? > > > > Best regards, > > Matthias > > >

5 6

RFR (XS) 8188828 Intermittent ClassNotFoundException: jdk.test.lib.Platform for compiler tests
by Ioi Lam 09 Oct '17

09 Oct '17

Please review this very simple change: https://bugs.openjdk.java.net/browse/JDK-8188828 http://ioilinux.us.oracle.com/webrev/jdk10/8188828_compiler_test_class_not_… The dependency of FileInstaller -> Utils -> JDKToolLauncher -> Platform has caused many intermittent ClassNotFoundException in the hotspot nightly runs. While this fix does not address the root cause (proper dependencies are not specified in the test cases -- which we are planning to fix), we will hopefully see much fewer occurrences of this annoying failure scenario. Thanks a lot to Igor for suggesting the simple fix! - Ioi

5 6

[10] [testbug] RFR of JDK-8187700 SetAuthenticator tests should handle the proxy port
by Frank Yuan 09 Oct '17

09 Oct '17

Hi Daniel and all Would you like to review http://cr.openjdk.java.net/~fyuan/8187700/webrev.00/? Bug: https://bugs.openjdk.java.net/browse/JDK-8187700 I applied the fix for proxy port reusing issue, which was reviewed in bug JDK-8187507. And made a few code cleaning in HTTPSetAuthenticatorTest.java Thanks Frank

2 2

RE: [10] [testbug] RFR of JDK-8187700 SetAuthenticator tests should handle the proxy port
by Frank Yuan 09 Oct '17

09 Oct '17

Hi Daniel and all Would you like to have a look on this review? It's the same issue as JDK-8188086 but in SetAuthenticator tests. The fix's verified by running the test in loop. Thanks Frank From: Frank Yuan [mailto:frank.yuan@oracle.com] Subject: [10] [testbug] RFR of JDK-8187700 SetAuthenticator tests should handle the proxy port Hi Daniel and all Would you like to review http://cr.openjdk.java.net/~fyuan/8187700/webrev.00/? Bug: https://bugs.openjdk.java.net/browse/JDK-8187700 I applied the fix for proxy port reusing issue, which was reviewed in bug JDK-8187507. And made a few code cleaning in HTTPSetAuthenticatorTest.java Thanks Frank

1 0

jdk9/10 reject zip/jar files where seconds value of timestamp is out of supported range 0 - 59
by Baesken, Matthias 06 Oct '17

06 Oct '17

Hello, When iterating on the zip entries (and printing the time stamps) of an old sqljdbc.jar the exception "java.time.DateTimeException: Invalid value for SecondOfMinute (valid values 0 - 59): 60" occured (issue has been seen on Windows). Looks like the dosToJavaTime function in ZipUtils.java of jdk9/jdk10 is a bit more "sensitive" about the input it gets, compared to jdk8. In both implementations : java.base/share/classes/java/util/zip/ZipUtils.java jdk.zipfs/share/classes/jdk/nio/zipfs/ZipUtils.java the computation of the "seconds" values is done by public static long dosToJavaTime(long dtime) { ... (int) ((dtime << 1) & 0x3e)); ... } 0x3e is binary 111110 or decimal 62, so larger values than the supported maximum 59 can occur (maybe only with old/problematic archives but still we had such an example). When looking a bit more closely into it, we found that the "second = 60" value was coming from a long value 930973758 that was passed to ZipUtils.dosToJavaTime as dtime argument. (found this out by adding an enhanced chained exception to jdk9 showing the dtime value passed to dosToJavaTime). Here is an example of the iteration on entries of such a "bad" zip file : C:\JVM\openjdk10\bin\java ZipIterate Iterate von ZipFile: -------------------------------------- name of ZipEntry:com/microsoft/sqlserver/jdbc/AppDTVImpl$SetValueOp.class Exception in thread "main" java.time.DateTimeException: Invalid value for SecondOfMinute (valid values 0 - 59): 60 at java.base/java.time.temporal.ValueRange.checkValidValue(ValueRange.java:311) at java.base/java.time.temporal.ChronoField.checkValidValue(ChronoField.java:714) at java.base/java.time.LocalTime.of(LocalTime.java:322) at java.base/java.time.LocalDateTime.of(LocalDateTime.java:337) at java.base/java.util.zip.ZipUtils.dosToJavaTime(ZipUtils.java:101) at java.base/java.util.zip.ZipUtils.extendedDosToJavaTime(ZipUtils.java:114) at java.base/java.util.zip.ZipEntry.getTime(ZipEntry.java:198) at ZipIterate.main(ZipIterate.java:37) A similar problem (dealing with days/months not seconds out of supported range) is described here : https://bugs.openjdk.java.net/browse/JDK-8184940 "JDK 9 rejects zip files where the modified day or month is 0" Compared to jdk8, this is a regression so I think it would be good to have a better handling of problematic out of range seconds-values. Do you think it would be sufficient to adjust the out of range seconds to the supported max-value 59 ? Or is something more sophisticated prefered ? I found the enhanced chained exception showing the dtime long values helpful too, should I submit it ? Best regards, Matthias

3 2

Review Request JDK-8188052: JNI_FindClass needs to specify the class loading context used for library lifecycle hooks
by mandy chung 05 Oct '17

05 Oct '17

This patch separates the JNI `FindClass` issue from the review thread for JDK-8188052 [1] into a different issue. webrev: http://cr.openjdk.java.net/~mchung/jdk10/webrevs/8188052/webrev.00/index.ht… This patch changes `FindClass` to specify the class loading context used for the load and unload hook. `FindClass` when called from JNI_OnUnload is changed to use the system class loader as the context. This is an incompatible behavioral change and this bumps the version so that a native library can determine the new behavior if desire. This change proposes to defines JNI_VERSION_10 and I expect this name will be revisited along with the version string for the proposed new release cadence [2]. CSR: https://bugs.openjdk.java.net/browse/JDK-8188069 Thanks David for the suggested spec wordings. thanks Mandy [1] http://mail.openjdk.java.net/pipermail/core-libs-dev/2017-September/049292.… [2] http://mail.openjdk.java.net/pipermail/discuss/2017-September/004281.html

2 3

[10] RFR(XS) 8188776: jdk.internal.vm.ci can't export package to upgradeable modules
by Vladimir Kozlov 04 Oct '17

04 Oct '17

https://bugs.openjdk.java.net/browse/JDK-8188776 8182701 added exports for jdk.vm.ci.runtime package [1] but did not add new exception in the test. Added missing exception in JdkQualifiedExportTest.java test: --- a/test/jdk/jdk/modules/etc/JdkQualifiedExportTest.java +++ b/test/jdk/jdk/modules/etc/JdkQualifiedExportTest.java @@ -70,6 +70,7 @@ static Set<String> KNOWN_EXCEPTIONS = Set.of("jdk.internal.vm.ci/jdk.vm.ci.services", + "jdk.internal.vm.ci/jdk.vm.ci.runtime", "jdk.jsobject/jdk.internal.netscape.javascript.spi"); static void checkExports(ModuleDescriptor md) { Verified with this test. Thanks, Vladimir [1] http://hg.openjdk.java.net/jdk10/hs/rev/8b2054b7d02c#l3.1

2 2

Review Request JDK-8188321:(jdeps) help message should say "requires transitive" rather than "requires public"
by mandy chung 04 Oct '17

04 Oct '17

http://cr.openjdk.java.net/~mchung/jdk10/webrevs/8188321/webrev.00/ This changes the jdeps help message fixing the typo, move -q and -version to the first section and group the module-related options together in a new section. thanks Mandy

3 2