September 2018 - core-libs-dev

Review Request JDK-8210841: test/jdk/vm/runtime/ReflectStackOverflow.java fails with NoClassDefFoundError
by mandy chung 18 Sep '18

18 Sep '18

test/jdk/vm/runtime/ReflectStackOverflow.java fails and it's caused by JDK-8210721. I propose to revert the change in InvocationTargetException to give us time to investigate this regression. webrev: http://cr.openjdk.java.net/~mchung/jdk12/webrevs/8210841/webrev.00/ thanks Mandy

3 2

RFR JDK-8034802: (zipfs) newFileSystem throws UOE when the zip file is located in a custom file system
by Xueming Shen 18 Sep '18

18 Sep '18

Hi Please help review the change for JDK-8034802. issue: https://bugs.openjdk.java.net/browse/JDK-8034802 webrev: http://cr.openjdk.java.net/~sherman/8034802/webrev One of the reasons that the implementation works this way is that I didn't have a complete SeekableByteChannel support (mainly the position(...)) from newByteChannel() back then (so you really can't get a ZipFileSystem from a jar/zip file inside a zfs :-)) So this changeset also includes a ByteArrayChannel which implements SBC and has better position()/position(int) support, and now we can have a zipfs from a zip file inside a zipfs. Thanks, Sherman

3 3

RFR (12): 8210819: Update the host name in CNameTest.java
by Frank Yuan 18 Sep '18

18 Sep '18

Hi Would you like to review the following patch for Bug: https://bugs.openjdk.java.net/browse/JDK-8210819 --- a/test/jdk/sun/net/InetAddress/nameservice/dns/CNameTest.java Wed Sep 12 21:56:59 2018 -0700 +++ b/test/jdk/sun/net/InetAddress/nameservice/dns/CNameTest.java Mon Sep 17 15:00:55 2018 +0800 @@ -1,5 +1,5 @@ /* - * Copyright (c) 2017, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2017, 2018, 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 @@ -38,7 +38,7 @@ * @summary Test DNS provider's handling of CNAME records */ public class CNameTest { - private static final String HOST = "www-proxy.us.oracle.com"; + private static final String HOST = "www.w3c.org"; CNameTest.java is to test DNS provider's handling of CNAME records, so the test requires dns has cname record for the hostname. The old name is not appropriate, I would replace it with www.w3c.org <http://www.w3c.org> . Thanks Frank

2 2

RFR(JDK12/JAXP/java.xml) 8207760: SAXException: Invalid UTF-16 surrogate detected: d83c ?
by Joe Wang 17 Sep '18

17 Sep '18

Hi, Please review a patch for a situation where a surrogate pair is at the edge of a buffer. What the existing impl did was to report it as an error. This patch fixes it by caching the high surrogate and prints it out along with the low surrogate. Similar issue exists also in the CDATA section and is fixed in this patch. The CDATA impl had a couple of bugs where an indent could be written inside the CDATA and an unicode character written in between two CDATA sections. Both are fixed in this patch. JBS: https://bugs.openjdk.java.net/browse/JDK-8207760 webrevs: http://cr.openjdk.java.net/~joehw/jdk12/8207760/webrev/ Thanks, Joe

3 9

[12] Review Request: 8210692 The "com.sun.awt.SecurityWarning" class can be dropped
by Sergey Bylokhov 17 Sep '18

17 Sep '18

Hello. Please review fix for jdk12. Bug: https://bugs.openjdk.java.net/browse/JDK-8210692 Webrev: http://cr.openjdk.java.net/~serb/8210692/webrev.00 CSR: https://bugs.openjdk.java.net/browse/JDK-8210693 The client code has a "com.sun.awt.SecurityWarning" class which at some point in the past, JDK 6u10, was used as a kind of "public" API. During development of jdk9 it was considered that this API is not supported and may be removed in the future: JDK-8051640. In jdk11 this class was marked as "forRemoval = true" (see JDK-8205588) and can be dropped in 12. In the fix these things were removed: - com/sun/awt/SecurityWarning.java - The test for SecurityWarning class - Test groups, because SecurityWarning is the latest class in "com/sun/awt" package - The reference to "com/sun/awt" in the "default.policy" (cc) core-libs-dev: I am not sure who is responsible to review the change in "default.policy". -- Best regards, Sergey.

