[
https://issues.apache.org/jira/browse/GROOVY-12223?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18102378#comment-18102378
]
ASF GitHub Bot commented on GROOVY-12223:
-----------------------------------------
blackdrag commented on code in PR #2755:
URL: https://github.com/apache/groovy/pull/2755#discussion_r3727508137
##########
src/main/java/org/apache/groovy/util/HiddenClassDefiner.java:
##########
@@ -0,0 +1,215 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.groovy.util;
+
+import org.codehaus.groovy.control.CompilerConfiguration;
+import org.objectweb.asm.ClassReader;
+import org.objectweb.asm.ClassWriter;
+import org.objectweb.asm.commons.ClassRemapper;
+import org.objectweb.asm.commons.SimpleRemapper;
+
+import java.lang.invoke.MethodHandles;
+import java.lang.invoke.MethodHandles.Lookup;
+import java.util.Collections;
+
+/**
+ * Central facility for defining <em>hidden classes</em>
+ * (<a href="https://openjdk.org/jeps/371";>JEP 371</a>).
+ *
+ * <h2>Lookup ownership (read this first)</h2>
+ * <p>{@link MethodHandles#lookup()} is <em>caller-sensitive</em>: it returns a
+ * full-privilege lookup only for the class that literally contains the call.
+ * A lookup captured in this utility therefore only has full privilege for
+ * {@code HiddenClassDefiner} itself — never for arbitrary foreign classes
+ * (for example {@code java.lang.String} in {@code java.base}).
+ *
+ * <p>Consequently the <strong>preferred</strong> entry point is
+ * {@link #tryDefineNestmate(Lookup, byte[], boolean)}, where the caller
+ * supplies a {@link Lookup} obtained inside the intended nest-host class:
+ * <pre>{@code
+ * // Inside the class that should host the hidden nestmate:
+ * private static final MethodHandles.Lookup LOOKUP = MethodHandles.lookup();
+ *
+ * Class<?> hidden = HiddenClassDefiner.tryDefineNestmate(LOOKUP, bytecode,
false);
+ * if (hidden == null) {
+ * // fall back to ClassLoader.defineClass(...)
+ * }
+ * }</pre>
+ *
+ * <p>The overload {@link #tryDefineNestmate(Class, byte[], boolean)} is a
+ * <em>best-effort</em> helper for foreign hosts (user classes Groovy does not
+ * control). It uses {@link MethodHandles#privateLookupIn(Class, Lookup)} from
+ * this class and therefore succeeds only when the host's package is accessible
+ * to Groovy's module — typically true for unnamed-module application classes,
+ * and typically false for sealed / unopened packages such as those in
+ * {@code java.base}. Callers must always handle a {@code null} result.
+ *
+ * <h2>What this utility centralises</h2>
+ * <ul>
+ * <li>{@code NESTMATE} + weak-lifecycle policy for dynamic Groovy
classes;</li>
+ * <li>rewriting {@code this_class} into the lookup class's package (required
+ * by {@link Lookup#defineHiddenClass});</li>
+ * <li>a soft API that returns {@code null} on the <em>expected</em> failure
+ * modes ({@link IllegalAccessException}, {@link
IllegalArgumentException},
+ * {@link SecurityException}, {@link LinkageError}) so call sites fall
back
+ * to {@link ClassLoader#defineClass} with one null check. Unexpected
+ * failures (e.g. programming errors in callers) are not swallowed as a
+ * blanket {@link RuntimeException}.</li>
+ * </ul>
+ *
+ * <h2>Kill switch</h2>
+ * <p>{@code -Dgroovy.hidden.classes.disable=true} forces every {@code try*}
+ * method to return {@code null}.
+ *
+ * @since 6.0.0
+ * @see Lookup#defineHiddenClass(byte[], boolean, Lookup.ClassOption...)
+ */
+public final class HiddenClassDefiner {
+
+ /** System property that disables hidden-class definitions. */
+ public static final String PROPERTY_DISABLE =
"groovy.hidden.classes.disable";
+
+ /**
+ * {@code true} when hidden-class definitions are globally disabled.
+ * Evaluated once at class-init so hot paths pay no property-lookup cost.
+ */
+ public static final boolean HIDDEN_CLASSES_DISABLED =
+ SystemUtil.getBooleanSafe(PROPERTY_DISABLE, false);
+
+ /**
+ * Lookup for <em>this</em> class only — used exclusively as the caller
+ * argument to {@link MethodHandles#privateLookupIn(Class, Lookup)} in the
+ * foreign-host overload. It is never used as a nest host for user code.
+ */
+ private static final Lookup LOOKUP = MethodHandles.lookup();
+
+ /** Nestmate + weak (eager unloading) — the only option set production
uses. */
+ private static final Lookup.ClassOption[] NESTMATE_WEAK =
+ new Lookup.ClassOption[]{Lookup.ClassOption.NESTMATE};
+
+ private HiddenClassDefiner() {
+ }
+
+ /**
+ * @return {@code true} when hidden-class definition is enabled
+ * (the default unless {@value #PROPERTY_DISABLE} is set)
+ */
+ public static boolean isEnabled() {
+ return !HIDDEN_CLASSES_DISABLED;
+ }
+
+ /**
+ * Defines {@code bytes} as a hidden nestmate of {@code
lookup.lookupClass()}
+ * with a weak lifecycle.
+ *
+ * <p>The lookup must have been obtained via {@link MethodHandles#lookup()}
+ * inside the intended nest-host class (or otherwise carry full privilege
+ * for that class). The class-file package is rewritten to match the lookup
+ * class before definition.
+ *
+ * @param lookup full-privilege lookup for the nest host
+ * @param bytes class-file bytes
+ * @param initialize {@code true} to run {@code <clinit>} immediately
+ * @return the hidden class, or {@code null} if definition is not possible
+ */
+ public static Class<?> tryDefineNestmate(
+ final Lookup lookup,
+ final byte[] bytes,
+ final boolean initialize) {
+ if (HIDDEN_CLASSES_DISABLED || lookup == null || bytes == null) {
Review Comment:
My Point is that this code is always called from within the Groovy Runtime.
Which means MethodHandles.lookup() will always return the same. It will be for
the module the runtime is in and allow access to public methods in that module
as well as accessible methods in other classes. And that later case is the
potential problem, since if the runtime has no access to a different module the
ProxyAdapter will not have that too. You can do that MethodHandles.lookup()
anywhere in the Groovy runtime to get the same result. It also means the
private lookup is restricted. What does it mean for nest mates? Since we need
private access privileges for the target host class I have high doubts that
this works all as intended in a module system. Just try to create a nest mate
for String for example.
Looking at the usage I see ReflectorLoader. I found only a test, no actual
code. Which actually means this class should be deprecated/removed.
ProxyGeneratorAdapter is used by InlinedASTCustomizerFactory and
ProxyGenerator. InlinedASTCustomizerFactory is probably fine, but for
ProxyGenerator: what if the interface is not accessible by the runtime? And I
mean groovy can inspect the class, the methods are public? Map for example is
no problem. It is in a different and restricted module, but Map itself is
public API. The scenario I always keep in mind for such cases is that of 2
modules. Module A in Java, Module B in Groovy and Module C, the Groovy runtime.
And B uses A. B has to give C access, but why should A give C access? And then
all kinds of things may start failing. So I try to improve the code base
step-by-step in a way that can handle such cases for the time when Groovy
becomes a real module and has to handle Groovy programs in real modules.
Now you probably say something like that this is why we have the fallback.
And you are right about that. But assume we are in the modules case, how useful
is the nestmate variant then in the end? What would it cover outside of module
B and C? And is that enough for us? If you think it is good enough feel free to
resolve this conversation.
> Introduce hidden class support
> ------------------------------
>
> Key: GROOVY-12223
> URL: https://issues.apache.org/jira/browse/GROOVY-12223
> Project: Groovy
> Issue Type: Improvement
> Reporter: Daniel Sun
> Priority: Major
>
> h2. Background
> Groovy generates many short-lived synthetic classes at runtime, including:
> * map/interface proxies ({{ProxyGeneratorAdapter}})
> * reflection dispatch helpers ({{Reflector}} / {{ReflectorLoader}})
> * per-class meta-method artifacts ({{ClassLoaderForClassArtifacts}})
> Today these are defined with {{ClassLoader#defineClass}} as ordinary *named*
> classes. That has three practical downsides:
> # *Name pollution* — the synthetic types are discoverable via
> {{Class.forName}} / {{ClassLoader#loadClass}}.
> # *Metaspace pressure* — their lifetime is tied to the defining class loader;
> long-running applications that generate many artifacts retain them until the
> loader itself is collected.
> # *Access friction* — without nest membership, generated code cannot share
> private access with the host class the way a true nestmate can.
> JDK 15 introduced *hidden classes* ([JEP 371|https://openjdk.org/jeps/371]):
> classes defined through {{Lookup#defineHiddenClass}} that are
> non-discoverable by name, may join an access-control nest ({{NESTMATE}}), and
> may be unloaded independently of the defining loader when not marked
> {{STRONG}}.
> Groovy 6 requires JDK 17+, so the API is always present on supported runtimes.
> h2. Proposal
> Centralise hidden-class definition behind a single utility and prefer it for
> the dynamic class-generation sites listed above, with a transparent fallback
> to the existing {{ClassLoader#defineClass}} path.
> h3. New API
> {{org.apache.groovy.util.HiddenClassDefiner}} — the only call-site that
> invokes {{Lookup#defineHiddenClass}}:
> * {{defineHiddenClass(lookup, bytes, initialize, nestmate, strong)}} — full
> control
> * {{defineNestmateClass(lookup, bytes, initialize)}} — nestmate + weak
> lifecycle (default for proxies / reflectors / artifacts)
> * {{defineStrongHiddenClass(lookup, bytes, initialize)}} — non-discoverable,
> loader-tied lifetime
> * helpers: {{privateLookupIn(hostClass)}}, {{findConstructor(hiddenClass,
> ...parameterTypes)}}
> Kill-switch (evaluated once at class-init for hot-path cost):
> {noformat}
> -Dgroovy.hidden.classes.disable=true
> {noformat}
> When disabled (or when private lookup / definition fails), callers fall back
> to defining a normal visible class.
> h3. Integration points
> || Site || Nest host || Preferred options || Fallback ||
> | {{ClassLoaderForClassArtifacts#define}} | target (klazz) | nestmate, weak |
> {{ClassLoader#defineClass}} + protection domain |
> | {{ProxyGeneratorAdapter}} | non-{{Object}} superclass if present; else
> {{ProxyGeneratorAdapter}} | nestmate, weak | {{InnerLoader#defineClass}} |
> | {{ReflectorLoader#defineClass}} | {{Reflector}} | nestmate, weak |
> {{ClassLoader#defineClass}} + protection domain |
> Behaviour for callers of these generators is unchanged: proxies still
> implement the requested interfaces, reflectors still dispatch, artifacts
> still construct. The only observable differences when the hidden path
> succeeds are the synthetic name form (contains {{/}}) and {{Class#isHidden()
> == true}}.
> h2. Benefits
> * Non-discoverable synthetic types (cleaner class-space / tooling view).
> * Nestmate private access where the nest host can be opened for private
> lookup.
> * Eager unloading of weak hidden classes reduces long-run metaspace retention
> for short-lived proxies and artifacts.
> * One policy / upgrade point if future JDKs add further
> {{Lookup.ClassOption}} values.
> h2. Compatibility
> * Default-on when the JVM can obtain a full-privilege lookup for the chosen
> nest host; silent fallback otherwise (e.g. sealed / unopened module packages).
> * Opt-out: {{-Dgroovy.hidden.classes.disable=true}}.
> * No public language-surface change; no change to successful proxy /
> reflector / artifact *behaviour*, only to how the {{Class}} is defined.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
