Skip to content

Commit 4e63029

Browse files
author
Christian Wimmer
committed
[GR-39852] Ensure that type reachability handlers finish before creating fields of that type.
PullRequest: graal/12453
2 parents 16c4f18 + 725847c commit 4e63029

File tree

14 files changed

+198
-92
lines changed

14 files changed

+198
-92
lines changed

‎substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/AbstractAnalysisEngine.java‎

Lines changed: 27 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,22 @@
2424
*/
2525
package com.oracle.graal.pointsto;
2626

27+
import java.io.PrintWriter;
28+
import java.util.Collections;
29+
import java.util.List;
30+
import java.util.concurrent.ForkJoinPool;
31+
import java.util.function.Function;
32+
33+
import org.graalvm.compiler.api.replacements.SnippetReflectionProvider;
34+
import org.graalvm.compiler.debug.DebugContext;
35+
import org.graalvm.compiler.debug.DebugContext.Builder;
36+
import org.graalvm.compiler.debug.DebugHandlersFactory;
37+
import org.graalvm.compiler.debug.Indent;
38+
import org.graalvm.compiler.nodes.spi.Replacements;
39+
import org.graalvm.compiler.options.OptionValues;
40+
import org.graalvm.compiler.printer.GraalDebugHandlersFactory;
41+
import org.graalvm.nativeimage.hosted.Feature;
42+
2743
import com.oracle.graal.pointsto.api.HostVM;
2844
import com.oracle.graal.pointsto.api.PointstoOptions;
2945
import com.oracle.graal.pointsto.constraints.UnsupportedFeatures;
@@ -38,22 +54,8 @@
3854
import com.oracle.graal.pointsto.util.CompletionExecutor;
3955
import com.oracle.graal.pointsto.util.Timer;
4056
import com.oracle.graal.pointsto.util.TimerCollection;
41-
import jdk.vm.ci.meta.ConstantReflectionProvider;
42-
import org.graalvm.compiler.api.replacements.SnippetReflectionProvider;
43-
import org.graalvm.compiler.debug.DebugContext;
44-
import org.graalvm.compiler.debug.DebugContext.Builder;
45-
import org.graalvm.compiler.debug.DebugHandlersFactory;
46-
import org.graalvm.compiler.debug.Indent;
47-
import org.graalvm.compiler.nodes.spi.Replacements;
48-
import org.graalvm.compiler.options.OptionValues;
49-
import org.graalvm.compiler.printer.GraalDebugHandlersFactory;
50-
import org.graalvm.nativeimage.hosted.Feature;
5157

52-
import java.io.PrintWriter;
53-
import java.util.Collections;
54-
import java.util.List;
55-
import java.util.concurrent.ForkJoinPool;
56-
import java.util.function.Function;
58+
import jdk.vm.ci.meta.ConstantReflectionProvider;
5759

