Skip to content

8368199: Add @AOTSafeClassInitializer to jdk.internal.access.SharedSecrets #27880

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 4 commits into from
Closed

8368199: Add @AOTSafeClassInitializer to jdk.internal.access.SharedSecrets #27880

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a886aa8
8368199: Add @AOTSafeClassInitializer to jdk.internal.access.SharedSe…
iklam Oct 17, 2025
c6ea93c
Added StatelessOopsFinder
iklam Oct 28, 2025
6ae1172
Merge branch 'master' into 8368199-add-aot-safe-class-initializer-ann…
iklam Oct 28, 2025
3121fd1
Updated comments about @AOTSafeClassInitializer in SharedSecrets.java
iklam Oct 28, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions src/hotspot/share/cds/cdsHeapVerifier.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,9 +181,10 @@ class CDSHeapVerifier::CheckStaticFields : public FieldClosure {
return;
}

if (fd->signature()->equals("Ljdk/internal/access/JavaLangAccess;")) {
// A few classes have static fields that point to SharedSecrets.getJavaLangAccess().
// This object carries no state and we can create a new one in the production run.
if (fd->signature()->starts_with("Ljdk/internal/access/") &&
fd->signature()->ends_with("Access;")) {
// The jdk/internal/access/*Access classes carry no states so they can be safely
Copy link
Contributor

@minborg minborg Oct 27, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might be true for the time being, but adding such an assumption is a constraint for the future and should be documented. Perhaps we should have an interface Access that the various access classes implement, and where we could document this and other constraints of the access classes.

Copy link
Member Author

@iklam iklam Oct 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed this hard-coded check and instead added CDSHeapVerifier::add_shared_secret_accessors(), which requires all AOT-initialized accessors to be stateless.

I also added a negative test case for SharedSecrets::javaObjectInputFilterAccess, which is not stateless so it cannot be initialized in the AOT assembly phase.

// cached.
return;
}
oop static_obj_field = _ik->java_mirror()->obj_field(fd->offset());
Expand Down
6 changes: 0 additions & 6 deletions src/java.base/share/classes/java/lang/invoke/MethodHandleImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@
import jdk.internal.foreign.abi.NativeEntryPoint;
import jdk.internal.reflect.CallerSensitive;
import jdk.internal.reflect.Reflection;
import jdk.internal.vm.annotation.AOTRuntimeSetup;
import jdk.internal.vm.annotation.AOTSafeClassInitializer;
import jdk.internal.vm.annotation.ForceInline;
import jdk.internal.vm.annotation.Hidden;
Expand Down Expand Up @@ -1536,11 +1535,6 @@ private static NamedFunction createFunction(byte func) {
}

static {
runtimeSetup();
}

@AOTRuntimeSetup
private static void runtimeSetup() {
SharedSecrets.setJavaLangInvokeAccess(new JavaLangInvokeAccess() {
@Override
public Class<?> getDeclaringClass(Object rmname) {
Expand Down
6 changes: 0 additions & 6 deletions src/java.base/share/classes/java/lang/module/ModuleDescriptor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@

import jdk.internal.module.Checks;
import jdk.internal.module.ModuleInfo;
import jdk.internal.vm.annotation.AOTRuntimeSetup;
import jdk.internal.vm.annotation.AOTSafeClassInitializer;


Expand Down Expand Up @@ -2668,11 +2667,6 @@ private static <E extends Enum<E>> long modsValue(Set<E> set) {
}

static {
runtimeSetup();
}

@AOTRuntimeSetup
private static void runtimeSetup() {
/**
* Setup the shared secret to allow code in other packages access
* private package methods in java.lang.module.
Expand Down
6 changes: 0 additions & 6 deletions src/java.base/share/classes/java/lang/ref/Reference.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@

package java.lang.ref;

import jdk.internal.vm.annotation.AOTRuntimeSetup;
import jdk.internal.vm.annotation.AOTSafeClassInitializer;
import jdk.internal.vm.annotation.ForceInline;
import jdk.internal.vm.annotation.IntrinsicCandidate;
Expand Down Expand Up @@ -291,11 +290,6 @@ static void startReferenceHandlerThread(ThreadGroup tg) {
}

static {
runtimeSetup();
}

@AOTRuntimeSetup
private static void runtimeSetup() {
// provide access in SharedSecrets
SharedSecrets.setJavaLangRefAccess(new JavaLangRefAccess() {
@Override
Expand Down
6 changes: 0 additions & 6 deletions src/java.base/share/classes/java/net/URI.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@
import jdk.internal.access.JavaNetUriAccess;
import jdk.internal.access.SharedSecrets;
import jdk.internal.util.Exceptions;
import jdk.internal.vm.annotation.AOTRuntimeSetup;
import jdk.internal.vm.annotation.AOTSafeClassInitializer;
import sun.nio.cs.UTF_8;

Expand Down Expand Up @@ -3731,11 +3730,6 @@ private int scanHexSeq(int start, int n)
}

static {
runtimeSetup();
}

@AOTRuntimeSetup
private static void runtimeSetup() {
SharedSecrets.setJavaNetUriAccess(
new JavaNetUriAccess() {
public URI create(String scheme, String path) {
Expand Down
6 changes: 0 additions & 6 deletions src/java.base/share/classes/java/net/URL.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@
import jdk.internal.access.JavaNetURLAccess;
import jdk.internal.access.SharedSecrets;
import jdk.internal.misc.VM;
import jdk.internal.vm.annotation.AOTRuntimeSetup;
import jdk.internal.vm.annotation.AOTSafeClassInitializer;
import sun.net.util.IPAddressUtil;
import static jdk.internal.util.Exceptions.filterNonSocketInfo;
Expand Down Expand Up @@ -1747,11 +1746,6 @@ private void setSerializedHashCode(int hc) {
}

static {
runtimeSetup();
}

@AOTRuntimeSetup
private static void runtimeSetup() {
SharedSecrets.setJavaNetURLAccess(
new JavaNetURLAccess() {
@Override
Expand Down
4 changes: 4 additions & 0 deletions src/java.base/share/classes/jdk/internal/access/SharedSecrets.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@

package jdk.internal.access;

import jdk.internal.vm.annotation.AOTSafeClassInitializer;
import jdk.internal.vm.annotation.Stable;

import javax.crypto.SealedObject;
Expand Down Expand Up @@ -60,6 +61,9 @@ interface and provides the ability to call package-private methods
* </strong>
*/

// Static fields in this class are stateless, so the values initialized in the
// AOT assembly phase can be safely cached.
Copy link
Contributor

@DanHeidinga DanHeidinga Oct 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking through the implementations of the Access classes, and I have concerns about:
setJavaObjectInputFilterAccess as it is implemented using a lambda:

SharedSecrets.setJavaObjectInputFilterAccess(Config::createFilter2);

Correct me if I'm wrong, but I believe that will cause the Config class to be AOTInitialized as well?

Config has a couple of system properties (-Djdk.serialFilter= for one) that we may not want to initialize during the assembly phase.

Copy link
Contributor

@DanHeidinga DanHeidinga Oct 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There may be a similar issue with ObjectInputStream as well as I think this forces the class to be AOTInitialized.

        SharedSecrets.setJavaObjectInputStreamAccess(ObjectInputStream::checkArray);
        SharedSecrets.setJavaObjectInputStreamReadString(ObjectInputStream::readString);

Copy link
Member Author

@iklam iklam Oct 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These two accessors are currently not used in the AOT assembly phase. Maybe we can add an assert that the corresponding fields are null, and abort the AOT assembly otherwise?

Copy link
Contributor

@DanHeidinga DanHeidinga Oct 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a particular subset of the SharedSecrets accessors that we want to allow to be set during the assembly phase?

Is there a way we can mark the fields in SharedSecrets as allowed to be assembly initialized vs those that must be null?

The unfortunate thing is that if these fields didn't use Lambdas, they would also be fine to assembly-time initialize as it's the side-effect of the lambda forcing init that's the problem

Copy link
Member Author

@iklam iklam Oct 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I looked at all the calls of the pattern SharedSecrets.set.*::

java/io/ObjectInputFilter.java: SharedSecrets.setJavaObjectInputFilterAccess(Config::createFilter2);
java/io/ObjectInputStream.java: SharedSecrets.setJavaObjectInputStreamAccess(ObjectInputStream::checkArray);
java/io/ObjectInputStream.java: SharedSecrets.setJavaObjectInputStreamReadString(ObjectInputStream::readString);
javax/crypto/SealedObject.java: SharedSecrets.setJavaxCryptoSealedObjectAccess(SealedObject::getExtObjectInputStream);

These calls are all done inside a <clinit>. In the four cases, only the first class (java.io.ObjectInputFilter.Config) has environment-dependent code inside its <clinit>.

Maybe we should mark the java.io.ObjectInputFilter.Config class with a new annotation AOTUnsafeClassInitializer (the opposite of the existing AOTSafeClassInitializer). If this class is initialized in the assembly phase, the VM will exit.

I think we can leave the other 3 cases alone.

An alternative is to rewrite the first case from:

SharedSecrets.setJavaObjectInputFilterAccess(Config::createFilter2);

to

SharedSecrets.setJavaObjectInputFilterAccess(new JavaObjectInputFilterAccess() {
    ObjectInputFilter createFilter2(String pattern) {
        return Config.createFilter2(pattern);
    }
});

Copy link

@ExE-Boss ExE-Boss Oct 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The ObjectInputStreamReadString interface should just be merged into ObjectInputStreamAccess:

SharedSecrets.setJavaObjectInputStreamAccess(new ObjectInputStreamAccess() {
	public void checkArray(ObjectInputStream ois, Class<?> arrayType, int arrayLength) throws ObjectStreamException {
		ois.checkArray(arrayType, arrayLength);
	}

	public String readString(ObjectInputStream ois) throws IOException {
		return ois.readString();
	}
});

Copy link
Member Author

@iklam iklam Oct 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The ObjectInputStreamReadString interface should just be merged into ObjectInputStreamAccess.

I agree. This seems better than using two separate Access interfaces with two separate lambdas. Maybe this should be done in a separate RFE?

Copy link
Member Author

@iklam iklam Oct 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a particular subset of the SharedSecrets accessors that we want to allow to be set during the assembly phase?

@DanHeidinga , I updated the code to disallow any AOT-initialized accessors that are not stateless. See CDSHeapVerifier::SharedSecretsAccessorFinder::do_field(). This check should cover all existing use of Lambdas in setting the accessors, as well as future changes in the core lib that might add other types of states in the accessors.

Copy link
Contributor

@DanHeidinga DanHeidinga Oct 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you update the comment on SharedSecrets.java to indicate that lambdas aren't safe to pre-initialize in this context?

Copy link
Member Author

@iklam iklam Oct 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I updated the comments.

@AOTSafeClassInitializer
public class SharedSecrets {
// This field is not necessarily stable
private static JavaAWTFontAccess javaAWTFontAccess;
Copy link
Member

@liach liach Oct 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does aot initialization work with this field?

Copy link
Member Author

@iklam iklam Oct 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this field is safe. There are two places that could set it, but they will always set it to an instance of JavaAWTFontAccessImpl, which is stateless.

jdk/src/java.desktop/share/classes/java/awt/font/NumericShaper.java

Lines 148 to 149 in eff6439

if (SharedSecrets.getJavaAWTFontAccess() == null) {
SharedSecrets.setJavaAWTFontAccess(new JavaAWTFontAccessImpl());

jdk/src/java.desktop/share/classes/java/awt/font/TextAttribute.java

Lines 251 to 252 in eff6439

if (SharedSecrets.getJavaAWTFontAccess() == null) {
SharedSecrets.setJavaAWTFontAccess(new JavaAWTFontAccessImpl());

Expand Down