Fix: message PythonNativeWrapperLibrary.isNative/getNativePointer did not consider assumption.

Author: fangerer

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/PythonAbstractObject.java

@@ -79,6 +79,7 @@
 import com.oracle.graal.python.builtins.objects.bytes.PBytes;
 import com.oracle.graal.python.builtins.objects.cext.CApiGuards;
 import com.oracle.graal.python.builtins.objects.cext.DynamicObjectNativeWrapper;
+import com.oracle.graal.python.builtins.objects.cext.PythonNativeWrapper;
 import com.oracle.graal.python.builtins.objects.common.SequenceNodes;
 import com.oracle.graal.python.builtins.objects.dict.PDict;
 import com.oracle.graal.python.builtins.objects.function.PArguments;
@@ -134,6 +135,7 @@
 import com.oracle.graal.python.runtime.PythonOptions;
 import com.oracle.graal.python.runtime.exception.PException;
 import com.oracle.graal.python.util.OverflowException;
+import com.oracle.truffle.api.Assumption;
 import com.oracle.truffle.api.CompilerDirectives;
 import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
 import com.oracle.truffle.api.TruffleLanguage;
@@ -186,7 +188,7 @@ public final void clearNativeWrapper(ConditionProfile hasHandleValidAssumptionPr
         // The null check is important because it might be that we actually never got a to-native
         // message but still modified the reference count.
         if (hasHandleValidAssumptionProfile.profile(nativeWrapper != null && nativeWrapper.getHandleValidAssumption() != null)) {
-            nativeWrapper.getHandleValidAssumption().invalidate("releasing handle for native wrapper");
+            PythonNativeWrapper.invalidateAssumption(nativeWrapper.getHandleValidAssumption());
         }
         nativeWrapper = null;
     }

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

@@ -3054,7 +3054,7 @@ static void doPrimitiveNativeWrapper(@SuppressWarnings("unused") Object delegate
             assert !isSmallIntegerWrapperSingleton(contextRef, nativeWrapper) : "clearing primitive native wrapper singleton of small integer";
             Assumption handleValidAssumption = nativeWrapper.getHandleValidAssumption();
             if (hasHandleValidAssumptionProfile.profile(handleValidAssumption != null)) {
-                invalidate(handleValidAssumption);
+                PythonNativeWrapper.invalidateAssumption(handleValidAssumption);
             }
         }
 
@@ -3075,11 +3075,6 @@ static void doOther(@SuppressWarnings("unused") Object delegate, @SuppressWarnin
             // ignore
         }
 
-        @TruffleBoundary
-        private static void invalidate(Assumption assumption) {
-            assumption.invalidate("releasing handle for native wrapper");
-        }
-
         static boolean isPrimitiveNativeWrapper(PythonNativeWrapper nativeWrapper) {
             return nativeWrapper instanceof PrimitiveNativeWrapper;
         }

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

@@ -45,13 +45,15 @@
 import com.oracle.graal.python.PythonLanguage;
 import com.oracle.truffle.api.Assumption;
 import com.oracle.truffle.api.CompilerAsserts;
+import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
 import com.oracle.truffle.api.Truffle;
 import com.oracle.truffle.api.dsl.Cached;
 import com.oracle.truffle.api.dsl.Cached.Exclusive;
 import com.oracle.truffle.api.dsl.Specialization;
 import com.oracle.truffle.api.interop.TruffleObject;
 import com.oracle.truffle.api.library.ExportLibrary;
 import com.oracle.truffle.api.library.ExportMessage;
