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-57395] Add support for weak symbols to avoid leaks with class unloading. #10627

Merged
merged 1 commit into from
Feb 6, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Add support for weak symbols in Espresso.
  • Loading branch information
mukel committed Feb 5, 2025
commit 2c8bd6da4a394c77eda1e6c790c788d27ce01280
Original file line number Diff line number Diff line change
Expand Up @@ -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");

Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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);
}
}
}

Expand Down Expand Up @@ -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());
}
}
}
}
Expand Down Expand Up @@ -1695,7 +1700,7 @@ private CodeAttribute parseCodeAttribute(Symbol<Name> name) throws ValidationExc
}
}

if (totalLocalTableCount > 0) {
if (validate && totalLocalTableCount > 0) {
validateLocalTables(codeAttributes);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand All @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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.
*
Expand Down Expand Up @@ -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);
}

/**
Expand All @@ -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);
}

/**
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand All @@ -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;
}
}
Loading