Skip to content

Commit e861aa7

Browse files
fangererfangerer
committed
Fix: do not traverse infinitely and only decref temp varargs tuple.
1 parent be9ab6d commit e861aa7

File tree

1 file changed

+60
-85
lines changed

1 file changed

+60
-85
lines changed

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/modules/ExternalFunctionNodes.java‎

Lines changed: 60 additions & 85 deletions
Original file line numberDiff line numberDiff line change
@@ -45,30 +45,27 @@
4545
import com.oracle.graal.python.builtins.modules.ExternalFunctionNodesFactory.CreateArgsTupleNodeGen;
4646
import com.oracle.graal.python.builtins.modules.ExternalFunctionNodesFactory.MaterializePrimitiveNodeGen;
4747
import com.oracle.graal.python.builtins.modules.ExternalFunctionNodesFactory.ReleaseNativeWrapperNodeGen;
48-
import com.oracle.graal.python.builtins.modules.ExternalFunctionNodesFactory.TraverseNativeWrapperNodeGen;
4948
import com.oracle.graal.python.builtins.modules.PythonCextBuiltins.CheckFunctionResultNode;
5049
import com.oracle.graal.python.builtins.objects.PNone;
51-
import com.oracle.graal.python.builtins.objects.PythonAbstractObject;
5250
import com.oracle.graal.python.builtins.objects.cext.CApiGuards;
5351
import com.oracle.graal.python.builtins.objects.cext.CExtNodes;
5452
import com.oracle.graal.python.builtins.objects.cext.CExtNodes.ConvertArgsToSulongNode;
5553
import com.oracle.graal.python.builtins.objects.cext.CExtNodes.SubRefCntNode;
5654
import com.oracle.graal.python.builtins.objects.cext.CExtNodes.ToJavaStealingNode;
5755
import com.oracle.graal.python.builtins.objects.cext.CExtNodes.ToNewRefNode;
5856
import com.oracle.graal.python.builtins.objects.cext.CExtNodesFactory.ToJavaStealingNodeGen;
59-
import com.oracle.graal.python.builtins.objects.cext.common.CExtCommonNodes.ConvertPIntToPrimitiveNode;
60-
import com.oracle.graal.python.builtins.objects.cext.common.CExtCommonNodesFactory.ConvertPIntToPrimitiveNodeGen;
6157
import com.oracle.graal.python.builtins.objects.cext.CExtNodesFactory.ToNewRefNodeGen;
6258
import com.oracle.graal.python.builtins.objects.cext.DynamicObjectNativeWrapper;
6359
import com.oracle.graal.python.builtins.objects.cext.PythonNativeWrapper;
64-
import com.oracle.graal.python.builtins.objects.cext.PythonNativeWrapperLibrary;
65-
import com.oracle.graal.python.builtins.objects.common.SequenceStorageNodes.GetElementType;
60+
import com.oracle.graal.python.builtins.objects.cext.common.CExtCommonNodes.ConvertPIntToPrimitiveNode;
61+
import com.oracle.graal.python.builtins.objects.cext.common.CExtCommonNodesFactory.ConvertPIntToPrimitiveNodeGen;
6662
import com.oracle.graal.python.builtins.objects.common.SequenceStorageNodes.ToArrayNode;
6763
import com.oracle.graal.python.builtins.objects.floats.PFloat;
6864
import com.oracle.graal.python.builtins.objects.function.PArguments;
6965
import com.oracle.graal.python.builtins.objects.function.PKeyword;
7066
import com.oracle.graal.python.builtins.objects.function.Signature;
7167
import com.oracle.graal.python.builtins.objects.ints.PInt;
68+
import com.oracle.graal.python.builtins.objects.object.PythonObject;
7269
import com.oracle.graal.python.builtins.objects.str.PString;
7370
import com.oracle.graal.python.builtins.objects.tuple.PTuple;
7471
import com.oracle.graal.python.nodes.ErrorMessages;
@@ -88,8 +85,6 @@
8885
import com.oracle.graal.python.runtime.ExecutionContext.ForeignCallContext;
8986
import com.oracle.graal.python.runtime.PythonContext;
9087
import com.oracle.graal.python.runtime.object.PythonObjectFactory;
91-
import com.oracle.graal.python.runtime.sequence.storage.SequenceStorage;
92-
import com.oracle.graal.python.runtime.sequence.storage.SequenceStorage.ListStorageType;
9388
import com.oracle.graal.python.util.PythonUtils;
9489
import com.oracle.truffle.api.Assumption;
9590
import com.oracle.truffle.api.CompilerDirectives;
@@ -107,14 +102,12 @@
107102
import com.oracle.truffle.api.interop.InteropLibrary;
108103
import com.oracle.truffle.api.interop.UnsupportedMessageException;
109104
import com.oracle.truffle.api.interop.UnsupportedTypeException;
110-
import com.oracle.truffle.api.library.CachedLibrary;
111105
import com.oracle.truffle.api.nodes.ExplodeLoop;
112106
import com.oracle.truffle.api.nodes.ExplodeLoop.LoopExplosionKind;
113107
import com.oracle.truffle.api.nodes.Node;
114108
import com.oracle.truffle.api.nodes.NodeCost;
115109
import com.oracle.truffle.api.nodes.UnexpectedResultException;
116110
import com.oracle.truffle.api.profiles.ConditionProfile;
117-
import com.oracle.truffle.api.profiles.ValueProfile;
118111

