Are lambdas for java.base defined in the unnamed module?

[andresluuk]

I built openJ9 myself on my local machine and added the following log line in createramclass.cpp~2810 after
module = javaVM->unamedModuleForSystemLoader;
printf("creatramclass.internalCreateRAMClassFromROMClass.unamed %s\n", J9UTF8_DATA(className));

Now in the log i see the following line:
creatramclass.internalCreateRAMClassFromROMClass.unamed jdk/internal/loader/BuiltinClassLoader$$Lambda$1/000000009B3D8510

So I got a question, are the lambdas for java.base classes like (BuiltinClassLoader) defined in unnamed module, or I am reading something wrong.

[DanHeidinga]

@hangshao0 Can you take a look at this?

[hangshao0]

Yes, I will take a look.

[hangshao0]

The default loadLocationType for Java.base is LOAD_LOCATION_UNKNOWN. If I change that to LOAD_LOCATION_MODULE, this issue goes away.

[DanHeidinga]

Lambda's are defined with jdk.internal.misc.Unsafe.defineAnonymousClass(). The module for the anonymous class should be the same as the hostclass.

Does keeping that relationship require changing the default LOAD Location?

[hangshao0]

Does keeping that relationship require changing the default LOAD Location?

In createramclass.cpp, we assign modules based on the load location when J9_RUNTIME_JAVA_BASE_MODULE_CREATED is not set.

if (J9ROMCLASS_IS_PRIMITIVE_TYPE(romClass)
	|| (LOAD_LOCATION_PATCH_PATH == locationType)
	|| (LOAD_LOCATION_MODULE == locationType)
) {
	module = javaVM->javaBaseModule;
} else {
        module = javaVM->unamedModuleForSystemLoader;
}

Directly assigning hostClass->module to module also works, which looks like a better solution. I have updated the change to set module to host class module.

[DanHeidinga]

Thanks for updating this.

Looking through the code, I wonder what happens to anonymous classes that are defined before java.base has been created?

I wonder if the behaviour difference JRebel has seen with classes that are defined "early" being in the unnamed module on OpenJ9 and the java.base in the RI could be due to the same kind of issue. (#2751)

What if we assigned every class created before java.base is created to java.base? ie: modify this code: https://github.com/hangshao0/openj9/blob/c6fca26da528cd78742e7f224784391110cdb6ff/runtime/vm/createramclass.cpp#L2806 to remove the special cases and unconditionally assign to java.base.

Would that solve both issues? Would that make our behaviour more closely match the RI?

[hangshao0]

Looking through the code, I wonder what happens to anonymous classes that are defined before java.base has been created?

loadLocationType is set in dynload.c, for anonymous classes, because the className is empty and classNameLength is 0, searchClassInCPEntry() is not able to find the resource in Jimage, it returns a non-zero value, so we never set load location type to LOAD_LOCATION_MODULE. The default load location type is LOAD_LOCATION_UNKNOWN, which leads us to assign unamed module to anonymous classes.

Would that solve both issues? Would that make our behaviour more closely match the RI?

I think so.

[DanHeidinga]

Let's give that a try then. Maybe @andresluuk would be willing to test JRebel on a build with that change to confirm it makes OpenJ9 behave the same as the RI.

[hangshao0]

OK. Then I will mark PR #2845 as "WIP" for now, until we get confirmation from @andresluuk

[andresluuk]

Yes, I would like to try the fix. I will try to build #2845 tomorrow morning.

[andresluuk]

Good news everyone!
Defining everything before java.base is defined into java.base works for us.
With that small change I managed to get jrebel working.

There is also the additional benefit of avoiding the JVM crashes.
Before the change, when I defined a class that went to the unnamed module, I would get a JVM crash with a access check error and a core dump.
Now I didn't get it, but got a few Javas normal illegal access errors, that I managed to resolve easily.

When can I except to see this change in a nightly/release version of openJ9 (JDK9/10/11 etc)?

[DanHeidinga]

The change is fairly straightforward and therefore easy to review. I'm confident we'll get this into nightly builds very soon (maybe even tonight!).

Depending on when we get it merged, it may make the OpenJ9 0.10.0 release which will be the initial JDK11 release scheduled for the end of Sept. It will certainly be in for the 0.11.0 release scheduled for the end of Oct which will target JDK 8, 10 & 11. JDK 9 is no longer actively built.