Hello,

For the doc changes in MethodHandle, please supplement the paragraph

1937          * <p>
1938          * Note that this method throws errors related to loading and linking as
1939          * specified in Sections 12.2 and 12.3 of <em>The Java Language
1940          * Specification</em>.

with @jls tags like

@jls 12.2 Loading of Classes and Interfaces
@jsl 12.3 Linking of Classes and Interfaces

as done elsewhere in the base module. I think it would be slightly better to remove the section numbers from the main text but keep them in the @jls tags since it is now possible to check that the listed name of the section matches what is in the JLS.

Thanks,

-Joe


On 9/4/2019 2:12 PM, Brent Christian wrote:
Hi,

Please review my fix for JDK-8212117[1].  The webrev is here:

http://cr.openjdk.java.net/~bchristi/8212117/webrev09/

There is also a CSR[2] in need of review.

The spec for the 2-arg and 3-arg Class.forName() methods states they will "locate, load, and link" the class, however the linking part is not ensured to happen.

Although the VM spec allows flexibility WRT when classes are linked, this is a corner where the Class.forName() spec is not being upheld. While this is not an issue for most usages,  8181144 [3] demonstrates how this can be a problem (with the debugging interface, in this case).

This fix ensures that linking happens during the course of Class.forName().  Class.forName() already @throws LinkageError, so no spec change is needed there. MethodHandles.Lookup.findClass() needs a small spec update, due to calling Class.forName with init=false,

Of course Errors are not required to be caught.  It is therefore possible that the new behavior could introduce previously unseen, possibly unhandled LinkageErrors.  A new VM flag (-XX:+ClassForNameDeferLinking) is introduced to restore the previous behavior (to keep such code running until it can be updated).

This change surfaced a couple new "A JNI error has occurred" situations (see 8181033[5]) in the Launcher, by way of
  test/jdk/tools/launcher/MainClassCantBeLoadedTest.java
(using the 3-arg Class::forName, detailed in the bug report[4]),
and
  test/jdk/tools/launcher/modules/basic/LauncherErrors.java
(using the 2-arg Class::forName).

The Launcher is updated to maintain non-confusing error messages :).

The new test included with this fix ensures that 8181144[3] is addressed.  Thanks go to Serguei Spitsyn for writing the test.

Automated corelibs and hotspot tests pass cleanly.

Thanks,
-Brent

--
1. https://bugs.openjdk.java.net/browse/JDK-8212117

2. https://bugs.openjdk.java.net/browse/JDK-8222071

3. https://bugs.openjdk.java.net/browse/JDK-8181144

4. https://bugs.openjdk.java.net/browse/JDK-8212117?focusedCommentId=14215986&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-14215986

5. https://bugs.openjdk.java.net/browse/JDK-8181033

Reply via email to