[GR-57395] Add support for weak symbols to avoid leaks with class unloading.

PullRequest: graal/19535

Author: mukel

espresso/src/com.oracle.truffle.espresso.classfile/src/com/oracle/truffle/espresso/classfile/ClassfileParser.java

@@ -135,6 +135,7 @@ public final class ClassfileParser {
     private static final DebugTimer CODE_PARSE = DebugTimer.create("code parsing", PARSE_SINGLE_METHOD);
     private static final DebugTimer CODE_READ = DebugTimer.create("code read", CODE_PARSE);
     private static final DebugTimer EXCEPTION_HANDLERS = DebugTimer.create("exception handlers", CODE_PARSE);
+    private static final DebugTimer VALIDATION = DebugTimer.create("CP validation", KLASS_PARSE);
 
     private static final DebugCounter UTF8_ENTRY_COUNT = DebugCounter.create("UTF8 Constant Pool entries");
 
@@ -202,20 +203,20 @@ public final class ClassfileParser {
     private final boolean validate;
 
     private ClassfileParser(ParsingContext parsingContext, ClassfileStream stream, boolean verifiable, boolean loaderIsBootOrPlatform, Symbol<Type> requestedClassType, boolean isHidden,
-                    boolean forceAllowVMAnnotations) {
+                    boolean forceAllowVMAnnotations, boolean validate) {
         this.requestedClassType = requestedClassType;
         this.parsingContext = parsingContext;
         this.stream = Objects.requireNonNull(stream);
         this.verifiable = verifiable;
         this.loaderIsBootOrPlatform = loaderIsBootOrPlatform;
         this.isHidden = isHidden;
         this.forceAllowVMAnnotations = forceAllowVMAnnotations;
-        this.validate = true; // always validate
+        this.validate = validate;
     }
 
     // Note: only used for reading the class name from class bytes
     private ClassfileParser(ParsingContext parsingContext, ClassfileStream stream) {
-        this(parsingContext, stream, false, false, null, false, false);
+        this(parsingContext, stream, false, false, null, false, false, true);
     }
 
     void handleBadConstant(Tag tag, ClassfileStream s) {
@@ -256,8 +257,8 @@ void checkDynamicConstantSupport(Tag tag) {
     }
 
     public static ParserKlass parse(ParsingContext parsingContext, ClassfileStream stream, boolean verifiable, boolean loaderIsBootOrPlatform, Symbol<Type> requestedClassType, boolean isHidden,
-                    boolean forceAllowVMAnnotations) throws ValidationException {
-        return new ClassfileParser(parsingContext, stream, verifiable, loaderIsBootOrPlatform, requestedClassType, isHidden, forceAllowVMAnnotations).parseClass();
+                    boolean forceAllowVMAnnotations, boolean validate) throws ValidationException {
+        return new ClassfileParser(parsingContext, stream, verifiable, loaderIsBootOrPlatform, requestedClassType, isHidden, forceAllowVMAnnotations, validate).parseClass();
     }
 
     private ParserKlass parseClass() throws ValidationException {
@@ -530,8 +531,10 @@ private ImmutableConstantPool parseConstantPool() throws ValidationException {
 
         // Validation
         if (validate) {
-            for (int j = 1; j < constantPool.length(); ++j) {
-                entries[j].validate(constantPool);
+            try (DebugCloseable handlers = VALIDATION.scope(parsingContext.getTimers())) {
+                for (int j = 1; j < constantPool.length(); ++j) {
+                    entries[j].validate(constantPool);
+                }
             }
         }
 
@@ -661,16 +664,18 @@ private ParserMethod[] parseMethods(boolean isInterface) throws ValidationExcept
          * Classes in general have lots of methods: use a hash set rather than array lookup for dup
          * check.
          */
-        final HashSet<MethodKey> dup = new HashSet<>(methodCount);
+        final HashSet<MethodKey> dup = validate ? new HashSet<>(methodCount) : null;
         for (int i = 0; i < methodCount; ++i) {
             ParserMethod method;
             try (DebugCloseable closeable = PARSE_SINGLE_METHOD.scope(parsingContext.getTimers())) {
                 method = parseMethod(isInterface);
             }
             methods[i] = method;
-            try (DebugCloseable closeable = NO_DUP_CHECK.scope(parsingContext.getTimers())) {
-                if (!dup.add(new MethodKey(method))) {
-                    throw classFormatError("Duplicate method name and signature: " + method.getName() + " " + method.getSignature());
+            if (validate) {
+                try (DebugCloseable closeable = NO_DUP_CHECK.scope(parsingContext.getTimers())) {
+                    if (!dup.add(new MethodKey(method))) {
+                        throw classFormatError("Duplicate method name and signature: " + method.getName() + " " + method.getSignature());
+                    }
                 }
             }
         }
@@ -1695,7 +1700,7 @@ private CodeAttribute parseCodeAttribute(Symbol<Name> name) throws ValidationExc
             }
         }
 
-        if (totalLocalTableCount > 0) {
+        if (validate && totalLocalTableCount > 0) {
             validateLocalTables(codeAttributes);
         }
 

espresso/src/com.oracle.truffle.espresso.classfile/src/com/oracle/truffle/espresso/classfile/ParsingContext.java

@@ -45,10 +45,9 @@ public interface ParsingContext {
 
     Symbol<Name> getOrCreateName(ByteSequence byteSequence);
 
-    // symbolify(Types.nameToType(byteSequence))
     Symbol<Type> getOrCreateTypeFromName(ByteSequence byteSequence);
 
-    Utf8Constant getOrCreateUtf8Constant(ByteSequence bytes);
+    Utf8Constant getOrCreateUtf8Constant(ByteSequence byteSequence);
 
     interface Logger {
         void log(String message);

espresso/src/com.oracle.truffle.espresso.classfile/src/com/oracle/truffle/espresso/classfile/attributes/Local.java

@@ -60,16 +60,16 @@ public int hashCode(Object o) {
     private final Symbol<Name> name;
     private final Symbol<Type> type;
     private final Symbol<?> typeSignature;
-    private final int startBci;
-    private final int endBci;
-    private final int slot;
+    private final char startBci;
+    private final char endBci;
+    private final char slot;
 
     public Local(Symbol<Name> name, Symbol<Type> type, Symbol<?> typeSignature, int startBci, int endBci, int slot) {
         assert type != null || typeSignature != null;
         this.name = name;
-        this.startBci = startBci;
-        this.endBci = endBci;
-        this.slot = slot;
+        this.startBci = (char) startBci;
+        this.endBci = (char) endBci;
+        this.slot = (char) slot;
         this.type = type;
         this.typeSignature = typeSignature;
     }

espresso/src/com.oracle.truffle.espresso.classfile/src/com/oracle/truffle/espresso/classfile/descriptors/ByteSequence.java

@@ -24,6 +24,7 @@
 
 import java.io.IOException;
 import java.nio.ByteBuffer;
+import java.util.Arrays;
 import java.util.Objects;
 
 import com.oracle.truffle.api.CompilerDirectives.CompilationFinal;
@@ -137,11 +138,29 @@ public final int hashCode() {
         return hashCode;
     }
 
-    public final ByteSequence subSequence(int offset, int length) {
-        if (offset == 0 && length == length()) {
+    /**
+     * Returns a subsequence of this symbol from the specified index to the end.
+     *
+     * @param from The starting index (inclusive)
+     * @return A ByteSequence representing the subsequence
+     */
+    public ByteSequence subSequence(int from) {
+        return subSequence(from, length());
+    }
+
+    /**
+     * Returns a subsequence of this symbol between the specified indices.
+     *
+     * @param startInclusive The starting index (inclusive)
+     * @param endExclusive The ending index (exclusive)
+     * @return A ByteSequence representing the subsequence
+     */
+    public ByteSequence subSequence(int startInclusive, int endExclusive) {
+        assert 0 <= startInclusive && startInclusive <= endExclusive && endExclusive <= length();
+        if (startInclusive == 0 && endExclusive == length()) {
             return this;
         }
-        return wrap(getUnderlyingBytes(), offset() + offset, length);
+        return wrap(getUnderlyingBytes(), offset() + startInclusive, endExclusive - startInclusive);
     }
 
     public final boolean contentEquals(ByteSequence other) {
@@ -246,4 +265,17 @@ public ByteSequence concat(ByteSequence next) {
         next.writeTo(data, this.length());
         return wrap(data);
     }
+
+    @Override
+    public boolean equals(Object other) {
+        if (!(other instanceof ByteSequence that)) {
+            return false;
+        }
+        if (this.hashCode != that.hashCode) {
+            return false;
+        }
+        return Arrays.equals(
+                        this.value, this.offset(), this.offset() + this.length(),
+                        that.value, that.offset(), that.offset() + that.length());
+    }
 }

espresso/src/com.oracle.truffle.espresso.classfile/src/com/oracle/truffle/espresso/classfile/descriptors/NameSymbols.java

@@ -81,7 +81,20 @@ public Symbol<Name> getOrCreate(String nameString) {
      * @return A new or existing Symbol representing the name
      */
     public Symbol<Name> getOrCreate(ByteSequence nameBytes) {
-        return symbols.symbolify(nameBytes);
+        return getOrCreate(nameBytes, false);
+    }
+
+    /**
+     * Creates a new name symbol or retrieves an existing one from a byte sequence. If the symbol
+     * doesn't exist, it will be created.
+     *
+     * @param ensureStrongReference if {@code true}, the returned symbol is guaranteed to be
+     *            strongly referenced by the symbol table
+     * @param nameBytes The name as a byte sequence to create or retrieve
+     * @return A new or existing Symbol representing the name
+     */
+    public Symbol<Name> getOrCreate(ByteSequence nameBytes, boolean ensureStrongReference) {
+        return symbols.getOrCreate(nameBytes, ensureStrongReference);
     }
 
     /**

espresso/src/com.oracle.truffle.espresso.classfile/src/com/oracle/truffle/espresso/classfile/descriptors/ParserSymbols.java

@@ -28,8 +28,8 @@
  * Predefined symbols used by the parser and shared with Espresso. Contains static definitions for
  * commonly used types, names and signatures.
  * <p>
- * This class is not meant for general symbol management - use {@link Symbols}, {@link TypeSymbols},
- * {@link NameSymbols} etc. for that purpose.
+ * This class is not meant for general symbol management - use {@link SignatureSymbols},
+ * {@link TypeSymbols}, {@link NameSymbols} etc. for that purpose.
  * <p>
  * The symbols are initialized in a specific order via the inner classes:
  * <ul>
@@ -42,7 +42,7 @@
  */
 public final class ParserSymbols {
 
-    public static final StaticSymbols SYMBOLS = new StaticSymbols();
+    public static final StaticSymbols SYMBOLS = new StaticSymbols(1 << 8);
 
     static {
         ParserNames.ensureInitialized();

espresso/src/com.oracle.truffle.espresso.classfile/src/com/oracle/truffle/espresso/classfile/descriptors/SignatureSymbols.java

@@ -28,7 +28,6 @@
 import java.util.Arrays;
 import java.util.Iterator;
 import java.util.List;
-import java.util.concurrent.ConcurrentHashMap;
 
 import com.oracle.truffle.api.nodes.ExplodeLoop;
 import com.oracle.truffle.espresso.classfile.JavaKind;
@@ -54,8 +53,6 @@ public final class SignatureSymbols {
     private final TypeSymbols typeSymbols;
     private final Symbols symbols;
 
-    private final ConcurrentHashMap<Symbol<Signature>, Symbol<Type>[]> parsedSignatures = new ConcurrentHashMap<>();
-
     /**
      * Creates a new SignatureSymbols instance.
      *
@@ -103,10 +100,24 @@ public Symbol<Signature> lookupValidSignature(ByteSequence signatureBytes) {
      * @see Validation#validSignatureDescriptor(ByteSequence)
      */
     public Symbol<Signature> getOrCreateValidSignature(ByteSequence signatureBytes) {
+        return getOrCreateValidSignature(signatureBytes, false);
+    }
+
+    /**
+     * Creates or retrieves a valid method signature symbol from a byte sequence.
+     *
+     * @param ensureStrongReference if {@code true}, the returned symbol is guaranteed to be
+     *            strongly referenced by the symbol table
+     * @param signatureBytes The method signature bytes to create or retrieve
+     * @return The signature Symbol if valid, null otherwise
+     *
+     * @see Validation#validSignatureDescriptor(ByteSequence)
+     */
+    public Symbol<Signature> getOrCreateValidSignature(ByteSequence signatureBytes, boolean ensureStrongReference) {
         if (!Validation.validSignatureDescriptor(signatureBytes)) {
             return null;
         }
-        return symbols.symbolify(signatureBytes);
+        return symbols.getOrCreate(signatureBytes, ensureStrongReference);
     }
 
     /**
@@ -121,13 +132,13 @@ public TypeSymbols getTypes() {
     /**
      * Gets or computes the parsed form of a method signature. The parsed form is an array of Type
      * symbols where the last element is the return type and all preceding elements are parameter
-     * types. Results are cached for performance.
+     * types.
      *
      * @param signature The method signature to parse
      * @return Array of Type symbols representing parameter types followed by return type
      */
     public Symbol<Type>[] parsed(Symbol<Signature> signature) {
-        return parsedSignatures.computeIfAbsent(signature, key -> parse(SignatureSymbols.this.getTypes(), signature, 0));
+        return parse(SignatureSymbols.this.getTypes(), signature, 0);
     }
 
     /**
@@ -340,7 +351,7 @@ public static Symbol<Type>[] makeParsedUncached(Symbol<Type> returnType, Symbol<
 
     @SafeVarargs
     public final Symbol<Signature> makeRaw(Symbol<Type> returnType, Symbol<Type>... parameterTypes) {
-        return symbols.symbolify(createSignature(returnType, parameterTypes));
+        return symbols.getOrCreate(createSignature(returnType, parameterTypes));
     }
 
     @SafeVarargs

espresso/src/com.oracle.truffle.espresso.classfile/src/com/oracle/truffle/espresso/classfile/descriptors/StaticSymbols.java

@@ -22,6 +22,11 @@
  */
 package com.oracle.truffle.espresso.classfile.descriptors;
 
+import java.util.HashSet;
+import java.util.Set;
+
+import org.graalvm.collections.EconomicMap;
+
 /**
  * To be populated in static initializers, always before the first runtime symbol table is spawned.
  *
@@ -32,56 +37,56 @@
 public final class StaticSymbols {
 
     private boolean frozen = false;
-    private final Symbols symbols;
+    private final EconomicMap<ByteSequence, Symbol<?>> symbols;
 
-    public StaticSymbols(StaticSymbols seed) {
-        this.symbols = new Symbols(seed.freeze());
+    public StaticSymbols(StaticSymbols seed, int initialCapacity) {
+        this.symbols = EconomicMap.create(initialCapacity);
+        this.symbols.putAll(seed.symbols);
     }
 
-    public StaticSymbols() {
-        this.symbols = new Symbols();
+    public StaticSymbols(int initialCapacity) {
+        this.symbols = EconomicMap.create(initialCapacity);
     }
 
     public Symbol<Name> putName(String nameString) {
         ErrorUtil.guarantee(!nameString.isEmpty(), "empty name");
         ByteSequence byteSequence = ByteSequence.create(nameString);
-        Symbol<Name> name = symbols.lookup(byteSequence);
-        if (name != null) {
-            return name;
-        }
-        ErrorUtil.guarantee(!isFrozen(), "static symbols are frozen");
-        return symbols.symbolify(byteSequence);
+        return getOrCreateSymbol(byteSequence);
     }
 
     public Symbol<Type> putType(String internalName) {
         ByteSequence byteSequence = ByteSequence.create(internalName);
         ErrorUtil.guarantee(Validation.validTypeDescriptor(byteSequence, true), "invalid type");
-        Symbol<Type> type = symbols.lookup(byteSequence);
-        if (type != null) {
-            return type;
-        }
-        ErrorUtil.guarantee(!isFrozen(), "static symbols are frozen");
-        return symbols.symbolify(byteSequence);
+        return getOrCreateSymbol(byteSequence);
     }
 
     @SafeVarargs
     public final Symbol<Signature> putSignature(Symbol<Type> returnType, Symbol<Type>... parameterTypes) {
         ByteSequence signatureBytes = SignatureSymbols.createSignature(returnType, parameterTypes);
         ErrorUtil.guarantee(Validation.validSignatureDescriptor(signatureBytes, false), "invalid signature");
-        Symbol<Signature> signature = symbols.lookup(signatureBytes);
-        if (signature != null) {
-            return signature;
-        }
-        ErrorUtil.guarantee(!isFrozen(), "static symbols are frozen");
-        return symbols.symbolify(signatureBytes);
+        return getOrCreateSymbol(signatureBytes);
     }
 
     public boolean isFrozen() {
         return frozen;
     }
 
-    public Symbols freeze() {
+    public Set<Symbol<?>> freeze() {
         frozen = true;
-        return symbols;
+        Set<Symbol<?>> result = new HashSet<>(symbols.size());
+        symbols.getValues().forEach(result::add);
+        return result;
+    }
+
+    @SuppressWarnings("unchecked")
+    private <T> Symbol<T> getOrCreateSymbol(ByteSequence byteSequence) {
+        Symbol<T> symbol = (Symbol<T>) symbols.get(byteSequence);
+        if (symbol != null) {
+            return symbol;
+        }
+        ErrorUtil.guarantee(!isFrozen(), "static symbols are frozen");
+        symbol = Symbols.createSymbolInstanceUnsafe(byteSequence);
+        symbols.put(symbol, symbol);
+        return symbol;
     }
 }