All registration after analysis throw UserError.

Refactorization of sealed across implementations of ConditionalConfigurationRegistry.

Author: kimeta

substratevm/src/com.oracle.svm.hosted.foreign/src/com/oracle/svm/hosted/foreign/ForeignFunctionsFeature.java

@@ -155,7 +155,6 @@ public class ForeignFunctionsFeature implements InternalFeature {
                                     "jdk.internal.foreign.layout"});
 
     /** Indicates if the registration of stubs is no longer allowed. */
-    private boolean sealed;
     private RuntimeForeignAccessSupportImpl accessSupport;
 
     /** Indicates if at least one stub was registered. */
@@ -173,10 +172,6 @@ public static ForeignFunctionsFeature singleton() {
         return ImageSingletons.lookup(ForeignFunctionsFeature.class);
     }
 
-    private void checkNotSealed() {
-        UserError.guarantee(!sealed, "Registration of foreign functions was closed.");
-    }
-
     /**
      * Descriptor that represents both, up- and downcalls.
      */
@@ -219,7 +214,7 @@ void duringSetup(AnalysisMetaAccess metaAccess, AnalysisUniverse analysisUnivers
 
         @Override
         public void registerForDowncall(ConfigurationCondition condition, FunctionDescriptor desc, Linker.Option... options) {
-            checkNotSealed();
+            abortIfSealed();
             try {
                 LinkerOptions linkerOptions = LinkerOptions.forDowncall(desc, options);
                 SharedDesc sharedDesc = new SharedDesc(desc, linkerOptions);
@@ -231,7 +226,7 @@ public void registerForDowncall(ConfigurationCondition condition, FunctionDescri
 
         @Override
         public void registerForUpcall(ConfigurationCondition condition, FunctionDescriptor desc, Linker.Option... options) {
-            checkNotSealed();
+            abortIfSealed();
             try {
                 LinkerOptions linkerOptions = LinkerOptions.forUpcall(desc, options);
                 SharedDesc sharedDesc = new SharedDesc(desc, linkerOptions);
@@ -243,7 +238,7 @@ public void registerForUpcall(ConfigurationCondition condition, FunctionDescript
 
         @Override
         public void registerForDirectUpcall(ConfigurationCondition condition, MethodHandle target, FunctionDescriptor desc, Linker.Option... options) {
-            checkNotSealed();
+            abortIfSealed();
             DirectMethodHandleDesc directMethodHandleDesc = target.describeConstable()
                             .filter(x -> x instanceof DirectMethodHandleDesc dmh && dmh.kind() == Kind.STATIC)
                             .map(x -> ((DirectMethodHandleDesc) x))
@@ -706,7 +701,7 @@ public void beforeAnalysis(BeforeAnalysisAccess a) {
 
     @Override
     public void afterAnalysis(AfterAnalysisAccess access) {
-        sealed = true;
+        accessSupport.sealed();
         if (!ForeignFunctionsRuntime.areFunctionCallsSupported() && stubsRegistered) {
             assert getCreatedDowncallStubsCount() == 0;
             assert getCreatedUpcallStubsCount() == 0;
@@ -840,17 +835,17 @@ public boolean handleInvoke(GraphBuilderContext b, ResolvedJavaMethod method, Va
     /* Testing and reporting interface */
 
     public int getCreatedDowncallStubsCount() {
-        assert sealed;
+        assert accessSupport.isSealed();
         return foreignFunctionsRuntime.getDowncallStubsCount();
     }
 
     public int getCreatedUpcallStubsCount() {
-        assert sealed;
+        assert accessSupport.isSealed();
         return foreignFunctionsRuntime.getUpcallStubsCount();
     }
 
     public int getCreatedDirectUpcallStubsCount() {
-        assert sealed;
+        assert accessSupport.isSealed();
         return foreignFunctionsRuntime.getDirectUpcallStubsCount();
     }
 }

substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/ConditionalConfigurationRegistry.java

@@ -32,6 +32,7 @@
 import java.util.concurrent.ConcurrentLinkedQueue;
 import java.util.function.Consumer;
 
+import com.oracle.svm.core.util.UserError;
 import com.oracle.graal.pointsto.meta.AnalysisUniverse;
 import org.graalvm.nativeimage.hosted.Feature;
 import org.graalvm.nativeimage.impl.ConfigurationCondition;
@@ -42,6 +43,7 @@
 public abstract class ConditionalConfigurationRegistry {
     private Feature.BeforeAnalysisAccess beforeAnalysisAccess;
     private SVMHost hostVM;
+    private boolean sealed = false;
     protected AnalysisUniverse universe;
     private final Map<Class<?>, Collection<Runnable>> pendingReachabilityHandlers = new ConcurrentHashMap<>();
     private final Set<ConditionalTask> pendingConditionalTasks = ConcurrentHashMap.newKeySet();
@@ -117,6 +119,24 @@ protected String nullErrorMessage(String elementKind, String accessKind) {
         return "Cannot register null value as " + elementKind + " for " + accessKind + ". Please ensure that all values you register are not null.";
     }
 
+    public void sealed() {
+        sealed = true;
+    }
+
+    public boolean isSealed() {
+        return sealed;
+    }
+
+    public void abortIfSealed() {
+        /*
+         * The UserError needs a cause argument to print out the stack trace, so users can see
+         * exactly where the exception occurred.
+         */
+        if (sealed) {
+            throw UserError.abort(new IllegalStateException(), "All elements must be registered for runtime access before the analysis has completed.");
+        }
+    }
+
     public void setHostVM(SVMHost hostVM) {
         this.hostVM = hostVM;
     }

substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/ResourcesFeature.java

@@ -160,7 +160,7 @@ public static class Options {
         public static final HostedOptionKey<Boolean> GenerateEmbeddedResourcesFile = new HostedOptionKey<>(false);
     }
 
-    private boolean sealed = false;
+    private ResourcesRegistryImpl resourcesRegistry;
 
     private record ConditionalPattern(ConfigurationCondition condition, String pattern, Object origin) {
     }
@@ -229,31 +229,34 @@ public void addResourceEntry(Module module, String resourcePath, Object origin)
 
         @Override
         public void injectResource(Module module, String resourcePath, byte[] resourceContent, Object origin) {
+            abortIfSealed();
             EmbeddedResourcesInfo.singleton().declareResourceAsRegistered(module, resourcePath, "INJECTED", origin);
             Resources.currentLayer().registerResource(module, resourcePath, resourceContent);
         }
 
         @Override
         public void ignoreResources(ConfigurationCondition condition, String pattern, Object origin) {
+            abortIfSealed();
             registerConditionalConfiguration(condition, (cnd) -> {
-                UserError.guarantee(!sealed, "Resources ignored too late: %s", pattern);
-
                 excludedResourcePatterns.add(new ConditionalPattern(condition, pattern, origin));
             });
         }
 
         @Override
         public void addResourceBundles(ConfigurationCondition condition, String name) {
+            abortIfSealed();
             registerConditionalConfiguration(condition, (cnd) -> ImageSingletons.lookup(LocalizationFeature.class).prepareBundle(cnd, name));
         }
 
         @Override
         public void addClassBasedResourceBundle(ConfigurationCondition condition, String basename, String className) {
+            abortIfSealed();
             registerConditionalConfiguration(condition, (cnd) -> ImageSingletons.lookup(LocalizationFeature.class).prepareClassResourceBundle(basename, className));
         }
 
         @Override
         public void addResourceBundles(ConfigurationCondition condition, String basename, Collection<Locale> locales) {
+            abortIfSealed();
             registerConditionalConfiguration(condition, (cnd) -> ImageSingletons.lookup(LocalizationFeature.class).prepareBundle(cnd, basename, locales));
         }
 
@@ -388,7 +391,7 @@ private void registerResource(Module module, String resourcePath, boolean fromJa
     public void afterRegistration(AfterRegistrationAccess a) {
         FeatureImpl.AfterRegistrationAccessImpl access = (FeatureImpl.AfterRegistrationAccessImpl) a;
         imageClassLoader = access.getImageClassLoader();
-        ResourcesRegistryImpl resourcesRegistry = new ResourcesRegistryImpl();
+        resourcesRegistry = new ResourcesRegistryImpl();
         ImageSingletons.add(ResourcesRegistry.class, resourcesRegistry);
         ImageSingletons.add(RuntimeResourceSupport.class, resourcesRegistry);
         EmbeddedResourcesInfo embeddedResourcesInfo = new EmbeddedResourcesInfo();
@@ -649,7 +652,7 @@ boolean moduleNameMatches(String resourceContainerModuleName) {
 
     @Override
     public void afterAnalysis(AfterAnalysisAccess access) {
-        sealed = true;
+        resourcesRegistry.sealed();
         if (Options.GenerateEmbeddedResourcesFile.getValue()) {
             Path reportLocation = NativeImageGenerator.generatedFiles(HostedOptionValues.singleton()).resolve(Options.EMBEDDED_RESOURCES_FILE_NAME);
             try (JsonWriter writer = new JsonWriter(reportLocation)) {
@@ -709,7 +712,7 @@ public boolean isDecorator() {
 
             @Override
             public boolean apply(GraphBuilderContext b, ResolvedJavaMethod targetMethod, Receiver receiver, ValueNode arg) {
-                VMError.guarantee(!sealed, "All bytecode parsing happens before the analysis, i.e., before the registry is sealed");
+                VMError.guarantee(!resourcesRegistry.isSealed(), "All bytecode parsing happens before the analysis, i.e., before the registry is sealed");
                 Class<?> clazz = SubstrateGraphBuilderPlugins.asConstantObject(b, Class.class, receiver.get(false));
                 String resource = SubstrateGraphBuilderPlugins.asConstantObject(b, String.class, arg);
                 if (clazz != null && resource != null) {

substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/jni/JNIAccessFeature.java

@@ -165,9 +165,8 @@ static final class JNICallableJavaMethod {
         }
     }
 
-    private JNIRuntimeAccessibilitySupportImpl runtimeSupport;
+    private JNIRuntimeAccessibilitySupportImpl runtimeSupport = new JNIRuntimeAccessibilitySupportImpl();
 
-    private boolean sealed = false;
     private final Map<String, JNICallTrampolineMethod> trampolineMethods = new ConcurrentHashMap<>();
     private final Map<ResolvedSignature<ResolvedJavaType>, JNIJavaCallWrapperMethod> javaCallWrapperMethods = new ConcurrentHashMap<>();
     private final Map<ResolvedSignature<ResolvedJavaType>, JNIJavaCallVariantWrapperGroup> callVariantWrappers = new ConcurrentHashMap<>();
@@ -191,10 +190,6 @@ public static class Options {
         public static final HostedOptionKey<Boolean> PrintJNIMethods = new HostedOptionKey<>(false);
     }
 
-    private void abortIfSealed() {
-        UserError.guarantee(!sealed, "Classes, methods and fields must be registered for JNI access before the analysis has completed.");
-    }
-
     @Override
     public List<Class<? extends Feature>> getRequiredFeatures() {
         // Ensure that KnownOffsets is fully initialized before we access it
@@ -207,7 +202,6 @@ public void afterRegistration(AfterRegistrationAccess arg) {
 
         JNIReflectionDictionary.create();
 
-        runtimeSupport = new JNIRuntimeAccessibilitySupportImpl();
         ImageSingletons.add(RuntimeJNIAccessSupport.class, runtimeSupport);
 
         ConfigurationConditionResolver<ConfigurationCondition> conditionResolver = new NativeImageConditionResolver(access.getImageClassLoader(),
@@ -347,7 +341,7 @@ public JNICallTrampolineMethod getOrCreateCallTrampolineMethod(MetaAccessProvide
     }
 
     public JNINativeLinkage makeLinkage(String declaringClass, String name, String descriptor) {
-        UserError.guarantee(!sealed,
+        UserError.guarantee(!runtimeSupport.isSealed(),
                         "All linkages for JNI calls must be created before the analysis has completed.%nOffending class: %s name: %s descriptor: %s",
                         declaringClass, name, descriptor);
 
@@ -539,9 +533,9 @@ private static void addNegativeFieldLookup(Class<?> declaringClass, String field
     @Override
     @SuppressWarnings("unused")
     public void afterAnalysis(AfterAnalysisAccess access) {
-        sealed = true;
+        runtimeSupport.sealed();
         if (wereElementsAdded()) {
-            abortIfSealed();
+            runtimeSupport.abortIfSealed();
         }
 
         int numClasses = 0;

substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/reflect/ReflectionDataBuilder.java

@@ -124,8 +124,6 @@ public class ReflectionDataBuilder extends ConditionalConfigurationRegistry impl
     private final ClassForNameSupport classForNameSupport;
     private LayeredReflectionDataBuilder layeredReflectionDataBuilder;
 
-    private boolean sealed;
-
     // Reflection data
     private final Map<Class<?>, RecordComponent[]> registeredRecordComponents = new ConcurrentHashMap<>();
 
@@ -186,9 +184,7 @@ public void beforeAnalysis(BeforeAnalysisAccessImpl beforeAnalysisAccess) {
     }
 
     private void runConditionalInAnalysisTask(ConfigurationCondition condition, Consumer<ConfigurationCondition> task) {
-        if (sealed) {
-            throw new UnsupportedFeatureException("Too late to add classes, methods, and fields for reflective access. Registration must happen in a Feature before the analysis has finished.");
-        }
+        abortIfSealed();
         runConditionalTask(condition, task);
     }
 
@@ -1174,7 +1170,7 @@ private static void reportLinkingErrors(Class<?> clazz, List<Throwable> errors)
     }
 
     protected void afterAnalysis() {
-        sealed = true;
+        sealed();
         processedTypes = null;
         if (!throwMissingRegistrationErrors()) {
             pendingRecordClasses = null;
@@ -1184,7 +1180,7 @@ protected void afterAnalysis() {
 
     @Override
     public Map<Class<?>, Set<Class<?>>> getReflectionInnerClasses() {
-        assert sealed;
+        assert isSealed();
         return Collections.unmodifiableMap(innerClasses);
     }
 
@@ -1203,37 +1199,37 @@ public int getEnabledReflectionQueries(Class<?> clazz) {
 
     @Override
     public Map<AnalysisType, Map<AnalysisField, ConditionalRuntimeValue<Field>>> getReflectionFields() {
-        assert sealed;
+        assert isSealed();
         return Collections.unmodifiableMap(registeredFields);
     }
 
     @Override
     public Map<AnalysisType, Map<AnalysisMethod, ConditionalRuntimeValue<Executable>>> getReflectionExecutables() {
-        assert sealed;
+        assert isSealed();
         return Collections.unmodifiableMap(registeredMethods);
     }
 
     @Override
     public Object getAccessor(AnalysisMethod method) {
-        assert sealed;
+        assert isSealed();
         return methodAccessors.get(method);
     }
 
     @Override
     public Set<ResolvedJavaField> getHidingReflectionFields() {
-        assert sealed;
+        assert isSealed();
         return Collections.unmodifiableSet(hidingFields);
     }
 
     @Override
     public Set<ResolvedJavaMethod> getHidingReflectionMethods() {
-        assert sealed;
+        assert isSealed();
         return Collections.unmodifiableSet(hidingMethods);
     }
 
     @Override
     public RecordComponent[] getRecordComponents(Class<?> type) {
-        assert sealed;
+        assert isSealed();
         return registeredRecordComponents.get(type);
     }
 
@@ -1242,7 +1238,7 @@ public void registerHeapDynamicHub(Object object, ScanReason reason) {
         DynamicHub hub = (DynamicHub) object;
         Class<?> javaClass = hub.getHostedJavaClass();
         if (heapDynamicHubs.add(hub)) {
-            if (sealed) {
+            if (isSealed()) {
                 throw new UnsupportedFeatureException("Registering new class for reflection when the image heap is already sealed: " + javaClass);
             }
             if (!SubstitutionReflectivityFilter.shouldExclude(javaClass, metaAccess, universe)) {
@@ -1253,15 +1249,15 @@ public void registerHeapDynamicHub(Object object, ScanReason reason) {
 
     @Override
     public Set<DynamicHub> getHeapDynamicHubs() {
-        assert sealed;
+        assert isSealed();
         return Collections.unmodifiableSet(heapDynamicHubs);
     }
 
     @Override
     public void registerHeapReflectionField(Field reflectField, ScanReason reason) {
         AnalysisField analysisField = metaAccess.lookupJavaField(reflectField);
         if (heapFields.put(analysisField, reflectField) == null) {
-            if (sealed) {
+            if (isSealed()) {
                 throw new UnsupportedFeatureException("Registering new field for reflection when the image heap is already sealed: " + reflectField);
             }
             if (!SubstitutionReflectivityFilter.shouldExclude(reflectField, metaAccess, universe)) {
@@ -1286,7 +1282,7 @@ public void registerHeapReflectionExecutable(Executable reflectExecutable, ScanR
         }
         AnalysisMethod analysisMethod = metaAccess.lookupJavaMethod(reflectExecutable);
         if (heapMethods.put(analysisMethod, reflectExecutable) == null) {
-            if (sealed) {
+            if (isSealed()) {
                 throw new UnsupportedFeatureException("Registering new method for reflection when the image heap is already sealed: " + reflectExecutable);
             }
             if (!SubstitutionReflectivityFilter.shouldExclude(reflectExecutable, metaAccess, universe)) {
@@ -1300,13 +1296,13 @@ public void registerHeapReflectionExecutable(Executable reflectExecutable, ScanR
 
     @Override
     public Map<AnalysisField, Field> getHeapReflectionFields() {
-        assert sealed;
+        assert isSealed();
         return Collections.unmodifiableMap(heapFields);
     }
 
     @Override
     public Map<AnalysisMethod, Executable> getHeapReflectionExecutables() {
-        assert sealed;
+        assert isSealed();
         return Collections.unmodifiableMap(heapMethods);
     }
 
@@ -1353,21 +1349,21 @@ public Map<Class<?>, Throwable> getRecordComponentLookupErrors() {
     private static final AnnotationValue[] NO_ANNOTATIONS = new AnnotationValue[0];
 
     public AnnotationValue[] getAnnotationData(AnnotatedElement element) {
-        assert sealed;
+        assert isSealed();
         return filteredAnnotations.getOrDefault(element, NO_ANNOTATIONS);
     }
 
     private static final AnnotationValue[][] NO_PARAMETER_ANNOTATIONS = new AnnotationValue[0][0];
 
     public AnnotationValue[][] getParameterAnnotationData(AnalysisMethod element) {
-        assert sealed;
+        assert isSealed();
         return filteredParameterAnnotations.getOrDefault(element, NO_PARAMETER_ANNOTATIONS);
     }
 
     private static final TypeAnnotationValue[] NO_TYPE_ANNOTATIONS = new TypeAnnotationValue[0];
 
     public TypeAnnotationValue[] getTypeAnnotationData(AnnotatedElement element) {
-        assert sealed;
+        assert isSealed();
         return filteredTypeAnnotations.getOrDefault(element, NO_TYPE_ANNOTATIONS);
     }