From 167cb79e566c5f4e3b555931848e2a0ac4da89e1 Mon Sep 17 00:00:00 2001 From: stepan Date: Mon, 11 Nov 2019 14:54:22 +0100 Subject: [PATCH] Fix: protect Rf_unprotect'ed until next GNU-R compatible GC cycle (cherry picked from commit 15a89d5ffa301506437b0bba3f6d091a9ddfaa7d) --- .../r/ffi/impl/nodes/ReleaseObjectNode.java | 3 +++ .../r/ffi/impl/nodes/UnprotectNode.java | 25 ++++++++++++++++--- .../r/ffi/impl/nodes/UnprotectPtrNode.java | 12 ++++++++- .../r/runtime/data/NativeDataAccess.java | 10 +++++--- .../oracle/truffle/r/runtime/ffi/RFFILog.java | 6 +++++ 5 files changed, 49 insertions(+), 7 deletions(-) diff --git a/com.oracle.truffle.r.ffi.impl/src/com/oracle/truffle/r/ffi/impl/nodes/ReleaseObjectNode.java b/com.oracle.truffle.r.ffi.impl/src/com/oracle/truffle/r/ffi/impl/nodes/ReleaseObjectNode.java index 1664893a23..4701761919 100644 --- a/com.oracle.truffle.r.ffi.impl/src/com/oracle/truffle/r/ffi/impl/nodes/ReleaseObjectNode.java +++ b/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 { diff --git a/com.oracle.truffle.r.ffi.impl/src/com/oracle/truffle/r/ffi/impl/nodes/UnprotectNode.java b/com.oracle.truffle.r.ffi.impl/src/com/oracle/truffle/r/ffi/impl/nodes/UnprotectNode.java index 672271f5ad..e0aef0f4b1 100644 --- a/com.oracle.truffle.r.ffi.impl/src/com/oracle/truffle/r/ffi/impl/nodes/UnprotectNode.java +++ b/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 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 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,12 +91,14 @@ Object unprotectMultipleCached(@SuppressWarnings("unused") int n, @Specialization(guards = "n > 1", replaces = "unprotectMultipleCached") Object unprotectMultipleUnchached(int n, + @Cached BranchProfile registerNativeRefProfile, @CachedContext(TruffleRLanguage.class) ContextReference 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)"); @@ -97,6 +106,16 @@ Object unprotectMultipleUnchached(int n, 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); diff --git a/com.oracle.truffle.r.ffi.impl/src/com/oracle/truffle/r/ffi/impl/nodes/UnprotectPtrNode.java b/com.oracle.truffle.r.ffi.impl/src/com/oracle/truffle/r/ffi/impl/nodes/UnprotectPtrNode.java index 502e04f57b..3229e04750 100644 --- a/com.oracle.truffle.r.ffi.impl/src/com/oracle/truffle/r/ffi/impl/nodes/UnprotectPtrNode.java +++ b/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 ctxRef) { RFFIContext ctx = ctxRef.get().getStateRFFI(); Collections.ArrayListObj 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; } } diff --git a/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/data/NativeDataAccess.java b/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/data/NativeDataAccess.java index c7c47940b1..e34841b096 100644 --- a/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/data/NativeDataAccess.java +++ b/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."); } } diff --git a/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/ffi/RFFILog.java b/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/ffi/RFFILog.java index ef0d843706..c7e540c440 100644 --- a/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/ffi/RFFILog.java +++ b/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 args) { logCall(CallMode.UP, name, getContext().getCallDepth(), args.toArray()); }