+import com.oracle.truffle.api.nodes.InvalidAssumptionException;
 
 @ExportLibrary(PythonNativeWrapperLibrary.class)
 public abstract class PythonNativeWrapper implements TruffleObject {
@@ -98,6 +100,11 @@ public final void setRefCount(long refCount) {
         this.refCount = refCount;
     }
 
+    @TruffleBoundary
+    public static void invalidateAssumption(Assumption handleValidAssumption) {
+        handleValidAssumption.invalidate("releasing handle for native wrapper");
+    }
+
     public final Assumption getHandleValidAssumption() {
         return handleValidAssumption;
     }
@@ -122,6 +129,15 @@ protected static WeakReference<PythonNativeWrapper> weak(PythonNativeWrapper wra
         return new WeakReference<>(wrapper);
     }
 
+    protected static WeakReference<Assumption> weakAssumption(PythonNativeWrapper wrapper) {
+        Assumption handleValidAssumption = wrapper.getHandleValidAssumption();
+        // The assumption only exists if the wrapper was used in a cache at some point.
+        if (handleValidAssumption != null) {
+            return new WeakReference<>(handleValidAssumption);
+        }
+        return null;
+    }
+
     @ExportMessage(name = "getDelegate")
     protected static class GetDelegate {
         @Specialization(guards = {"isEq(cachedWrapper.get(), wrapper)", "!isEq(delegate.get(), null)"}, assumptions = "singleContextAssumption()")
@@ -148,10 +164,16 @@ protected void setDelegate(Object delegate) {
 
     @ExportMessage(name = "getNativePointer")
     protected static class GetNativePointer {
-        @Specialization(guards = {"isEq(cachedWrapper.get(), wrapper)", "!isEq(nativePointer.get(), null)"}, assumptions = "singleContextAssumption()")
+        @Specialization(guards = {"isEq(cachedWrapper.get(), wrapper)", "!isEq(nativePointer.get(), null)"}, assumptions = "singleContextAssumption()", //
+                        rewriteOn = InvalidAssumptionException.class)
         protected static Object getCachedPtr(@SuppressWarnings("unused") PythonNativeWrapper wrapper,
                         @Exclusive @SuppressWarnings("unused") @Cached("weak(wrapper)") WeakReference<PythonNativeWrapper> cachedWrapper,
-                        @Exclusive @Cached("wrapper.getNativePointerPrivate()") WeakReference<Object> nativePointer) {
+                        @Exclusive @Cached("weakAssumption(wrapper)") WeakReference<Assumption> cachedAssumption,
+                        @Exclusive @Cached("wrapper.getNativePointerPrivate()") WeakReference<Object> nativePointer) throws InvalidAssumptionException {
+            if (cachedAssumption != null) {
+                // as long as the wrapper exists, the assumption will also exist
+                cachedAssumption.get().check();
+            }
             return nativePointer.get();
         }
 
@@ -176,16 +198,27 @@ public void setNativePointer(Object nativePointer) {
 
     @ExportMessage(name = "isNative")
     protected static class IsNative {
-        @Specialization(guards = {"isEq(cachedWrapper.get(), wrapper)", "!isEq(nativePointer.get(), null)"}, assumptions = "singleContextAssumption()")
+        @Specialization(guards = {"isEq(cachedWrapper.get(), wrapper)", "!isEq(nativePointer.get(), null)"}, assumptions = "singleContextAssumption()", //
+                        rewriteOn = InvalidAssumptionException.class)
         protected static boolean isCachedNative(@SuppressWarnings("unused") PythonNativeWrapper wrapper,
                         @Exclusive @SuppressWarnings("unused") @Cached("weak(wrapper)") WeakReference<PythonNativeWrapper> cachedWrapper,
-                        @Exclusive @SuppressWarnings("unused") @Cached("wrapper.getNativePointerPrivate()") WeakReference<Object> nativePointer) {
+                        @Exclusive @Cached("weakAssumption(wrapper)") WeakReference<Assumption> cachedAssumption,
+                        @Exclusive @SuppressWarnings("unused") @Cached("wrapper.getNativePointerPrivate()") WeakReference<Object> nativePointer) throws InvalidAssumptionException {
+            if (cachedAssumption != null) {
+                // as long as the wrapper exists, the assumption will also exist
+                cachedAssumption.get().check();
+            }
             return true;
         }
 
         @Specialization(replaces = "isCachedNative")
         protected static boolean isNative(PythonNativeWrapper wrapper) {
-            return wrapper.nativePointer != null;
+            if (wrapper.nativePointer != null) {
+                Assumption handleValidAssumption = wrapper.getHandleValidAssumption();
+                // If an assumption exists, it must be valid
+                return handleValidAssumption == null || handleValidAssumption.isValid();
+            }
+            return false;
         }
     }
 }