RFR: 8284616: Implement workaround for CODETOOLS-7901986 in serviceability/jvmti/RedefineClasses

[Ioi Lam]

On Mon, 11 Apr 2022 07:52:34 GMT, Alan Bateman <alanb at openjdk.org> wrote:

> > @AlanBateman adding the explicit compile commands to add `--enable-preview` is exactly what causes the problem. By compiling that individual file first it also compiled some testlib dependencies but puts the classes in a different place. When another class is later compiled, the compilation path includes that destination and so compilation succeeds, but at runtime the path is different and we get the NCDFE. This is the analysis that Alex did in a similar issue - see his comment in JBS.
> 
> Adding `--enable-preview` did not intentionally mean to introduce implicit compilation but it looks like we'll have to re-visit the ordering (as @alexmenkov suggests) in the loom repo.

I believe the underlying cause of this type of failure is https://bugs.openjdk.java.net/browse/CODETOOLS-7902847 "Class directory of a test case should not be used to compile a library"

You can easily see the bug by adding this to test/hotspot/jtreg/serviceability/jvmti/RedefineClasses/RedefineRunningMethods.java in the mainline JDK repo:


+ * @compile RedefineRunningMethods.java
  * @run main RedefineClassHelper


Clean up your jtreg "work" directory and run this test by itself. Afterwards, look for .class files in the jtreg work dir:


./work/classes/test/lib/RedefineClassHelper.class
./work/classes/serviceability/jvmti/RedefineClasses/RedefineRunningMethods.d/RedefineRunningMethods.class
./work/classes/serviceability/jvmti/RedefineClasses/RedefineRunningMethods.d/RedefineRunningMethods$2.class
./work/classes/serviceability/jvmti/RedefineClasses/RedefineRunningMethods.d/jdk/test/lib/compiler/InMemoryJavaCompiler$FileManagerWrapper$1.class
./work/classes/serviceability/jvmti/RedefineClasses/RedefineRunningMethods.d/jdk/test/lib/compiler/InMemoryJavaCompiler$FileManagerWrapper.class
./work/classes/serviceability/jvmti/RedefineClasses/RedefineRunningMethods.d/jdk/test/lib/compiler/InMemoryJavaCompiler$MemoryJavaFileObject.class
./work/classes/serviceability/jvmti/RedefineClasses/RedefineRunningMethods.d/jdk/test/lib/compiler/InMemoryJavaCompiler.class
./work/classes/serviceability/jvmti/RedefineClasses/RedefineRunningMethods.d/jdk/test/lib/helpers/ClassFileInstaller$Manifest.class
./work/classes/serviceability/jvmti/RedefineClasses/RedefineRunningMethods.d/jdk/test/lib/helpers/ClassFileInstaller.class
./work/classes/serviceability/jvmti/RedefineClasses/RedefineRunningMethods.d/RedefineRunningMethods$1.class
./work/classes/serviceability/jvmti/RedefineClasses/RedefineRunningMethods.d/RedefineRunningMethods$3.class
./work/classes/serviceability/jvmti/RedefineClasses/RedefineRunningMethods.d/RedefineClassHelper.class
./work/classes/serviceability/jvmti/RedefineClasses/RedefineRunningMethods.d/RedefineRunningMethods_B.class


You can see that the `@library /test/lib` is split into two parts: /work/classes/test/lib/

- Most of it is in the directory that's used only by the RedefineRunningMethods.java test case
- Some of it is in the `work/classes/test/lib/` directory, which is shared by all test cases.

Now, create a test case RedefineRunningMethods2.java in the same directory that looks like this:


/*
 * @test
 * @requires vm.jvmti
 * @library /test/lib
 * @modules java.base/jdk.internal.misc
 * @modules java.compiler
 *          java.instrument
 *          jdk.jartool/sun.tools.jar
 * @run main RedefineClassHelper
 */


If you run these two test together, RedefineRunningMethods.java is executed first, leaving the work directory in the same condition as shown above.

Then, jtreg runs RedefineRunningMethods2.java, and sees `work/classes/test/lib/RedefineClassHelper.class` is already there. So it executes RedefineClassHelper without rebuilding it, leading to a failure because classes that RedefineClassHelper depends on are not available to this second test.

I have a very simple reproducer in the bug above with a detailed log that shows what is happening.

While this problem could be worked around, I believe the right fix should be done inside jtreg.

-------------

PR: https://git.openjdk.java.net/jdk/pull/8170