5860
/**
5961
* This abstract class is shared between Reachability and Points-to. It contains generic methods
@@ -291,6 +293,16 @@ protected void schedule(Runnable task) {
291293
executor.execute((d) -> task.run());
292294
}
293295

296+
@Override
297+
public final void postTask(CompletionExecutor.DebugContextRunnable task) {
298+
executor.execute(task);
299+
}
300+
301+
@Override
302+
public final boolean executorIsStarted() {
303+
return executor.isStarted();
304+
}
305+
294306
@Override
295307
public Replacements getReplacements() {
296308
return replacements;

‎substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/BigBang.java‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,10 +28,10 @@
2828
import java.util.List;
2929
import java.util.function.Function;
3030

31-
import com.oracle.graal.pointsto.util.CompletionExecutor;
3231
import org.graalvm.compiler.api.replacements.SnippetReflectionProvider;
3332
import org.graalvm.compiler.debug.DebugContext;
3433
import org.graalvm.compiler.debug.DebugHandlersFactory;
34+
import org.graalvm.compiler.nodes.spi.Replacements;
3535
import org.graalvm.compiler.options.OptionValues;
3636

3737
import com.oracle.graal.pointsto.api.HostVM;
@@ -42,10 +42,10 @@
4242
import com.oracle.graal.pointsto.meta.AnalysisType.UsageKind;
4343
import com.oracle.graal.pointsto.meta.AnalysisUniverse;
4444
import com.oracle.graal.pointsto.meta.HostedProviders;
45+
import com.oracle.graal.pointsto.util.CompletionExecutor;
4546

4647
import jdk.vm.ci.code.BytecodePosition;
4748
import jdk.vm.ci.meta.ConstantReflectionProvider;
48-
import org.graalvm.compiler.nodes.spi.Replacements;
4949

5050
/**
5151
* Central static analysis interface that groups together the functionality of reachability analysis
@@ -118,6 +118,8 @@ default void onTypeInitialized(AnalysisType type) {
118118

119119
void postTask(CompletionExecutor.DebugContextRunnable task);
120120

121+
boolean executorIsStarted();
122+
121123
void initializeMetaData(AnalysisType type);
122124

123125
/**

‎substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/PointsToAnalysis.java‎

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -291,10 +291,6 @@ public Iterable<AnalysisType> getAllSynchronizedTypes() {
291291
return allSynchronizedTypeFlow.getState().types(this);
292292
}
293293

294-
public boolean executorIsStarted() {
295-
return executor.isStarted();
296-
}
297-
298294
@Override
299295
public AnalysisMethod addRootMethod(Executable method, boolean invokeSpecial) {
300296
return addRootMethod(metaAccess.lookupJavaMethod(method), invokeSpecial);
@@ -504,11 +500,6 @@ public TypeFlow<?> getTypeFlow() {
504500
});
505501
}
506502

507-
@Override
508-
public void postTask(final DebugContextRunnable task) {
509-
executor.execute(task);
510-
}
511-
512503
public void postTask(final Runnable task) {
513504
executor.execute(new DebugContextRunnable() {
514505
@Override

‎substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/heap/ImageHeapScanner.java‎

Lines changed: 64 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737
import org.graalvm.compiler.debug.GraalError;
3838
import org.graalvm.word.WordBase;
3939

40+
import com.oracle.graal.pointsto.BigBang;
4041
import com.oracle.graal.pointsto.ObjectScanner;
4142
import com.oracle.graal.pointsto.ObjectScanner.ArrayScan;
4243
import com.oracle.graal.pointsto.ObjectScanner.EmbeddedRootScan;
@@ -53,6 +54,7 @@
5354
import com.oracle.graal.pointsto.meta.AnalysisUniverse;
5455
import com.oracle.graal.pointsto.util.AnalysisError;
5556
import com.oracle.graal.pointsto.util.AnalysisFuture;
57+
import com.oracle.graal.pointsto.util.CompletionExecutor;
5658
import com.oracle.graal.pointsto.util.GraalAccess;
5759
import com.oracle.svm.util.ReflectionUtil;
5860

@@ -75,6 +77,7 @@ public abstract class ImageHeapScanner {
7577

7678
private static final JavaConstant[] emptyConstantArray = new JavaConstant[0];
7779

80+
protected final BigBang bb;
7881
protected final ImageHeap imageHeap;
7982
protected final AnalysisMetaAccess metaAccess;
8083
protected final AnalysisUniverse universe;
@@ -90,8 +93,9 @@ public abstract class ImageHeapScanner {
9093
/** Marker object installed when encountering scanning issues like illegal objects. */
9194
private static final ImageHeapObject NULL_IMAGE_HEAP_OBJECT = new ImageHeapInstance(JavaConstant.NULL_POINTER, 0);
9295