6 7

Review Request JDK-8210721: Replace legacy serial exception field with Throwable::cause
by mandy chung 17 Sep '18

17 Sep '18

A few exception classes such as ExceptionInInitializerError, InvocationTargetException, etc were defined prior to the exception chaining facility and provides the API to get the cause of the exception. These classes keep its legacy serial field e.g. ExceptionInInitializerError::exception to get the serialization to work on older JDK version. During the review of JDK-8209553 [1], Peter and Joe suggest to remove this legacy serial field and implement the serialization using serialPersistentFields, readObject and writeObject. It turns out that these redundant fields were removed when the exception chaining support was initially implemented but it got reverted to fix JDK-4385429 since Throwable::setCause rejects any further change as Throwable::cause has been initialized to null. This patch removes these legacy serial field and stores the cause in Throwable::cause. It's essentially a redo for the fix for JDK-4385429. webrev: http://cr.openjdk.java.net/~mchung/jdk12/webrevs/8210721/webrev.00/ Mandy [1] http://mail.openjdk.java.net/pipermail/core-libs-dev/2018-August/054878.html

3 4

[12] RFR 8210695: Create test to cover JDK-8205330 InitialDirContext ctor sometimes throws NPE if the server has sent a disconnection
by Chris Yin 17 Sep '18

17 Sep '18

Please have a review for below new added test to cover JDK-8205330, thanks This test used dummy ldap server to simulate the scenario to send “Notice of Disconnection” after binding, it will repeat InitialDirContext() for 1000 times (normally the NPE bug should be hit less than 100 times run, but just in case), test failed with NPE if without JDK-8205330 fix change and passed after the fix. I put this test under test/jdk/com/sun/jndi/ldap/ since the bug root cause is from com.sun.jndi.ldap.LdapClient, feel free to let me know if it should be moved to other suitable folder. bug: https://bugs.openjdk.java.net/browse/JDK-8210695 webrev: http://cr.openjdk.java.net/~xyin/8210695/webrev.00/ Regards, Chris

2 4

[JDK-8209078] Unable to call default interface methods from named module
by Johannes Kuhn 16 Sep '18

16 Sep '18

