June 2014 - core-libs-dev

RFR 9 and 8u: JDK-8029674: (reflect) getMethods returns default methods that are not members of the class
by Joel Borggrén-Franck 12 Jun '14

12 Jun '14

Hi, Since Java 8 the methods Class.getMethod() and Class.getMethods() can in some circumstances return methods that are not members of the class. This is due to a spec change in 8 not due to a code change. For this to happen you need to have an interface hierarchy that looks something like: interface I { void m(); } interface J extends I { void m(); } and either one of: interface K extends ***I,*** J {} class C implements ***I,*** J {} In Java 7 K.class.getMethods() would return [ I.m, J.m ]. The 7 spec is a bit vague but I have been convinced by experts that this is actually correct in 7. However this was changed in the spec for 8, now only J.m() is a member of K or C so the result should only contain [ J.m() ]. See the bug: https://bugs.openjdk.java.net/browse/JDK-8029674 for a more thorough discussion of this. The fix proposed for 8u (and 9 at first) is a partial fix that ensures that: - a concrete class does not have any abstract methods - nothing is changed unless someone introduces default methods into the interface hierarchy This is to minimise the behavioural compatibility concerns in a minor release. For 9 we might implement the full solution. (See the 9 bug: https://bugs.openjdk.java.net/browse/JDK-8029459) Basically in the fix proposed for 8u, default methods prune less specific methods from the result if they have the same discriptor. There are some complications due to preserving the old lookup order and the fact that we don’t want different results depending on the order of pairwise comparisons. Since this work and MethodArray in general is comparing descriptors bridges should not interact with this patch. Bug: https://bugs.openjdk.java.net/browse/JDK-8029674 Patch: http://cr.openjdk.java.net/~jfranck/8029674/webrev.02/ *** Testing *** I made a test with a bit over 100 cases where 40 of the tests test behaviour that shouldn’t change (IE they pass both with and without the patch). The test is a bit vast, but I have commented some of the more interesting results. *** Performance *** The time complexity of this is O(defaultMethods * methods). getMethods() is already somewhat quadratic in complexity due to removing interface methods that have a concrete implementation. Also the result is cached so this is a one time increase per Class. I have done some experiments on a patched jdk without the cache and the performance looks decent: - Loading every class in rt.jar and calling getMethods() on every Class takes 8s on my laptop both before and after the patch. Perhaps not that surprising since there aren’t that many default methods in the jdk and this is probably dominated by IO. - Calling getMethods on a Class with 16K methods, 10% which are default methods, seem to take roughly 50% longer on my laptop (2,4 instead of 1,7 seconds). I’m working on converting this benchmark to JMH, but at least I don’t think I have made the most basic benchmark errors. If this turns out to be a performance problem I have an idea for an index that will make this O(methods sharing name * methods) for an addition of a few allocations, but I don’t expect this to be necessary. Thanks in advance cheers /Joel

3 4

RFR (XL) 8046070 - Class Data Sharing clean up and refactoring, round #1
by Ioi Lam 11 Jun '14

11 Jun '14

Hi Folks, Please review a the following clean up and refactoring of the AppCDS code http://cr.openjdk.java.net/~iklam/8046070-cds-cleanup-v1/ https://bugs.openjdk.java.net/browse/JDK-8046070 Summary of fix: Clean up and refactor the Class Data Sharing (CDS) code, including: + Improve archive integrity checking + Support bytecode verification during archive dumping time + Allow the user to configure the CDS class list and archive file. + Allow future extension of the CDS class loading mechanism. Tests: JPRT (hotspot only, and jdk) UTE (vm.runtime.testlist, vm.quick.testlist, vm.parallel_class_loading.testlist) Thanks - Ioi

1 0

Re: RFR 9 and 8u: JDK-8029674: (reflect) getMethods returns default methods that are not members of the class
by Andrej Golovnin 10 Jun '14

10 Jun '14

Hi Joe, > Sorry for the belated review. > > Generally the change looks good. One question, in > > 2803 private boolean matchesNameAndDescriptor(Method m1, Method m2) { > 2804 return m1.getReturnType() == m2.getReturnType() && > 2805 m1.getName() == m2.getName() && > 2806 arrayContentsEq(m1.getParameterTypes(), > 2807 m2.getParameterTypes()); > 2808 } > > Should the equality check on 2805 be .equals rather than == ? > "==" can be used in this case as the method name is interned by JVM. Here is the comment for the field "name" from java.lang.reflect.Method: // This is guaranteed to be interned by the VM in the 1.4 // reflection implementation private String name; BTW, in the old version of Class in the line 2766 there was already a similar check: 2764 if (m != null && 2765 m.getReturnType() == toRemove.getReturnType() && 2766 m.getName() == toRemove.getName() && 2767 arrayContentsEq(m.getParameterTypes(), 2768 toRemove.getParameterTypes())) { 2769 methods[i] = null; Best regards, Andrej Golovnin

3 5

RFR for bug JDK-8004807: java/util/Timer/Args.java failing intermittently in HS testing
by Eric Wang 09 Jun '14

09 Jun '14

Hi, Please help to review the fix for bug https://bugs.openjdk.java.net/browse/JDK-8004807 as below: http://cr.openjdk.java.net/~ewang/JDK-8004807/webrev.00/ The root cause of the failure is if Timer.cancel() is not called in time, the recurring timer task keeps running which cause the the assertion "check(latch.getCount() > 0)" failed. The fix includes: 1. As the purpose of the assertion "check(latch.getCount() > 0)" in timer task is used to check if the unrepeatable task is executed repeatedly and this assertion is meaningless for a repeatable task. it can be replaced by other way, see the webrev. 2. The timer thread should be terminated ASAP once all tasks are finished to avoid time spent on thread switch. 3. The test should be executed in othervm mode to avoid time spent on thread switch. Thanks, Eric