93-
public ImageHeapScanner(ImageHeap heap, AnalysisMetaAccess aMetaAccess, SnippetReflectionProvider aSnippetReflection,
96+
public ImageHeapScanner(BigBang bb, ImageHeap heap, AnalysisMetaAccess aMetaAccess, SnippetReflectionProvider aSnippetReflection,
9497
ConstantReflectionProvider aConstantReflection, ObjectScanningObserver aScanningObserver) {
98+
this.bb = bb;
9599
imageHeap = heap;
96100
metaAccess = aMetaAccess;
97101
universe = aMetaAccess.getUniverse();
@@ -467,51 +471,72 @@ protected boolean skipScanning() {
467471
return false;
468472
}
469473

470-
public Object rescanRoot(Field reflectionField) {
474+
/**
475+
* When a re-scanning is triggered while the analysis is running in parallel, it is necessary to
476+
* do the re-scanning in a separate executor task to avoid deadlocks. For example,
477+
* lookupJavaField might need to wait for the reachability handler to be finished that actually
478+
* triggered the re-scanning.
479+
*
480+
* In the (legacy) Feature.duringAnalysis state, the executor is not running and we must not
481+
* schedule new tasks, because that would be treated as "the analysis has not finsihed yet". So
482+
* in that case we execute the task directly.
483+
*/
484+
private void maybeRunInExecutor(CompletionExecutor.DebugContextRunnable task) {
485+
if (bb.executorIsStarted()) {
486+
bb.postTask(task);
487+
} else {
488+
task.run(null);
489+
}
490+
}
491+
492+
public void rescanRoot(Field reflectionField) {
471493
if (skipScanning()) {
472-
return null;
494+
return;
473495
}
474-
AnalysisType type = metaAccess.lookupJavaType(reflectionField.getDeclaringClass());
475-
if (type.isReachable()) {
476-
AnalysisField field = metaAccess.lookupJavaField(reflectionField);
477-
JavaConstant fieldValue = readHostedFieldValue(field, null).get();
478-
TypeData typeData = field.getDeclaringClass().getOrComputeData();
479-
AnalysisFuture<JavaConstant> fieldTask = patchStaticField(typeData, field, fieldValue, OtherReason.RESCAN, null);
480-
if (field.isRead() || field.isFolded()) {
481-
Object root = asObject(fieldTask.ensureDone());
482-
rescanCollectionElements(root);
483-
return root;
496+
497+
maybeRunInExecutor(unused -> {
498+
AnalysisType type = metaAccess.lookupJavaType(reflectionField.getDeclaringClass());
499+
if (type.isReachable()) {
500+
AnalysisField field = metaAccess.lookupJavaField(reflectionField);
501+
JavaConstant fieldValue = readHostedFieldValue(field, null).get();
502+
TypeData typeData = field.getDeclaringClass().getOrComputeData();
503+
AnalysisFuture<JavaConstant> fieldTask = patchStaticField(typeData, field, fieldValue, OtherReason.RESCAN, null);
504+
if (field.isRead() || field.isFolded()) {
505+
Object root = asObject(fieldTask.ensureDone());
506+
rescanCollectionElements(root);
507+
}
484508
}
485-
}
486-
return null;
509+
});
487510
}
488511

489512
public void rescanField(Object receiver, Field reflectionField) {
490513
if (skipScanning()) {
491514
return;
492515
}
493-
AnalysisType type = metaAccess.lookupJavaType(reflectionField.getDeclaringClass());
494-
if (type.isReachable()) {
495-
AnalysisField field = metaAccess.lookupJavaField(reflectionField);
496-
assert !field.isStatic();
497-
JavaConstant receiverConstant = asConstant(receiver);
498-
Optional<JavaConstant> replaced = maybeReplace(receiverConstant, OtherReason.RESCAN);
499-
if (replaced.isPresent()) {
500-
if (replaced.get().isNull()) {
501-
/* There was some problem during replacement, bailout. */
502-
return;
516+
maybeRunInExecutor(unused -> {
517+
AnalysisType type = metaAccess.lookupJavaType(reflectionField.getDeclaringClass());
518+
if (type.isReachable()) {
519+
AnalysisField field = metaAccess.lookupJavaField(reflectionField);
520+
assert !field.isStatic();
521+
JavaConstant receiverConstant = asConstant(receiver);
522+
Optional<JavaConstant> replaced = maybeReplace(receiverConstant, OtherReason.RESCAN);
523+
if (replaced.isPresent()) {
524+
if (replaced.get().isNull()) {
525+
/* There was some problem during replacement, bailout. */
526+
return;
527+
}
528+
receiverConstant = replaced.get();
503529
}
504-
receiverConstant = replaced.get();
505-
}
506-
JavaConstant fieldValue = readHostedFieldValue(field, universe.toHosted(receiverConstant)).get();
507-
if (fieldValue != null) {
508-
ImageHeapInstance receiverObject = (ImageHeapInstance) toImageHeapObject(receiverConstant);
509-
AnalysisFuture<JavaConstant> fieldTask = patchInstanceField(receiverObject, field, fieldValue, OtherReason.RESCAN, null);
510-
if (field.isRead() || field.isFolded()) {
511-
rescanCollectionElements(asObject(fieldTask.ensureDone()));
530+
JavaConstant fieldValue = readHostedFieldValue(field, universe.toHosted(receiverConstant)).get();
531+
if (fieldValue != null) {
532+
ImageHeapInstance receiverObject = (ImageHeapInstance) toImageHeapObject(receiverConstant);
533+
AnalysisFuture<JavaConstant> fieldTask = patchInstanceField(receiverObject, field, fieldValue, OtherReason.RESCAN, null);
534+
if (field.isRead() || field.isFolded()) {
535+
rescanCollectionElements(asObject(fieldTask.ensureDone()));
536+
}
512537
}
513538
}
514-
}
539+
});
515540
}
516541

517542
protected AnalysisFuture<JavaConstant> patchStaticField(TypeData typeData, AnalysisField field, JavaConstant fieldValue, ScanReason reason, Consumer<ScanReason> onAnalysisModified) {
@@ -540,7 +565,6 @@ protected AnalysisFuture<JavaConstant> patchInstanceField(ImageHeapInstance rece
540565
*/
541566
public void rescanObject(Object object) {
542567
rescanObject(object, OtherReason.RESCAN);
543-
rescanCollectionElements(object);
544568
}
545569

546570
/**
@@ -553,7 +577,11 @@ public void rescanObject(Object object, ScanReason reason) {
553577
if (object == null) {
554578
return;
555579
}
556-
doScan(asConstant(object), reason);
580+
581+
maybeRunInExecutor(unused -> {
582+
doScan(asConstant(object), reason);
583+
rescanCollectionElements(object);
584+
});
557585
}
558586

559587
private void rescanCollectionElements(Object object) {

0 commit comments

Comments
 (0)