Skip to content

Commit 7222197

Browse files
fangererfangerer
committed
Fix: do not cache native pointers in multi-context mode.
1 parent d3ad596 commit 7222197

File tree

4 files changed

+78
-32
lines changed

4 files changed

+78
-32
lines changed

‎graalpython/com.oracle.graal.python.test/src/com/oracle/graal/python/test/advance/MultiContextTest.java‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2017, 2018, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2017, 2019, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* The Universal Permissive License (UPL), Version 1.0
@@ -48,7 +48,7 @@
4848

4949
public class MultiContextTest extends PythonTests {
5050
@Test
51-
public void testContextReuse() {
51+
public void testSharingWithMemoryview() {
5252
Engine engine = Engine.newBuilder().build();
5353
for (int i = 0; i < 10; i++) {
5454
try (Context context = newContext(engine)) {
@@ -57,6 +57,17 @@ public void testContextReuse() {
5757
}
5858
}
5959

60+
@Test
61+
public void testSharingWithStruct() {
62+
Engine engine = Engine.newBuilder().build();
63+
for (int i = 0; i < 10; i++) {
64+
try (Context context = newContext(engine)) {
65+
context.eval("python", "import struct\n" +
66+
"n = struct.unpack('<q', struct.pack('<d', 1.1))[0]\n");
67+
}
68+
}
69+
}
70+
6071
private static Context newContext(Engine engine) {
6172
return Context.newBuilder().allowAllAccess(true).engine(engine).build();
6273
}

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/cext/CExtNodes.java‎

Lines changed: 47 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -475,8 +475,9 @@ Object doNativeWrapper(PythonNativeWrapper object) {
475475
Object doNativeObject(TruffleObject object,
476476
@Cached PythonObjectFactory factory,
477477
@SuppressWarnings("unused") @Cached("create()") GetLazyClassNode getClassNode,
478-
@SuppressWarnings("unused") @Cached("create()") IsBuiltinClassProfile isForeignClassProfile) {
479-
return factory.createNativeObjectWrapper(object);
478+
@SuppressWarnings("unused") @Cached("create()") IsBuiltinClassProfile isForeignClassProfile,
479+
@CachedContext(PythonLanguage.class) PythonContext context) {
480+
return factory.createNativeObjectWrapper(object, context);
480481
}
481482

482483
@Specialization
@@ -543,13 +544,13 @@ public static Object doSlowPath(Object object, boolean forceNativeClass) {
543544
return ((PythonNativeWrapper) object).getDelegate();
544545
} else if (IsBuiltinClassProfile.profileClassSlowPath(GetClassNode.getUncached().execute(object), PythonBuiltinClassType.TruffleObject)) {
545546
if (forceNativeClass) {
546-
return PythonLanguage.getCore().factory().createNativeClassWrapper((TruffleObject) object);
547+
return PythonObjectFactory.getUncached().createNativeClassWrapper((TruffleObject) object);
547548
}
548-
return PythonLanguage.getCore().factory().createNativeObjectWrapper((TruffleObject) object);
549+
return PythonObjectFactory.getUncached().createNativeObjectWrapper((TruffleObject) object);
549550
} else if (object instanceof String || object instanceof Number || object instanceof Boolean || object instanceof PythonNativeNull || object instanceof PythonAbstractObject) {
550551
return object;
551552
}
552-
throw PythonLanguage.getCore().raise(PythonErrorType.SystemError, "invalid object from native: %s", object);
553+
throw PRaiseNode.getUncached().raise(PythonErrorType.SystemError, "invalid object from native: %s", object);
553554
}
554555

555556
// TODO(fa): Workaround for DSL bug: did not import factory at users
@@ -1673,11 +1674,14 @@ public abstract static class GetTypeMemberNode extends CExtBaseNode {
16731674
* native context, so we can be sure that the "nativeClassStableAssumption" (which is
16741675
* per-context) is from the context in which this native object was created.
16751676
*/
1676-
@Specialization(guards = {"cachedObj.equals(obj)", "memberName == cachedMemberName"}, limit = "1", assumptions = "getNativeClassStableAssumption(cachedObj)")
1677-
public Object doCachedObj(@SuppressWarnings("unused") PythonNativeClass obj, @SuppressWarnings("unused") String memberName,
1677+
@Specialization(guards = {"isSameNativeObjectNode.execute(cachedObj, obj)", "memberName == cachedMemberName"}, //
1678+
limit = "1", //
1679+
assumptions = {"getNativeClassStableAssumption(cachedObj)", "singleContextAssumption()"})
1680+
public Object doCachedObj(@SuppressWarnings("unused") PythonAbstractNativeObject obj, @SuppressWarnings("unused") String memberName,
1681+
@SuppressWarnings("unused") @Cached IsSameNativeObjectFastNode isSameNativeObjectNode,
16781682
@SuppressWarnings("unused") @Cached("memberName") String cachedMemberName,
16791683
@SuppressWarnings("unused") @Cached("getterFuncName(memberName)") String getterFuncName,
1680-
@Cached("obj") @SuppressWarnings("unused") PythonNativeClass cachedObj,
1684+
@Cached("obj") @SuppressWarnings("unused") PythonAbstractNativeObject cachedObj,
16811685
@Cached("doSlowPath(obj, getterFuncName)") Object result) {
16821686
return result;
16831687
}
@@ -1757,4 +1761,39 @@ static Object getNativeNullWithoutModule(@SuppressWarnings("unused") Object modu
17571761
}
17581762

17591763
}
1764+
1765+
public abstract static class IsSameNativeObjectNode extends CExtBaseNode {
1766+
1767+
public abstract boolean execute(PythonAbstractNativeObject left, PythonAbstractNativeObject right);
1768+
1769+
protected static boolean doNativeFast(PythonAbstractNativeObject left, PythonAbstractNativeObject right) {
1770+
// This check is a bit dangerous since we cannot be sure about the code that is running.
1771+
// Currently, we assume that the pointer object is a Sulong pointer and for this it's
1772+
// fine.
1773+
return left.equals(right);
1774+
}
1775+
1776+
}
1777+
1778+
@GenerateUncached
1779+
public abstract static class IsSameNativeObjectFastNode extends IsSameNativeObjectNode {
1780+
1781+
@Specialization
1782+
boolean doSingleContext(PythonAbstractNativeObject left, PythonAbstractNativeObject right) {
1783+
return IsSameNativeObjectNode.doNativeFast(left, right);
1784+
}
1785+
}
1786+
1787+
@GenerateUncached
1788+
public abstract static class IsSameNativeObjectSlowNode extends IsSameNativeObjectNode {
1789+
1790+
@Specialization
1791+
boolean doSingleContext(PythonAbstractNativeObject left, PythonAbstractNativeObject right,
1792+
@Cached PointerCompareNode pointerCompareNode) {
1793+
if (IsSameNativeObjectNode.doNativeFast(left, right)) {
1794+
return true;
1795+
}
1796+
return pointerCompareNode.execute(SpecialMethodNames.__EQ__, left, right);
1797+
}
1798+
}
17601799
}

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/type/TypeNodes.java‎

Lines changed: 12 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,6 @@
4242

4343
import static com.oracle.graal.python.builtins.PythonBuiltinClassType.AttributeError;
4444
import static com.oracle.graal.python.builtins.PythonBuiltinClassType.SystemError;
45-
import static com.oracle.graal.python.nodes.SpecialMethodNames.__EQ__;
4645

4746
import java.util.ArrayList;
4847
import java.util.Collection;
@@ -54,6 +53,9 @@
5453
import com.oracle.graal.python.builtins.PythonBuiltinClassType;
5554
import com.oracle.graal.python.builtins.objects.cext.CExtNodes;
5655
import com.oracle.graal.python.builtins.objects.cext.CExtNodes.GetTypeMemberNode;
56+
import com.oracle.graal.python.builtins.objects.cext.CExtNodes.IsSameNativeObjectNode;
57+
import com.oracle.graal.python.builtins.objects.cext.CExtNodesFactory.IsSameNativeObjectFastNodeGen;
58+
import com.oracle.graal.python.builtins.objects.cext.CExtNodesFactory.IsSameNativeObjectSlowNodeGen;
5759
import com.oracle.graal.python.builtins.objects.cext.NativeMemberNames;
5860
import com.oracle.graal.python.builtins.objects.cext.PythonAbstractNativeObject;
5961
import com.oracle.graal.python.builtins.objects.cext.PythonNativeClass;
@@ -604,38 +606,29 @@ boolean doManaged(PythonBuiltinClass left, PythonBuiltinClassType right) {
604606
}
605607

606608
@Specialization
607-
boolean doNativeSlow(PythonAbstractNativeObject left, PythonAbstractNativeObject right) {
608-
if (fastCheck) {
609-
return doNativeFast(left, right);
610-
}
611-
if (doNativeFast(left, right)) {
612-
return true;
613-
}
614-
if (pointerCompareNode == null) {
615-
CompilerDirectives.transferToInterpreterAndInvalidate();
616-
pointerCompareNode = insert(CExtNodes.PointerCompareNode.create());
617-
}
618-
return pointerCompareNode.execute(__EQ__, left, right);
609+
boolean doNativeSingleContext(PythonAbstractNativeObject left, PythonAbstractNativeObject right,
610+
@Cached("createNativeEquals()") IsSameNativeObjectNode isSameNativeObjectNode) {
611+
return isSameNativeObjectNode.execute(left, right);
619612
}
620613

621614
@Fallback
622615
boolean doOther(@SuppressWarnings("unused") Object left, @SuppressWarnings("unused") Object right) {
623616
return false;
624617
}
625618

626-
private static boolean doNativeFast(PythonAbstractNativeObject left, PythonAbstractNativeObject right) {
627-
// This check is a bit dangerous since we cannot be sure about the code that is running.
628-
// Currently, we assume that the pointer object is a Sulong pointer and for this it's
629-
// fine.
630-
return left.object.equals(right.object);
619+
protected IsSameNativeObjectNode createNativeEquals() {
620+
if (fastCheck) {
621+
return IsSameNativeObjectFastNodeGen.create();
622+
}
623+
return IsSameNativeObjectSlowNodeGen.create();
631624
}
632625

633626
@TruffleBoundary
634627
public static boolean doSlowPath(Object left, Object right) {
635628
if (left instanceof PythonManagedClass && right instanceof PythonManagedClass) {
636629
return left == right;
637630
} else if (left instanceof PythonAbstractNativeObject && right instanceof PythonAbstractNativeObject) {
638-
return CExtNodes.PointerCompareNode.getUncached().execute(__EQ__, left, right);
631+
return IsSameNativeObjectFastNodeGen.getUncached().execute((PythonAbstractNativeObject) left, (PythonAbstractNativeObject) right);
639632
}
640633
return false;
641634
}

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/attributes/LookupAttributeInMRONode.java‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -211,8 +211,9 @@ protected PythonClassAssumptionPair findAttrClassAndAssumptionInMRO(PythonAbstra
211211
return new PythonClassAssumptionPair(attrAssumption, PNone.NO_VALUE);
212212
}
213213

214-
@Specialization(guards = {"isSameType(cachedKlass, klass)", "cachedClassInMROInfo != null"}, limit = "getAttributeAccessInlineCacheMaxDepth()", assumptions = {
215-
"cachedClassInMROInfo.assumption"})
214+
@Specialization(guards = {"isSameType(cachedKlass, klass)", "cachedClassInMROInfo != null"}, //
215+
limit = "getAttributeAccessInlineCacheMaxDepth()", //
216+
assumptions = {"cachedClassInMROInfo.assumption", "singleContextAssumption()"})
216217
protected Object lookupConstantMROCached(@SuppressWarnings("unused") PythonAbstractClass klass,
217218
@Cached("klass") @SuppressWarnings("unused") PythonAbstractClass cachedKlass,
218219
@Cached("findAttrClassAndAssumptionInMRO(cachedKlass)") PythonClassAssumptionPair cachedClassInMROInfo) {
@@ -227,7 +228,9 @@ protected ReadAttributeFromObjectNode[] create(int size) {
227228
return nodes;
228229
}
229230

230-
@Specialization(guards = {"isSameType(cachedKlass, klass)", "mroLength < 32"}, limit = "getAttributeAccessInlineCacheMaxDepth()", assumptions = "lookupStable")
231+
@Specialization(guards = {"isSameType(cachedKlass, klass)", "mroLength < 32"}, //
232+
limit = "getAttributeAccessInlineCacheMaxDepth()", //
233+
assumptions = {"lookupStable", "singleContextAssumption()"})
231234
@ExplodeLoop(kind = ExplodeLoop.LoopExplosionKind.FULL_EXPLODE_UNTIL_RETURN)
232235
protected Object lookupConstantMRO(@SuppressWarnings("unused") PythonAbstractClass klass,
233236
@Cached("klass") @SuppressWarnings("unused") PythonAbstractClass cachedKlass,

0 commit comments

Comments
 (0)