119112
public abstract class ExternalFunctionNodes {
120113

@@ -204,7 +197,6 @@ static final class ExternalFunctionInvokeNode extends PNodeWithContext implement
204197
@Child private ToJavaStealingNode asPythonObjectNode = ToJavaStealingNodeGen.create();
205198
@Child private InteropLibrary lib;
206199
@Child private PRaiseNode raiseNode;
207-
@Child private ReleaseNativeWrapperNode releaseNativeWrapperNode;
208200

209201
@CompilationFinal private Assumption nativeCodeDoesntNeedExceptionState = Truffle.getRuntime().createAssumption();
210202
@CompilationFinal private Assumption nativeCodeDoesntNeedMyFrame = Truffle.getRuntime().createAssumption();
@@ -264,8 +256,6 @@ public Object execute(VirtualFrame frame, String name, Object callable, Object[]
264256
// to simulate the global state semantics
265257
PArguments.setException(frame, ctx.getCaughtException());
266258
ForeignCallContext.exit(frame, ctx, state);
267-
268-
ensureReleaseNativeWrapperNode().execute(cArguments);
269259
}
270260
}
271261

@@ -281,14 +271,6 @@ private PRaiseNode ensureRaiseNode() {
281271
return raiseNode;
282272
}
283273

284-
private ReleaseNativeWrapperNode ensureReleaseNativeWrapperNode() {
285-
if (releaseNativeWrapperNode == null) {
286-
CompilerDirectives.transferToInterpreterAndInvalidate();
287-
releaseNativeWrapperNode = insert(ReleaseNativeWrapperNodeGen.create());
288-
}
289-
return releaseNativeWrapperNode;
290-
}
291-
292274
private PythonContext getContext() {
293275
if (contextRef == null) {
294276
CompilerDirectives.transferToInterpreterAndInvalidate();
@@ -321,57 +303,21 @@ public static ExternalFunctionInvokeNode create(ConvertArgsToSulongNode convertA
321303
*/
322304
abstract static class ReleaseNativeWrapperNode extends Node {
323305

324-
public abstract void execute(Object[] nativeArguments);
306+
public abstract void execute(Object pythonObject, PythonNativeWrapper nativeWrapper);
325307

326-
@Specialization(guards = {"nativeArguments.length == cachedLength", "nativeArguments.length < 8"}, limit = "1")
327-
@ExplodeLoop
328-
static void doCachedLength(Object[] nativeArguments,
329-
@Cached("nativeArguments.length") int cachedLength,
330-
@Cached(value = "createClassProfiles(cachedLength)", dimensions = 1) ValueProfile[] classProfiles,
331-
@CachedLibrary(limit = "nativeArguments.length") PythonNativeWrapperLibrary lib,
332-
@Cached("createTraverseNodes(cachedLength)") TraverseNativeWrapperNode[] traverseNativeWrapperNodes,
333-
@Cached SubRefCntNode subRefCntNode) {
334-
335-
for (int i = 0; i < cachedLength; i++) {
336-
doCheck(classProfiles[i].profile(nativeArguments[i]), subRefCntNode, lib, traverseNativeWrapperNodes[i]);
337-
}
338-
}
339-
340-
@Specialization(replaces = "doCachedLength")
341-
static void doGeneric(Object[] nativeArguments,
342-
@Cached("createClassProfile()") ValueProfile classProfile,
343-
@CachedLibrary(limit = "3") PythonNativeWrapperLibrary lib,
308+
@Specialization
309+
static void doPythonObjectWithWrapper(PythonObject pythonObject, PythonNativeWrapper nativeWrapper,
344310
@Cached TraverseNativeWrapperNode traverseNativeWrapperNode,
345-
@Cached SubRefCntNode freeNode) {
346-
347-
for (int i = 0; i < nativeArguments.length; i++) {
348-
doCheck(classProfile.profile(nativeArguments[i]), freeNode, lib, traverseNativeWrapperNode);
349-
}
350-
}
351-
352-
private static void doCheck(Object argument, SubRefCntNode refCntNode, PythonNativeWrapperLibrary lib, TraverseNativeWrapperNode traverseNativeWrapperNode) {
353-
if (CApiGuards.isNativeWrapper(argument)) {
354-
// in the cached case, refCntNode acts as a branch profile
355-
if (refCntNode.dec(argument) == 0) {
356-
traverseNativeWrapperNode.execute(lib.getDelegate((PythonNativeWrapper) argument));
357-
}
358-
}
359-
}
311+
@Cached SubRefCntNode subRefCntNode) {
360312

361-
static ValueProfile[] createClassProfiles(int length) {
362-
ValueProfile[] classProfiles = new ValueProfile[length];
363-
for (int i = 0; i < classProfiles.length; i++) {
364-
classProfiles[i] = ValueProfile.createClassProfile();
313+
// in the cached case, refCntNode acts as a branch profile
314+
if (subRefCntNode.dec(nativeWrapper) == 0) {
315+
traverseNativeWrapperNode.execute(pythonObject);
365316
}
366-
return classProfiles;
367317
}
368318

369-
static TraverseNativeWrapperNode[] createTraverseNodes(int length) {
370-
TraverseNativeWrapperNode[] nodes = new TraverseNativeWrapperNode[length];
371-
for (int i = 0; i < nodes.length; i++) {
372-
nodes[i] = TraverseNativeWrapperNodeGen.create();
373-
}
374-
return nodes;
319+
static boolean hasNativeWrapper(PythonObject object) {
320+
return CApiGuards.isNativeWrapper(object.getNativeWrapper());
375321
}
376322
}
377323

@@ -385,34 +331,26 @@ abstract static class TraverseNativeWrapperNode extends Node {
385331

386332
@Specialization
387333
static void doTuple(PTuple tuple,
388-
@Cached GetElementType getElementTypeNode,
389334
@Cached ToArrayNode toArrayNode,
390-
@Cached ReleaseNativeWrapperNode releaseNativeWrapperNode) {
391-
392-
SequenceStorage sequenceStorage = tuple.getSequenceStorage();
393-
ListStorageType storageType = getElementTypeNode.execute(sequenceStorage);
394-
if (storageType == ListStorageType.Generic || storageType == ListStorageType.List || storageType == ListStorageType.Tuple) {
395-
Object[] values = toArrayNode.execute(sequenceStorage);
396-
Object[] wrappers = new Object[values.length];
397-
for (int i = 0; i < values.length; i++) {
398-
Object value = values[i];
399-
if (value instanceof PythonAbstractObject) {
400-
DynamicObjectNativeWrapper nativeWrapper = ((PythonAbstractObject) value).getNativeWrapper();
401-
// only traverse if refCnt != 0; this will break the cycle
402-
if (nativeWrapper != null) {
403-
wrappers[i] = nativeWrapper.getRefCount() != 0 ? nativeWrapper : null;
404-
}
335+
@Cached SubRefCntNode subRefCntNode) {
336+
337+
Object[] values = toArrayNode.execute(tuple.getSequenceStorage());
338+
for (int i = 0; i < values.length; i++) {
339+
Object value = values[i];
340+
if (value instanceof PythonObject) {
341+
DynamicObjectNativeWrapper nativeWrapper = ((PythonObject) value).getNativeWrapper();
342+
// only traverse if refCnt != 0; this will break the cycle
343+
if (nativeWrapper != null) {
344+
subRefCntNode.dec(nativeWrapper);
405345
}
406346
}
407-
releaseNativeWrapperNode.execute(wrappers);
408347
}
409348
}
410349

411350
@Fallback
412351
static void doOther(@SuppressWarnings("unused") Object other) {
413352
// do nothing
414353
}
415-
416354
}
417355

418356
abstract static class MethodDescriptorRoot extends PRootNode {
@@ -447,7 +385,12 @@ public Object execute(VirtualFrame frame) {
447385
CalleeContext.enter(frame, customLocalsProfile);
448386
try {
449387
if (externalInvokeNode != null) {
450-
return externalInvokeNode.execute(frame, name, callable, prepareCArguments(frame), 0);
388+
Object[] cArguments = prepareCArguments(frame);
389+
try {
390+
return externalInvokeNode.execute(frame, name, callable, cArguments, 0);
391+
} finally {
392+
postprocessCArguments(frame, cArguments);
393+
}
451394
} else {
452395
assert externalInvokeNode == null;
453396
return invokeNode.execute(frame, callable, preparePArguments(frame), PArguments.getKeywordArguments(frame));
@@ -459,6 +402,10 @@ public Object execute(VirtualFrame frame) {
459402

460403
protected abstract Object[] prepareCArguments(VirtualFrame frame);
461404

405+
protected void postprocessCArguments(VirtualFrame frame, Object[] cArguments) {
406+
// default: do nothing
407+
}
408+
462409
protected Object[] preparePArguments(VirtualFrame frame) {
463410
Object[] variableArguments = PArguments.getVariableArguments(frame);
464411

@@ -522,6 +469,7 @@ public static final class MethKeywordsRoot extends MethodDescriptorRoot {
522469
@Child private ReadVarArgsNode readVarargsNode;
523470
@Child private ReadVarKeywordsNode readKwargsNode;
524471
@Child private CreateArgsTupleNode createArgsTupleNode;
472+
@Child private ReleaseNativeWrapperNode releaseNativeWrapperNode;
525473

526474
public MethKeywordsRoot(PythonLanguage language, String name, Object callable) {
527475
super(language, name, callable);
@@ -543,17 +491,31 @@ protected Object[] prepareCArguments(VirtualFrame frame) {
543491
return new Object[]{self, createArgsTupleNode.execute(factory, args), factory.createDict(kwargs)};
544492
}
545493

494+
protected void postprocessCArguments(VirtualFrame frame, Object[] cArguments) {
495+
PTuple varargsTuple = (PTuple) cArguments[1];
496+
ensureReleaseNativeWrapperNode().execute(varargsTuple, varargsTuple.getNativeWrapper());
497+
}
498+
546499
@Override
547500
public Signature getSignature() {
548501
return SIGNATURE;
549502
}
503+
504+
private ReleaseNativeWrapperNode ensureReleaseNativeWrapperNode() {
505+
if (releaseNativeWrapperNode == null) {
506+
CompilerDirectives.transferToInterpreterAndInvalidate();
507+
releaseNativeWrapperNode = insert(ReleaseNativeWrapperNodeGen.create());
508+
}
509+
return releaseNativeWrapperNode;
510+
}
550511
}
551512

552513
public static final class MethVarargsRoot extends MethodDescriptorRoot {
553514
private static final Signature SIGNATURE = new Signature(-1, false, 1, false, new String[]{"self"}, new String[0]);
554515
@Child private PythonObjectFactory factory;
555516
@Child private ReadVarArgsNode readVarargsNode;
556517
@Child private CreateArgsTupleNode createArgsTupleNode;
518+
@Child private ReleaseNativeWrapperNode releaseNativeWrapperNode;
557519

558520
public MethVarargsRoot(PythonLanguage language, String name, Object callable) {
559521
super(language, name, callable);
@@ -573,10 +535,23 @@ protected Object[] prepareCArguments(VirtualFrame frame) {
573535
return new Object[]{self, createArgsTupleNode.execute(factory, args)};
574536
}
575537

538+
protected void postprocessCArguments(VirtualFrame frame, Object[] cArguments) {
539+
PTuple varargsTuple = (PTuple) cArguments[1];
540+
ensureReleaseNativeWrapperNode().execute(varargsTuple, varargsTuple.getNativeWrapper());
541+
}
542+
576543
@Override
577544
public Signature getSignature() {
578545
return SIGNATURE;
579546
}
547+
548+
private ReleaseNativeWrapperNode ensureReleaseNativeWrapperNode() {
549+
if (releaseNativeWrapperNode == null) {
550+
CompilerDirectives.transferToInterpreterAndInvalidate();
551+
releaseNativeWrapperNode = insert(ReleaseNativeWrapperNodeGen.create());
552+
}
553+
return releaseNativeWrapperNode;
554+
}
580555
}
581556

582557
public static final class MethNoargsRoot extends MethodDescriptorRoot {

0 commit comments

Comments
 (0)