Hi everyone, I recently reported JDK-8209078 "Unable to call default method from interface in another module from named module" [1]. It is possible to call any default interface method directly using MethodHandles.lookup().findSpecial(Interf.class, "defaultMethodName", ..., Intef.class).invoke(...); This only works if the caller is in an unnamed module or Interf is in the same module as the caller. I now want to share some thoughts about this feature, and an idea how it could be fixed, but this probably requires further discussion. 1. Use Case: The most obvious use case is to to "implement" default methods in instances created by j.l.r.Proxy. An implementation of j.l.r.InvocationHandler that handles default methods could look like this: public Object invoke(Object proxy, Method method, Object[] args) throws Throwable { if (method.isDefault()) { MethodType mt = methodType(method.getReturnType(), method.getParameterTypes()); Class<?> declaringClass = method.getDeclaringClass(); Lookup l = MethodHandles.lookup(); MethodHandle mh = l.findSpecial(declaringClass, method.getName(), mt, declaringClass) .asFixedArity() .bindTo(proxy); return mh.invokeWithArguments(args); } // Handle the other methods return null; } Such a feature is very valuable so the InvocationHandler does not break if an interface is evolved by adding new default methods. 2. Documentation: When I faced the problem above, I did a search on how to invoke default methods on a proxy. The above code was inspired by this mail [2] from John Rose, where he mentions JDK-8130227 "JEP 274: Enhanced Method Handles" [3][4]. This is the only official documentation I could find on this feature. Indeed, even the Javadoc for findSpecial [5] states that my code above should not successfully run: > Before method resolution, if the explicitly specified caller class is > not identical with the lookup class, or if this lookup object does not > have private access privileges, the access fails. The specified caller class in the example above is the interface itself, not the lookup class. The documentation has to be at least changed to reflect this feature. 3. Cause of the discrepancy between lookup class in named module vs unnamed module As Mandy Chung observed: > IAE is thrown because the implementation calls Lookup::in teleporting > to a Lookup of the special class which lost all access since the > caller class and the requested lookup class are in different modules. This behavior is documented in the Lookup.in() [6]: > * If the old lookup class is in a named module, and the new lookup > class is in a different module M, then no members, not even public > members in M's exported packages, will be accessible. The exception to > this is when this lookup is publicLookup, in which case PUBLIC access > is not lost. > * If the old lookup class is in an unnamed module, and the new lookup > class is a different module then MODULE access is lost. This strikes me as a little bit odd, so I looked around for the reasoning for this difference, and found an old discussion on jigsaw-dev [7]. I especially found this mail from Alan Bateman [8] interesting, where he gave a reasoning for this: > Preserved but perhaps with the (initially surprising) consequence that > all access is lost when m(LC) is a named module and m(A) is a > different module. This arises because the two modules may read very > different sets of modules, the intersection cannot be expressed via a > lookup class + modes. While I agree that intersection might not be expressed via lookup class + modes, I don't think that it is necessary to express that. Instead don't allow any lookup on members in a different module if the MODULE flag is not set. But this is only a suggestion. It's up to you to decide how to deal with this. Please keep up the good work you are doing, and I hope this mail will help you fixing this bug. Thanks for your time, Johannes Kuhn [1]: https://bugs.openjdk.java.net/browse/JDK-8209078 [2]: http://mail.openjdk.java.net/pipermail/jigsaw-dev/2017-January/010741.html [3]: https://bugs.openjdk.java.net/browse/JDK-8130227 [4]: http://openjdk.java.net/jeps/274 [5]: https://docs.oracle.com/javase/10/docs/api/java/lang/invoke/MethodHandles.L… [6]: https://docs.oracle.com/javase/10/docs/api/java/lang/invoke/MethodHandles.L… [7]: http://mail.openjdk.java.net/pipermail/jigsaw-dev/2015-December/005751.html [8]: http://mail.openjdk.java.net/pipermail/jigsaw-dev/2015-December/005768.html

2 2

RFR 8210787 : Object.wait(long, int) throws inappropriate IllegalArgumentException
by Ivan Gerasimov 15 Sep '18

15 Sep '18

Hello! This is inspired by a recent fix for JDK- 8210004 (Thread.sleep(millis, nanos) timeout returns early). Currently, Object.wait(Long.MAX_VALUE, 1) would throw "IllegalArgumentException: timeout value is negative", which is not accurate. Would you please help review the fix? BUGURL: https://bugs.openjdk.java.net/browse/JDK-8210787 WEBREV: http://cr.openjdk.java.net/~igerasim/8210787/00/webrev/ -- With kind regards, Ivan Gerasimov

3 2

RFR(XS) : 8210779 : 8182404 and 8210732 haven't updated copyright years
by Igor Ignatyev 15 Sep '18

15 Sep '18

http://cr.openjdk.java.net/~iignatyev//8210779/webrev.00/index.html > 36 lines changed: 0 ins; 0 del; 36 mod; Hi all, could you please review this small and trivial follow-up fix for 8182404 and 8210732? I have forgot to update copyright years as a part of these two changesets, this patch fixes that. webrev: http://cr.openjdk.java.net/~iignatyev//8210779/webrev.00/index.html JBS: https://bugs.openjdk.java.net/browse/JDK-8210779 Thanks, -- Igor

2 1