Skip to content
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.

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

[GR-51059] Resolve configuration conditions during JSON parsing. #8250

Merged
merged 1 commit into from
Jan 29, 2024

Conversation

graalvmbot
Copy link
Collaborator

Before this PR we were using the following map in ConditionalConfigurationRegistry

public abstract class ConditionalConfigurationRegistry {
    private final Map<String, Collection<Runnable>> pendingReachabilityHandlers = new ConcurrentHashMap<>();

Unfortunately fully qualified Java type names are not suitable as unique keys throughout the lifetime of the analysis. It can easily happen that we have two classes with identical fully qualified Java type names but different meaning because they are coming from two different classloaders.

To correctly adjust to this requirement it means that already on the level of
org.graalvm.nativeimage.impl.ConfigurationCondition there must not be field org.graalvm.nativeimage.impl.ConfigurationCondition#typeName

private final String typeName; 

of type String but instead something like

private final Class<?> type;

consequently the static factory method org.graalvm.nativeimage.impl.ConfigurationCondition#create should be changed as well to only allow creating ConfigurationCondition with actual class objects.

public static ConfigurationCondition create(Class<?> typeReachability) {
    Objects.requireNonNull(typeReachability);
    if (OBJECT_REACHABLE.typeName.equals(typeReachability)) {
        return OBJECT_REACHABLE;
    }
    return new ConfigurationCondition(typeReachability);
}

The only place where it is valid to create ConfigurationCondition from fully qualified Java type names is if we are creating them for json-based configuration entries (e.g. "typeReachable": <fqn> in reflect-config.json).

Key Implementation Notes

The UnresolvedConfigurationCondition is used only during parsing and uses strings to distinguish between types. The ConditionalElement now is also a parsing-only concept.

As a consequence all parsers are now generic in the condition that they use and they accept the ConditionResolver<Cond>.

@oracle-contributor-agreement oracle-contributor-agreement bot added the OCA Verified All contributors have signed the Oracle Contributor Agreement. label Jan 25, 2024
@graalvmbot graalvmbot force-pushed the vj/GR-51059-resolved-condition branch 6 times, most recently from 725bd25 to 58cc679 Compare January 26, 2024 22:59
The UnresolvedConfigurationCondition is used only during parsing and uses strings to distinguish types. The ConditionalElement is also a parsing only concept now.

As a consequence all parsers are now generic in the condition that they use and they accept the ConditionResolver.
@graalvmbot graalvmbot force-pushed the vj/GR-51059-resolved-condition branch from 58cc679 to fff140b Compare January 26, 2024 23:49
@graalvmbot graalvmbot closed this Jan 29, 2024
@graalvmbot graalvmbot merged commit 768a4ad into master Jan 29, 2024
12 checks passed
@graalvmbot graalvmbot deleted the vj/GR-51059-resolved-condition branch January 29, 2024 19:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
OCA Verified All contributors have signed the Oracle Contributor Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants