[GR-19494] Fix: protect Rf_unprotect'ed until next GNU-R compatible GC cycle.

PullRequest: fastr/2225

Author: gilles-duboscq

com.oracle.truffle.r.ffi.impl/src/com/oracle/truffle/r/ffi/impl/nodes/ReleaseObjectNode.java

@@ -59,6 +59,9 @@ Object protect(RBaseObject x,
             int decrementAndGet = atomicInteger.decrementAndGet();
             if (decrementAndGet == 0) {
                 // remove from "list"
+                // Note: developers expect the "unprotected" references to be still alive until next
+                // GNU-R compatible GC cycle
+                ctx.registerReferenceUsedInNative(x);
                 preserveList.removeKey(x);
             }
         } else {

com.oracle.truffle.r.ffi.impl/src/com/oracle/truffle/r/ffi/impl/nodes/UnprotectNode.java

@@ -30,11 +30,14 @@
 import com.oracle.truffle.api.dsl.Specialization;
 import com.oracle.truffle.api.library.CachedLibrary;
 import com.oracle.truffle.api.nodes.ExplodeLoop;
+import com.oracle.truffle.api.profiles.BranchProfile;
 import com.oracle.truffle.r.runtime.Collections.StackLibrary;
 import com.oracle.truffle.r.runtime.RError;
 import com.oracle.truffle.r.runtime.context.RContext;
 import com.oracle.truffle.r.runtime.context.TruffleRLanguage;
 import com.oracle.truffle.r.runtime.data.RNull;
+import com.oracle.truffle.r.runtime.ffi.RFFIContext;
+import com.oracle.truffle.r.runtime.ffi.RFFILog;
 
 @GenerateUncached
 public abstract class UnprotectNode extends FFIUpCallNode.Arg1 {
@@ -54,11 +57,13 @@ Object unprotectNothing(@SuppressWarnings("unused") int n) {
 
     @Specialization(guards = "n == 1")
     Object unprotectSingle(@SuppressWarnings("unused") int n,
+                    @Cached BranchProfile registerNativeRefProfile,
                     @CachedContext(TruffleRLanguage.class) ContextReference<RContext> ctxRef,
                     @CachedLibrary(limit = "1") StackLibrary stacks) {
         RContext ctx = ctxRef.get();
+        RFFIContext rffiCtx = ctx.getRFFI();
         try {
-            stacks.pop(ctx.getStateRFFI().rffiContextState.protectStack);
+            popProtectedObject(ctx, rffiCtx, stacks, registerNativeRefProfile);
         } catch (IndexOutOfBoundsException e) {
             debugWarning("mismatched protect/unprotect (unprotect with empty protect stack)");
         }
@@ -69,12 +74,14 @@ Object unprotectSingle(@SuppressWarnings("unused") int n,
     @ExplodeLoop
     Object unprotectMultipleCached(@SuppressWarnings("unused") int n,
                     @Cached("n") int nCached,
+                    @Cached BranchProfile registerNativeRefProfile,
                     @CachedContext(TruffleRLanguage.class) ContextReference<RContext> ctxRef,
                     @CachedLibrary(limit = "1") StackLibrary stacks) {
         RContext ctx = ctxRef.get();
+        RFFIContext rffiCtx = ctx.getRFFI();
         try {
             for (int i = 0; i < nCached; i++) {
-                stacks.pop(ctx.getStateRFFI().rffiContextState.protectStack);
+                popProtectedObject(ctx, rffiCtx, stacks, registerNativeRefProfile);
             }
         } catch (IndexOutOfBoundsException e) {
             debugWarning("mismatched protect/unprotect (unprotect with empty protect stack)");
@@ -84,19 +91,31 @@ Object unprotectMultipleCached(@SuppressWarnings("unused") int n,
 
     @Specialization(guards = "n > 1", replaces = "unprotectMultipleCached")
     Object unprotectMultipleUnchached(int n,
+                    @Cached BranchProfile registerNativeRefProfile,
                     @CachedContext(TruffleRLanguage.class) ContextReference<RContext> ctxRef,
                     @CachedLibrary(limit = "1") StackLibrary stacks) {
         RContext ctx = ctxRef.get();
+        RFFIContext rffiCtx = ctx.getRFFI();
         try {
             for (int i = 0; i < n; i++) {
-                stacks.pop(ctx.getStateRFFI().rffiContextState.protectStack);
+                popProtectedObject(ctx, rffiCtx, stacks, registerNativeRefProfile);
             }
         } catch (IndexOutOfBoundsException e) {
             debugWarning("mismatched protect/unprotect (unprotect with empty protect stack)");
         }
         return RNull.instance;
     }
 
+    private static void popProtectedObject(RContext ctx, RFFIContext rffiCtx, StackLibrary stacks, BranchProfile registerNativeRefProfile) {
+        Object removed = stacks.pop(ctx.getStateRFFI().rffiContextState.protectStack);
+        // Developers expect the "unprotected" references to be still alive until next GNU-R
+        // compatible GC cycle
+        rffiCtx.registerReferenceUsedInNative(removed, registerNativeRefProfile);
+        if (RFFILog.logEnabled()) {
+            RFFILog.logRObject("Unprotected: ", removed);
+        }
+    }
+
     private static boolean debugWarning(String message) {
         CompilerDirectives.transferToInterpreter();
         RError.warning(RError.SHOW_CALLER, RError.Message.GENERIC, message);

com.oracle.truffle.r.ffi.impl/src/com/oracle/truffle/r/ffi/impl/nodes/UnprotectPtrNode.java

@@ -23,14 +23,17 @@
 package com.oracle.truffle.r.ffi.impl.nodes;
 
 import com.oracle.truffle.api.TruffleLanguage.ContextReference;
+import com.oracle.truffle.api.dsl.Cached;
 import com.oracle.truffle.api.dsl.CachedContext;
 import com.oracle.truffle.api.dsl.GenerateUncached;
 import com.oracle.truffle.api.dsl.Specialization;
+import com.oracle.truffle.api.profiles.BranchProfile;
 import com.oracle.truffle.r.runtime.Collections;
 import com.oracle.truffle.r.runtime.context.RContext;
 import com.oracle.truffle.r.runtime.context.TruffleRLanguage;
 import com.oracle.truffle.r.runtime.data.RBaseObject;
 import com.oracle.truffle.r.runtime.ffi.RFFIContext;
+import com.oracle.truffle.r.runtime.ffi.RFFILog;
 
 @GenerateUncached
 public abstract class UnprotectPtrNode extends FFIUpCallNode.Arg1 {
@@ -45,11 +48,18 @@ public static UnprotectPtrNode getUncached() {
 
     @Specialization
     Object unprotect(RBaseObject x,
+                    @Cached BranchProfile registerNativeRefProfile,
                     @CachedContext(TruffleRLanguage.class) ContextReference<RContext> ctxRef) {
         RFFIContext ctx = ctxRef.get().getStateRFFI();
         Collections.ArrayListObj<RBaseObject> stack = ctx.rffiContextState.protectStack;
         for (int i = stack.size() - 1; i >= 0; i--) {
-            if (stack.get(i) == x) {
+            RBaseObject current = stack.get(i);
+            if (current == x) {
+                if (RFFILog.logEnabled()) {
+                    RFFILog.logRObject("Unprotected: ", current);
+                }
+                ctx.registerReferenceUsedInNative(current, registerNativeRefProfile);
+                stack.remove(i);
                 return null;
             }
         }

com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/data/NativeDataAccess.java

@@ -479,15 +479,19 @@ public static Object lookup(long address) {
     }
 
     private static RuntimeException reportDataAccessError(long address) {
-        RuntimeException location = TRACE_MIRROR_ALLOCATION_SITES ? nativeMirrorInfo.get(address) : null;
-        printDataAccessErrorLocation(location);
+        if (TRACE_MIRROR_ALLOCATION_SITES) {
+            printDataAccessErrorLocation(address);
+        }
         throw RInternalError.shouldNotReachHere("unknown native reference " + address + "L / 0x" + Long.toHexString(address) + " (current id count: " + Long.toHexString(counter.get()) + ")");
     }
 
-    private static void printDataAccessErrorLocation(RuntimeException location) {
+    private static void printDataAccessErrorLocation(long address) {
+        RuntimeException location = nativeMirrorInfo.get(address);
         if (location != null) {
             System.out.println("Location at which the native mirror was allocated:");
             location.printStackTrace();
+        } else {
+            System.out.println("Location at which the native mirror was allocated was not recorded.");
         }
     }
 

com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/ffi/RFFILog.java

@@ -74,6 +74,12 @@ private enum CallMode {
         }
     }
 
+    @TruffleBoundary
+    public static void logRObject(String message, Object obj) {
+        Object mirror = obj instanceof RBaseObject ? mirror = ((RBaseObject) obj).getNativeMirror() : null;
+        log(String.format("%s [%s, native mirror: %s]", message, Utils.getDebugInfo(obj), mirror));
+    }
+
     public static void logUpCall(String name, List<Object> args) {
         logCall(CallMode.UP, name, getContext().getCallDepth(), args.toArray());
     }