Skip to content

Commit

Permalink
[GR-40322] The hashCode() and equals() methods of InsightFilter are c…
Browse files Browse the repository at this point in the history 
…orrected and context management is fixed.

PullRequest: graal/12442
  • Loading branch information
@entlicher
entlicher committed Sep 3, 2022
2 parents 78720af + 9d15349 commit 6ef9cc1
Show file tree
Hide file tree
Showing 4 changed files with 85 additions and 33 deletions.
26 changes: 16 additions & 10 deletions ...ruffle.tools.agentscript/src/com/oracle/truffle/tools/agentscript/impl/InsightFilter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@
import com.oracle.truffle.api.interop.StopIterationException;
import com.oracle.truffle.api.interop.UnknownIdentifierException;
import com.oracle.truffle.api.interop.UnsupportedMessageException;
import java.lang.ref.Reference;
import java.lang.ref.WeakReference;
import java.net.URI;
import java.net.URISyntaxException;

Expand Down Expand Up @@ -66,18 +68,22 @@ final class InsightFilter {
private final URI sourceURI;
private final int line;
private final int column;
private final boolean rootNameFn;
private final boolean sourceFilterFn;
private final Reference<Object> rootNameFn;
private final int rootNameFnHash;
private final Reference<Object> sourceFilterFn;
private final int sourceFilterFnHash;

private InsightFilter(Set<Class<? extends Tag>> allTags, String rootNameRegExp, URI sourceURI, String sourcePathRegExp, int line, int column, boolean rootNameFn, boolean sourceFilterFn) {
private InsightFilter(Set<Class<? extends Tag>> allTags, String rootNameRegExp, URI sourceURI, String sourcePathRegExp, int line, int column, Object rootNameFn, Object sourceFilterFn) {
this.allTags = allTags;
this.rootNameRegExp = rootNameRegExp;
this.sourceURI = sourceURI;
this.sourcePathRegExp = sourcePathRegExp;
this.line = line;
this.column = column;
this.rootNameFn = rootNameFn;
this.sourceFilterFn = sourceFilterFn;
this.rootNameFn = new WeakReference<>(rootNameFn);
this.rootNameFnHash = (rootNameFn != null) ? rootNameFn.hashCode() : 0;
this.sourceFilterFn = new WeakReference<>(sourceFilterFn);
this.sourceFilterFnHash = (sourceFilterFn != null) ? sourceFilterFn.hashCode() : 0;
}

@Override
Expand All @@ -89,8 +95,8 @@ public int hashCode() {
hash = 79 * hash + Objects.hashCode(this.sourceURI);
hash = 79 * hash + line;
hash = 79 * hash + column;
hash = 79 * hash + (this.rootNameFn ? 1 : 0);
hash = 79 * hash + (this.sourceFilterFn ? 1 : 0);
hash = 79 * hash + rootNameFnHash;
hash = 79 * hash + sourceFilterFnHash;
return hash;
}

Expand All @@ -106,10 +112,10 @@ public boolean equals(Object obj) {
return false;
}
final InsightFilter other = (InsightFilter) obj;
if (this.rootNameFn != other.rootNameFn) {
if (this.rootNameFn.get() != other.rootNameFn.get()) {
return false;
}
if (this.sourceFilterFn != other.sourceFilterFn) {
if (this.sourceFilterFn.get() != other.sourceFilterFn.get()) {
return false;
}
if (!Objects.equals(this.rootNameRegExp, other.rootNameRegExp)) {
Expand Down Expand Up @@ -242,7 +248,7 @@ static InsightFilter.Data create(AgentType aType, Object[] arr)
throw new IllegalArgumentException("No elements specified to listen to for execution listener. Need to specify at least one element kind: expressions, statements or roots.");
}

InsightFilter filter = new InsightFilter(allTags, rootNameRegExp, sourceURI, sourcePathRegExp, lineIs, columnIs, rootNameFn != null, sourceFilterFn != null);
InsightFilter filter = new InsightFilter(allTags, rootNameRegExp, sourceURI, sourcePathRegExp, lineIs, columnIs, rootNameFn, sourceFilterFn);
return new Data(aType, filter, arr[1], rootNameFn, sourceFilterFn);
}

Expand Down
6 changes: 3 additions & 3 deletions ...le.tools.agentscript/src/com/oracle/truffle/tools/agentscript/impl/InsightInstrument.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2019, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2019, 2022, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand Down Expand Up @@ -73,7 +73,7 @@ public class InsightInstrument extends TruffleInstrument {
static final OptionKey<String> SCRIPT = new OptionKey<>("");

final IgnoreSources ignoreSources = new IgnoreSources();
final ContextLocal<InsightPerContext> perContextData;
private final ContextLocal<InsightPerContext> perContextData;
private Env env;
/** @GuardedBy("keys" */
private final BitSet keys = new BitSet();
Expand All @@ -82,7 +82,7 @@ public class InsightInstrument extends TruffleInstrument {

public InsightInstrument() {
this.perContextData = createContextLocal((context) -> {
return new InsightPerContext(this, context);
return new InsightPerContext(this);
});
this.keysUnchanged = Truffle.getRuntime().createAssumption();
}
Expand Down
15 changes: 6 additions & 9 deletions ...le.tools.agentscript/src/com/oracle/truffle/tools/agentscript/impl/InsightPerContext.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@

import com.oracle.truffle.api.Assumption;
import com.oracle.truffle.api.CompilerDirectives;
import com.oracle.truffle.api.TruffleContext;
import com.oracle.truffle.api.interop.InteropException;
import com.oracle.truffle.api.interop.InteropLibrary;
import com.oracle.truffle.api.source.Source;
Expand All @@ -41,19 +40,15 @@

final class InsightPerContext {
final InsightInstrument insight;
final TruffleContext context;
private final Map<InsightInstrument.Key, List<Object>> functionsForBinding = new HashMap<>();
@CompilerDirectives.CompilationFinal //
private int functionsLen;
@CompilerDirectives.CompilationFinal(dimensions = 2) //
private Object[][] functionsArray;
@CompilerDirectives.CompilationFinal //
private Assumption functionsArrayValid;
private final Map<InsightSourceFilter, Map<Source, Boolean>> sourceCache = new HashMap<>(2);

InsightPerContext(InsightInstrument insight, TruffleContext context) {
InsightPerContext(InsightInstrument insight) {
this.insight = insight;
this.context = context;
}

synchronized void register(InsightInstrument.Key key, Object function) {
Expand Down Expand Up @@ -94,10 +89,12 @@ Object functionFor(InsightInstrument.Key key, int at) {
final int index = key.index();
Object[] functions;
if (index >= 0) {
if (functionsArray == null || !functionsArrayValid.isValid()) {
updateFunctionsArraySlow();
synchronized (this) {
if (functionsArray == null || !functionsArrayValid.isValid()) {
updateFunctionsArraySlow();
}
functions = functionsArray[index];
}
functions = functionsArray[index];
} else {
functions = null;
}
Expand Down
71 changes: 60 additions & 11 deletions ...fle.tools.agentscript/src/com/oracle/truffle/tools/agentscript/impl/InsightPerSource.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
*/
package com.oracle.truffle.tools.agentscript.impl;

import java.lang.reflect.Array;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
Expand Down Expand Up @@ -57,12 +58,17 @@
import com.oracle.truffle.api.source.Source;
import com.oracle.truffle.api.source.SourceSection;

import org.graalvm.collections.EconomicMap;

final class InsightPerSource implements ContextsListener, AutoCloseable, LoadSourceListener {
private final InsightInstrument instrument;
private final Supplier<Source> src;
private final AgentObject insight;
private final IgnoreSources ignoredSources;
private volatile EventBinding<?> agentBinding;
// Agent initialization bindings for individual contexts.
// The context key is removed on binding dispose, or context close.
/* @GuardedBy("this") */
private final EconomicMap<TruffleContext, EventBinding<?>> initializeBindings = EconomicMap.create();
/* @GuardedBy("this") */
private InsightInstrument.Key sourceBinding;
/* @GuardedBy("this") */
Expand Down Expand Up @@ -122,15 +128,32 @@ public void onLanguageContextCreated(TruffleContext context, LanguageInfo langua

@Override
public void onLanguageContextInitialized(TruffleContext context, LanguageInfo language) {
if (agentBinding != null || language.isInternal()) {
if (language.isInternal()) {
return;
}
if (context.isEntered()) {
EventBinding<?> agentBinding;
synchronized (this) {
agentBinding = initializeBindings.removeKey(context);
}
if (agentBinding != null) {
agentBinding.dispose();
}
initializeAgent(context);
} else {
EventBinding<?> agentBinding;
synchronized (this) {
if (initializeBindings.containsKey(context) || registeredSource.containsKey(context)) {
// Either the binding exists already, or the source is registered already.
return;
}
}
final SourceSectionFilter anyRoot = SourceSectionFilter.newBuilder().tagIs(StandardTags.RootTag.class).build();
Instrumenter instrumenter = instrument.env().getInstrumenter();
agentBinding = instrumenter.attachExecutionEventListener(anyRoot, new InitializeLater(context));
synchronized (this) {
initializeBindings.put(context, agentBinding);
}
}
}

Expand All @@ -151,6 +174,13 @@ public void onLanguageContextDisposed(TruffleContext context, LanguageInfo langu

@Override
public void onContextClosed(TruffleContext context) {
EventBinding<?> binding;
synchronized (this) {
binding = initializeBindings.removeKey(context);
}
if (binding != null) {
binding.dispose();
}
}

@Override
Expand All @@ -164,11 +194,25 @@ public void close() {
bindings = null;
}
onInit.dispose();
EventBinding<?> agentInitBinding = agentBinding;
if (agentInitBinding != null) {
agentInitBinding.dispose();
EventBinding<?>[] binds; // copy of initializeBindings values, not to dispose under lock
synchronized (this) {
if (!initializeBindings.isEmpty()) {
int n = initializeBindings.size();
binds = (EventBinding<?>[]) Array.newInstance(EventBinding.class, n);
int i = 0;
for (EventBinding<?> agentInitBinding : initializeBindings.getValues()) {
binds[i++] = agentInitBinding;
}
initializeBindings.clear();
} else {
binds = null;
}
}
if (binds != null) {
for (EventBinding<?> agentInitBinding : binds) {
agentInitBinding.dispose();
}
}
agentBinding = null;
instrument.closeKeys(keys);
}

Expand Down Expand Up @@ -340,12 +384,17 @@ final class InitializeLater implements ExecutionEventListener {

@Override
public void onEnter(EventContext ctx, VirtualFrame frame) {
CompilerDirectives.transferToInterpreterAndInvalidate();
EventBinding<?> agentInitBinding = agentBinding;
if (agentInitBinding != null) {
agentInitBinding.dispose();
if (instrument.env().getEnteredContext() == context) {
CompilerDirectives.transferToInterpreterAndInvalidate();
EventBinding<?> agentInitBinding;
synchronized (InsightPerSource.this) {
agentInitBinding = initializeBindings.removeKey(context);
}
if (agentInitBinding != null) {
agentInitBinding.dispose();
initializeAgent(context);
}
}
initializeAgent(context);
}

@Override
Expand Down

0 comments on commit 6ef9cc1

Please sign in to comment.