4 15

RFR: 8046085: (s) inserting null key into HashMap treebin fails
by Mike Duigou 06 Jun '14

06 Jun '14

Hello all; This changeset corrects a problem inserting a null key into a HashMap that is using TreeBins. The root cause of the failure was diagnosed by Paul Sandoz and confirmed by Alan Bateman, Doug Lea and myself. I've prepared the changeset and a minimal test which reproduces the failure. jbsbug: https://bugs.openjdk.java.net/browse/JDK-8046085 webrev: http://cr.openjdk.java.net/~mduigou/JDK-8046085/0/webrev/ This change will be backported to 8u-dev as quickly as possible. Mike

2 1

RFR: 8042789: org.omg.CORBA.ORBSingletonClass loading no longer uses context class loader
by Seán Coffey 06 Jun '14

06 Jun '14

Changes in the ORB loading in 8u5 causes incompatibility changes for some environments where a 3rd party plugin might be deployed over plugin. Some CORBA deployment installations have an incorrect expectation on finding the custom ORBSingleton class using the context classloader. Proper approach is to expect to find such classes on the system classloader. For JDK 9, the 8u5(new) behaviour will remain. For JDK 8u and older update families, I'm proposing we revert back to the old behaviour to aid compatibility in current deployment scenarios. webrev : http://cr.openjdk.java.net/~coffeys/webrev.8042789/webrev/ bug : https://bugs.openjdk.java.net/browse/JDK-8042789 I'm working a testcase for this also. regards, Sean.

3 2

[9] RFR (S): 8032400: JSR292: invokeSpecial: InternalError attempting to lookup a method
by Vladimir Ivanov 06 Jun '14

06 Jun '14

https://bugs.openjdk.java.net/browse/JDK-8032400 http://cr.openjdk.java.net/~vlivanov/8032400/webrev.00/ Consider the following hierarchy: class T1 { int m() { return 1; }} class T2 extends T1 { static int m() { return 2; }} class T3 extends T2 { int m() { return 3; }} T3 has a method, which does the following using method handles: T3::test { invokespecial T1.m() T3 } T1.m lookup attempt from T3 ends up as T2.m lookup, but it fails since T2 has static method with the same signature. Lookup.getDirectMethodCommon doesn't expect such failure and throws InternalError. JVMS specification states the following: // JVMS 6.5: invokespecial // ... 2. Otherwise, if C is a class and has a superclass, a search // for a declaration of an instance method with the same name and // descriptor as the resolved method is performed, starting with the // direct superclass of C and continuing with the direct superclass // of that class, and so forth, until a match is found or no further // superclasses exist. If a match is found, then it is the method to // be invoked. The fix is to comply with the spec and search upper in the class hierarchy if lookup attempt fails. Testing: regression test, jdk/test/java/lang/invoke, tests on access checks. Thanks! Best regards, Vladimir Ivanov

3 7

RFR 8043954: Fix behavior difference of connect() for AIX
by Jonathan Lu 06 Jun '14

06 Jun '14

Hello, Could I have following patch reviewed for bug 8034954 ? http://cr.openjdk.java.net/~luchsh/JDK-8043954/ The patch is to fix a behavior difference of connect() API for AIX platform, according to the documentation, http://www-01.ibm.com/support/knowledgecenter/ssw_aix_71/com.ibm.aix.commtr… On AIX, when connect() got interrupted by signal, the underlying connection will be made asynchronously, "EINTR The attempt to establish a connection was interrupted by delivery of a signal that was caught; the connection will be established asynchronously." This fix tries to poll() for successfully established connection or error in the similar way as NET_Timeout(). Thanks Jonathan

3 9

Re: Approved: <Beans Dev> RFR: 8044855: Add missing @since tag under java.beans.*
by Henry Jen 05 Jun '14

05 Jun '14

Thanks Sergey and Phil for reviewing. I assume this can go into jdk9/dev although in JBS the category is client-libs? Cheers, Henry On 06/05/2014 02:40 AM, sergey malenkov wrote: > The fix looks good for me. > > Thanks, > SAM > > On 05.06.2014 2:14, Henry Jen wrote: >> Hi, >> >> Please review a trivial webrev that provides missing @since tag for >> elements under java.beans, >> >> This version is determined based on javadoc of 1.0.2, 1.1.8 and 1.2.2. >> >> http://cr.openjdk.java.net/~henryjen/jdk9/8044855/0/webrev >> >> Cheers, >> Henry >

2 1

RFR (JAXP): 8041523:Xerces Update: Serializer improvements from Xalan
by David Li 04 Jun '14

04 Jun '14

Hi, This is an update to JAXP serialization from Apache Xalan. For details, please refer to: https://bugs.openjdk.java.net/browse/JDK-8041523. Webrevs: http://cr.openjdk.java.net/~dli/8041523/webrev/ Summary of changes. * Changes from the Apache bug referenced in JDK-8041523 * Update ToStream.setCdataSectionElements to a newer version from the most recent Apache ToStream.java source file. This was causing the JCK test failures noted in the bug. Note, there are two setCdataSectionElement methods in ToStream.java. The one with the bug has been fixed. * Some updates to accommodate ArrayList, HashMap, and Generics. * Bundled in changes for JDK-8037259. This was an xpointer bug. Existing tests: JAXP SQE and unit tests passed. JCK tests passed. Thanks, David

2 5