JDK-8212117 : Class.forName may return a reference to a loaded but not linked Class
  • Type: Bug
  • Component: core-libs
  • Sub-Component: java.lang:class_loading
  • Affected Version: 8,9,10,11
  • Priority: P3
  • Status: Resolved
  • Resolution: Fixed
  • Submitted: 2018-10-12
  • Updated: 2019-11-04
  • Resolved: 2019-09-09
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 14
14 b14Fixed
Related Reports
Blocks :  
JDK-8227926 - add new JVM TI and JDI tests for classes loaded by Class::forName
CSR :  
JDK-8222071 - Class.forName may return a reference to a loaded but not linked Class
Relates :  
JDK-8230937 - Update bugid in ProblemList for vmTestbase/nsk/jdb/eval/eval001/eval001.java
Relates :  
JDK-8233272 - The Class.forName specification should be updated to match the long-standing implementation with respect to class linking
Relates :  
JDK-8226540 - Tighten spec around class loading/resolving/linking
Relates :  
JDK-8231924 - Field::getGenericType triggers loading and linking that may fail with LinkageError
Relates :  
JDK-8233260 - Class.forName should be able to load, but not link, a class
Relates :  
JDK-8181144 - JDI - VirtualMachine.allClasses() does not return loaded but uninitialized class
Relates :  
JDK-8233091 - Backout JDK-8212117: Class.forName loads a class but not linked if class is not initialized
Relates :  
JDK-8227926 - add new JVM TI and JDI tests for classes loaded by Class::forName
Sub Tasks
JDK-8222970 :  
Update ProblemList for vmTestbase/nsk/jdb/eval/eval001/eval001.java - Resolved
JDK-8230780 :  
Release Note: Classes are now loaded and linked by Class.forName() - Resolved
Description
As part of the investigation of JDK-8181144, it has been noticed that Class.forName, with initialize=false, can return a class that has not been linked. This conflict between the spec and implementation needs to be investigated and resolved.
Comments
The behavioral change by this fix has higher compatibility concern impacting existing frameworks. This fix changes the long-standing implementation to match the specification of `Class::forName` three-arg method that always links the loaded class or interface whether initialize argument is true or false. Existing frameworks that calls Class::forName with initialize=false now may need to prepare for different results, for example NCDFE or ICCE be thrown due to linkage error. The libraries that build on top of such a framework would need to upgrade to a newer version of that framework. JDK-8231924 shows that Field::getGenericType does not prepare NCDFE thrown due to the verification error caused by linking of the loaded class. We reconsidered the compatibility risk and propose to revert this behavioral change. Instead changing the API spec to align with the long-standing behavior. 1. JDK-8233091: backout/revert the fix for JDK-8212117 2. JDK-8233272: change Class::forName API spec to match the long-standing behavior, i.e. no linking if initialize=false 3. reopen JDK-8181144 and investigate if JDI VirtualMachine::allClasses and JVM TI AllLoadedClasses are expected to return linked classes and whether the fix should be in JDI and JVM TI or in the library to introduce a new API to load and link a class.
31-10-2019
URL: https://hg.openjdk.java.net/jdk/jdk/rev/db92a157dd70 User: bchristi Date: 2019-09-09 18:04:40 +0000
09-09-2019
Review thread: http://mail.openjdk.java.net/pipermail/core-libs-dev/2019-September/062142.html
05-09-2019
The 2-arg Class.forName was added in Java SE 9 and unlikely to be widely used. So the compatibility risk in changing its behavior is low. Ditto for Lookup.findClass although this method doesn't specify if it links or not. For the 3-arg Class.forName then I think existing code calling it with initialize=false needs to be concerned with LinkageError already. If the class file is malformed then ClassFormatError will be thrown. If the super class is final or not accessible then it will lead to VerifyError or IllegalAccessError. So the compatibility impact of changing the implementation may not be too bad, maybe manageable if we can get away with a flag for a release or two that reverts to long standing behavior.
16-10-2018
I would like the CSR review to determine whether to change the spec or the implementation. I don't expect code to catch LinkageError in the vast majority of cases so I don't think there is a problem. However as I said to Alan offline this is basically untestable - you would need to inject a class that fails linkage for every code path that calls forName(name, false, loader) and forName(module, name). If we change then spec then I strongly suggest that we also add a new overload of forName that does allow linking distinct from initialization. That would also give the opportunity to rationalize the different forName specifications, particularly with respect to exceptions. Alan has raised offline the issue of throwing AccessControlException due to package-access checks versus the specified permission related SecurityException.
16-10-2018
[~alanb] [~dholmes] you are right on no source incompatibility issue. What I should have said is that I'm not surprised to see some applications depending on this long standing behavior which might require update to the existing code to catch the error for whatever reason. Existing code calling Class.forName(cn, false, loader) catching CNFE but not LinkageError might be impacted if we change the long-standing behavior. It would require audit JDK implementation and determine if anything needs fixing (e.g. ServiceLoader that throws a different exception if fails to load a service provider class). Testing may not uncover this issue. I can't think of obvious benefit to change the long standing behavior besides detecting the linkage error before Class.forName returns. The other option fixing the spec to match the long-standing behavior has no impact to existing code and JDK implementation. The class will be linked when it's initialized. I'm getting to think keeping the long-standing behavior a better solution.
16-10-2018
Here's a more interesting failure: jdk/tools/launcher/MainClassCantBeLoadedTest.java ###TestError###: string <Error: Unable to initialize main class C> not found ###TestError###: string <Caused by: java.security.AccessControlException: access denied ("java.lang.RuntimePermission" "accessClassInPackage.com.sun.crypto.provider")> not found ++++Begin Test Info++++ Test Status: FAIL ++++Test Environment++++ JDK8_HOME=/scratch/opt/mach5/mesos/work_dir/jib-master/install/jdk/10/46/bundles/linux-x64/jdk-10_linux-x64_bin.tar.gz/jdk-10 PATH=/bin:/usr/bin JIB_HOME=/scratch/opt/mach5/mesos/work_dir/jib-master/install/com/oracle/java/jib/jib/3.0-SNAPSHOT/jib-3.0-SNAPSHOT-distribution.zip/jib-3.0-SNAPSHOT-distribution TEST_IMAGE_GRAAL_DIR=/scratch/opt/mach5/mesos/work_dir/jib-master/install/2018-10-14-2349059.david.holmes.jdk-dev/linux-x64-open.test/hotspot/jtreg/graal JTREG_TIMEOUT_OPTION=-timeoutFactor:10 JIB_DATA_DIR=/scratch/opt/mach5/mesos/work_dir/jib-master JTREG_SHOWAGENT=true JTREG_VERBOSE=fail,error,time CLASSPATH=/scratch/opt/mach5/mesos/work_dir/jib-master/install/java/re/jtreg/4.2/promoted/all/b13/bundles/jtreg_bin-4.2.zip/jtreg/lib/javatest.jar:/scratch/opt/mach5/mesos/work_dir/jib-master/install/java/re/jtreg/4.2/promoted/all/b13/bundles/jtreg_bin-4.2.zip/jtreg/lib/jtreg.jar HOME=/tmp/sparky-temp-home-7497122840514283610/user_home ++++Test Output++++ Error: A JNI error has occurred, please check your installation and try again Exception in thread "main" java.security.AccessControlException: access denied ("java.lang.RuntimePermission" "accessClassInPackage.com.sun.crypto.provider") at java.base/java.security.AccessControlContext.checkPermission(AccessControlContext.java:472) at java.base/java.security.AccessController.checkPermission(AccessController.java:895) at java.base/java.lang.SecurityManager.checkPermission(SecurityManager.java:408) at java.base/java.lang.SecurityManager.checkPackageAccess(SecurityManager.java:1324) at java.base/java.lang.ClassLoader$1.run(ClassLoader.java:690) at java.base/java.lang.ClassLoader$1.run(ClassLoader.java:688) at java.base/java.security.AccessController.doPrivileged(Native Method) at java.base/java.lang.ClassLoader.checkPackageAccess(ClassLoader.java:688) at java.base/java.lang.Class.forName0(Native Method) at java.base/java.lang.Class.forName(Class.java:397) at java.base/sun.launcher.LauncherHelper.loadMainClass(LauncherHelper.java:760) at java.base/sun.launcher.LauncherHelper.checkAndLoadMain(LauncherHelper.java:655) ++++Test Stack Trace++++ java.lang.Throwable: current stack of the test TestHelper.doExec(TestHelper.java:468) TestHelper.doExec(TestHelper.java:429) MainClassCantBeLoadedTest.testFailToInitializeMainClass(MainClassCantBeLoadedTest.java:136) java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method) java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) java.base/java.lang.reflect.Method.invoke(Method.java:566) TestHelper.run(TestHelper.java:203) MainClassCantBeLoadedTest.main(MainClassCantBeLoadedTest.java:152) java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method) java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) java.base/java.lang.reflect.Method.invoke(Method.java:566) com.sun.javatest.regtest.agent.MainActionHelper$AgentVMRunnable.run(MainActionHelper.java:246) java.base/java.lang.Thread.run(Thread.java:835) ++++End of Test Info++++ java.lang.RuntimeException: Test failed at MainClassCantBeLoadedTest.main(MainClassCantBeLoadedTest.java:155) at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.base/java.lang.reflect.Method.invoke(Method.java:566) at com.sun.javatest.regtest.agent.MainActionHelper$AgentVMRunnable.run(MainActionHelper.java:246) at java.base/java.lang.Thread.run(Thread.java:835) The test expects the class to fail initialization due to the attempt to access an inaccessible class. The LauncherHelper.loadMainClass does: mainClass = Class.forName(cn, false, scl); which now triggers linking and it is the linking that encounters the failure due to trying to access the inaccessible class. The LauncherHelper doesn't anticipate anything that is not a LinkageError and so the unexpected exception leads to the "A JNI Error has occurred" followed by the "Exception in thread main". This is a bug in the LauncherHelper as Class.forName can throw SecurityException so that should be handled as an anticipated error case, not an unexpected exception - this is another variation of the problem addressed in JDK-8181033 and related issues.
15-10-2018
Here's one test failure, presumably caused by a class that fails linking/verification: hotspot/jtreg/gc/logging/TestMetaSpaceLog.java stderr: [Exception in thread "main" java.lang.VerifyError: Operand stack underflow Exception Details: Location: case00.f()I @0: pop Reason: Attempt to pop empty stack. Current Frame: bci: @0 flags: { } locals: { 'case00' } stack: { } Bytecode: 0000000: 57ac at java.base/java.lang.Class.forName0(Native Method) at java.base/java.lang.Class.forName(Class.java:397) at TestMetaSpaceLog$StressMetaSpace.loadClass(TestMetaSpaceLog.java:137) at TestMetaSpaceLog$StressMetaSpace.main(TestMetaSpaceLog.java:129) Simple fix for such cases: s/Class.forName(name, false, loader)/loader.loadClass(name)/
15-10-2018
Here's a basic patch that only changes the Class.forName behaviour via a change in JVM_FindClassFromCaller. I pushed the change into find_class_from_class_loader so that we don't need to "unwrap" the jclass. diff -r 537dbfcef4a7 src/hotspot/share/prims/jni.cpp --- a/src/hotspot/share/prims/jni.cpp +++ b/src/hotspot/share/prims/jni.cpp @@ -428,7 +428,7 @@ } TempNewSymbol sym = SymbolTable::new_symbol(name, CHECK_NULL); - result = find_class_from_class_loader(env, sym, true, loader, + result = find_class_from_class_loader(env, sym, true, true, loader, protection_domain, true, thread); if (log_is_enabled(Debug, class, resolve) && result != NULL) { @@ -3291,7 +3291,7 @@ Handle protection_domain; // null protection domain TempNewSymbol sym = SymbolTable::new_symbol(name, CHECK_NULL); - jclass result = find_class_from_class_loader(env, sym, true, loader, protection_domain, true, CHECK_NULL); + jclass result = find_class_from_class_loader(env, sym, true, true, loader, protection_domain, true, CHECK_NULL); if (log_is_enabled(Debug, class, resolve) && result != NULL) { trace_class_resolution(java_lang_Class::as_Klass(JNIHandles::resolve_non_null(result))); diff -r 537dbfcef4a7 src/hotspot/share/prims/jvm.cpp --- a/src/hotspot/share/prims/jvm.cpp +++ b/src/hotspot/share/prims/jvm.cpp @@ -804,7 +804,7 @@ Handle h_loader(THREAD, loader_oop); Handle h_prot(THREAD, protection_domain); - jclass result = find_class_from_class_loader(env, h_name, init, h_loader, + jclass result = find_class_from_class_loader(env, h_name, init, true /*link*/, h_loader, h_prot, false, THREAD); if (log_is_enabled(Debug, class, resolve) && result != NULL) { @@ -843,7 +843,7 @@ } Handle h_loader(THREAD, class_loader); Handle h_prot (THREAD, protection_domain); - jclass result = find_class_from_class_loader(env, h_name, init, h_loader, + jclass result = find_class_from_class_loader(env, h_name, init, false /*link*/, h_loader, h_prot, true, thread); if (result != NULL) { oop mirror = JNIHandles::resolve_non_null(result); @@ -3534,7 +3534,7 @@ // Shared JNI/JVM entry points ////////////////////////////////////////////////////////////// -jclass find_class_from_class_loader(JNIEnv* env, Symbol* name, jboolean init, +jclass find_class_from_class_loader(JNIEnv* env, Symbol* name, jboolean init, jboolean link, Handle loader, Handle protection_domain, jboolean throwError, TRAPS) { // Security Note: @@ -3545,10 +3545,13 @@ // if there is no security manager in 3-arg Class.forName(). Klass* klass = SystemDictionary::resolve_or_fail(name, loader, protection_domain, throwError != 0, CHECK_NULL); - // Check if we should initialize the class + // Check if we should initialize the class (which implies linking) if (init && klass->is_instance_klass()) { klass->initialize(CHECK_NULL); } + else if (link && klass->is_instance_klass()) { + InstanceKlass::cast(klass)->link_class(CHECK_NULL); + } return (jclass) JNIHandles::make_local(env, klass->java_mirror()); } diff -r 537dbfcef4a7 src/hotspot/share/prims/jvm_misc.hpp --- a/src/hotspot/share/prims/jvm_misc.hpp +++ b/src/hotspot/share/prims/jvm_misc.hpp @@ -31,7 +31,8 @@ // Useful entry points shared by JNI and JVM interface. // We do not allow real JNI or JVM entry point to call each other. -jclass find_class_from_class_loader(JNIEnv* env, Symbol* name, jboolean init, Handle loader, Handle protection_domain, jboolean throwError, TRAPS); +jclass find_class_from_class_loader(JNIEnv* env, Symbol* name, jboolean init, jboolean link, + Handle loader, Handle protection_domain, jboolean throwError, TRAPS); void trace_class_resolution(Klass* to_class); Note initialization always implies linking. Currently running through initial testing tier1-3
15-10-2018
I don't think there is a source compatibility issue as LinkageError is an Error rather than a checked exception. I checked Class.forName and Lookup.findClass and all various specify LinkageError so I don't think there are any javadoc changes needed. We also have to to cautious about changing long standing behavior. They may be cases today where linkage errors are thrown at a later time but I can't imagine anything reliably relying on that. David's suggestion for an interim compatibility flag could be useful to have.
13-10-2018
I don't see that there is any source incompatibility issue here - forName can already throw LinkageError and it is not a checked exception so noone is forced to catch it. I see the risk as low. The only case that would be exposed is an erroneous class that is loaded but otherwise unused - that seems an extreme edge case. Why would you load but never use a class? My recommendation would be to fix the bug and add a flag/property that allows the old behaviour - at least for a release or 2. Otherwise modify the spec for forName to not return a linked class (unless also initialized of course) and add a new overload that takes an additional "link" flag. (Or you could change the default behaviour to link the class and provide the new version as a way for the user to choose not to link - a more permanent workaround if you like.)
12-10-2018
The Class::forName spec states that this method attempts to locate, load, and link the class or interface. The implementation does not match the specification and it's unnoticed for so many years as the class will be linked when it's used in practice. Note that the implementation for reflection and method handle in resolving a member will call InstanceKlass::link_class to ensure the class is linked. It seems that the compatibility risk is medium if Class::forName implementation is fixed to match the specification. It may break existing code that calls Class.forName("Foo", false, loader) but Foo is never used after loaded and per the spec, it should have failed with LinkageError (e.g. fails verification) if linked. It would be a source incompatibility change for such application that attempts to load classes without linking and it has to catch LinkageError.
12-10-2018