From 7b694f74b56e41994aac804fe6741d06aa2642d9 Mon Sep 17 00:00:00 2001 From: Austin Wise Date: Sat, 5 Nov 2022 14:28:44 -0700 Subject: [PATCH 01/14] NativeAOT Objective-C Marshal: object tracker support --- src/coreclr/nativeaot/Bootstrap/main.cpp | 14 ++ src/coreclr/nativeaot/CMakeLists.txt | 1 + .../src/Internal/Runtime/MethodTable.cs | 8 + .../src/System/Runtime/InternalCalls.cs | 3 + src/coreclr/nativeaot/Runtime/CMakeLists.txt | 6 + src/coreclr/nativeaot/Runtime/Crst.h | 1 + src/coreclr/nativeaot/Runtime/GCHelpers.cpp | 8 + src/coreclr/nativeaot/Runtime/ICodeManager.h | 3 + src/coreclr/nativeaot/Runtime/gcrhenv.cpp | 28 ++- .../nativeaot/Runtime/inc/MethodTable.h | 6 + .../nativeaot/Runtime/interoplibinterface.h | 26 +++ .../Runtime/interoplibinterface_objc.cpp | 187 ++++++++++++++++++ src/coreclr/nativeaot/Runtime/startup.cpp | 9 + .../src/System/EETypePtr.cs | 8 + .../ObjectiveCMarshal.NativeAot.cs | 120 ++++++++++- .../src/System/Runtime/RuntimeImports.cs | 12 ++ .../Internal/Runtime/EETypeBuilderHelpers.cs | 22 +++ .../Internal/Runtime/MethodTable.Constants.cs | 1 + .../ObjectiveCMarshalAPI/Program.cs | 2 + 19 files changed, 461 insertions(+), 4 deletions(-) create mode 100644 src/coreclr/nativeaot/Runtime/interoplibinterface.h create mode 100644 src/coreclr/nativeaot/Runtime/interoplibinterface_objc.cpp diff --git a/src/coreclr/nativeaot/Bootstrap/main.cpp b/src/coreclr/nativeaot/Bootstrap/main.cpp index 9f4281cff931ac..c4836677c7693d 100644 --- a/src/coreclr/nativeaot/Bootstrap/main.cpp +++ b/src/coreclr/nativeaot/Bootstrap/main.cpp @@ -112,6 +112,11 @@ extern "C" void OnFirstChanceException(); extern "C" void OnUnhandledException(); extern "C" void IDynamicCastableIsInterfaceImplemented(); extern "C" void IDynamicCastableGetInterfaceImplementation(); +#ifdef FEATURE_OBJCMARSHAL +extern "C" void ObjectiveCMarshalTryGetTaggedMemory(); +extern "C" void ObjectiveCMarshalGetIsTrackedReferenceCallback(); +extern "C" void ObjectiveCMarshalGetOnEnteredFinalizerQueueCallback(); +#endif typedef void(*pfn)(); @@ -126,6 +131,15 @@ static const pfn c_classlibFunctions[] = { &OnUnhandledException, &IDynamicCastableIsInterfaceImplemented, &IDynamicCastableGetInterfaceImplementation, +#ifdef FEATURE_OBJCMARSHAL + &ObjectiveCMarshalTryGetTaggedMemory, + &ObjectiveCMarshalGetIsTrackedReferenceCallback, + &ObjectiveCMarshalGetOnEnteredFinalizerQueueCallback, +#else + nullptr, + nullptr, + nullptr, +#endif }; #ifndef _countof diff --git a/src/coreclr/nativeaot/CMakeLists.txt b/src/coreclr/nativeaot/CMakeLists.txt index 2449ea783eecbc..a593a2f586b76e 100644 --- a/src/coreclr/nativeaot/CMakeLists.txt +++ b/src/coreclr/nativeaot/CMakeLists.txt @@ -34,6 +34,7 @@ if(CLR_CMAKE_HOST_UNIX) if(CLR_CMAKE_TARGET_OSX) add_definitions(-D_XOPEN_SOURCE) + add_definitions(-DFEATURE_OBJCMARSHAL) endif(CLR_CMAKE_TARGET_OSX) if(CLR_CMAKE_TARGET_ARCH_AMD64 OR CLR_CMAKE_TARGET_ARCH_I386) diff --git a/src/coreclr/nativeaot/Common/src/Internal/Runtime/MethodTable.cs b/src/coreclr/nativeaot/Common/src/Internal/Runtime/MethodTable.cs index ae5b43bb04be82..f26e029b88fd5f 100644 --- a/src/coreclr/nativeaot/Common/src/Internal/Runtime/MethodTable.cs +++ b/src/coreclr/nativeaot/Common/src/Internal/Runtime/MethodTable.cs @@ -791,6 +791,14 @@ internal bool IsHFA } } + internal bool IsTrackedReferenceWithFinalizer + { + get + { + return (ExtendedFlags & (ushort)EETypeFlagsEx.IsTrackedReferenceWithFinalizerFlag) != 0; + } + } + internal uint ValueTypeFieldPadding { get diff --git a/src/coreclr/nativeaot/Runtime.Base/src/System/Runtime/InternalCalls.cs b/src/coreclr/nativeaot/Runtime.Base/src/System/Runtime/InternalCalls.cs index 6cb63b9d60d91a..f97a62a9c98f64 100644 --- a/src/coreclr/nativeaot/Runtime.Base/src/System/Runtime/InternalCalls.cs +++ b/src/coreclr/nativeaot/Runtime.Base/src/System/Runtime/InternalCalls.cs @@ -44,6 +44,9 @@ internal enum ClassLibFunctionId OnUnhandledException = 7, IDynamicCastableIsInterfaceImplemented = 8, IDynamicCastableGetInterfaceImplementation = 9, + ObjectiveCMarshalTryGetTaggedMemory = 10, + ObjectiveCMarshalGetIsTrackedReferenceCallback = 11, + ObjectiveCMarshalGetOnEnteredFinalizerQueueCallback = 12, } internal static class InternalCalls diff --git a/src/coreclr/nativeaot/Runtime/CMakeLists.txt b/src/coreclr/nativeaot/Runtime/CMakeLists.txt index 5e49efd8f5fa07..7cca18b0149320 100644 --- a/src/coreclr/nativeaot/Runtime/CMakeLists.txt +++ b/src/coreclr/nativeaot/Runtime/CMakeLists.txt @@ -170,6 +170,12 @@ else() set(ASM_SUFFIX S) endif() +if (CLR_CMAKE_TARGET_OSX) + list(APPEND COMMON_RUNTIME_SOURCES + interoplibinterface_objc.cpp + ) +endif (CLR_CMAKE_TARGET_OSX) + if (CLR_CMAKE_TARGET_ARCH_AMD64 AND CLR_CMAKE_TARGET_WIN32) list(APPEND COMMON_RUNTIME_SOURCES ${GC_DIR}/vxsort/isa_detection.cpp diff --git a/src/coreclr/nativeaot/Runtime/Crst.h b/src/coreclr/nativeaot/Runtime/Crst.h index 658b0186429d02..5f2496eac16490 100644 --- a/src/coreclr/nativeaot/Runtime/Crst.h +++ b/src/coreclr/nativeaot/Runtime/Crst.h @@ -18,6 +18,7 @@ enum CrstType CrstInterfaceDispatchGlobalLists, CrstStressLog, CrstRestrictedCallouts, + CrstObjectiveCMarshalCallouts, CrstGcStressControl, CrstSuspendEE, CrstCastCache, diff --git a/src/coreclr/nativeaot/Runtime/GCHelpers.cpp b/src/coreclr/nativeaot/Runtime/GCHelpers.cpp index 43350c0932d279..6b38d63d5e4b08 100644 --- a/src/coreclr/nativeaot/Runtime/GCHelpers.cpp +++ b/src/coreclr/nativeaot/Runtime/GCHelpers.cpp @@ -18,6 +18,7 @@ #include "varint.h" #include "regdisplay.h" #include "StackFrameIterator.h" +#include "interoplibinterface.h" #include "thread.h" #include "RWLock.h" @@ -132,6 +133,13 @@ COOP_PINVOKE_HELPER(void, RhUnregisterGcCallout, (GcRestrictedCalloutKind eKind, RestrictedCallouts::UnregisterGcCallout(eKind, pCallout); } +#ifdef FEATURE_OBJCMARSHAL +COOP_PINVOKE_HELPER(FC_BOOL_RET, RhRegisterObjectiveCMarshalBeginEndCallback, (void * pCallback)) +{ + FC_RETURN_BOOL(ObjCMarshalNative::RegisterBeginEndCallback(pCallback)); +} +#endif + COOP_PINVOKE_HELPER(int32_t, RhGetLohCompactionMode, ()) { return GCHeapUtilities::GetGCHeap()->GetLOHCompactionMode(); diff --git a/src/coreclr/nativeaot/Runtime/ICodeManager.h b/src/coreclr/nativeaot/Runtime/ICodeManager.h index 30eb4ac6854f21..a5e188f97197be 100644 --- a/src/coreclr/nativeaot/Runtime/ICodeManager.h +++ b/src/coreclr/nativeaot/Runtime/ICodeManager.h @@ -154,6 +154,9 @@ enum class ClasslibFunctionId OnUnhandledException = 7, IDynamicCastableIsInterfaceImplemented = 8, IDynamicCastableGetInterfaceImplementation = 9, + ObjectiveCMarshalTryGetTaggedMemory = 10, + ObjectiveCMarshalGetIsTrackedReferenceCallback = 11, + ObjectiveCMarshalGetOnEnteredFinalizerQueueCallback = 12, }; enum class AssociatedDataFlags : unsigned char diff --git a/src/coreclr/nativeaot/Runtime/gcrhenv.cpp b/src/coreclr/nativeaot/Runtime/gcrhenv.cpp index 56864387123e68..da06cda3ddda82 100644 --- a/src/coreclr/nativeaot/Runtime/gcrhenv.cpp +++ b/src/coreclr/nativeaot/Runtime/gcrhenv.cpp @@ -45,6 +45,7 @@ #include "daccess.h" #include "GCMemoryHelpers.h" +#include "interoplibinterface.h" #include "holder.h" #include "volatile.h" @@ -682,13 +683,25 @@ void GCToEEInterface::GcStartWork(int condemned, int /*max_gen*/) void GCToEEInterface::BeforeGcScanRoots(int condemned, bool is_bgc, bool is_concurrent) { +#ifdef FEATURE_OBJCMARSHAL + if (!is_concurrent) + { + ObjCMarshalNative::BeforeRefCountedHandleCallbacks(); + } +#endif } // EE can perform post stack scanning action, while the user threads are still suspended -void GCToEEInterface::AfterGcScanRoots(int condemned, int /*max_gen*/, ScanContext* /*sc*/) +void GCToEEInterface::AfterGcScanRoots(int condemned, int /*max_gen*/, ScanContext* sc) { // Invoke any registered callouts for the end of the mark phase. RestrictedCallouts::InvokeGcCallouts(GCRC_AfterMarkPhase, condemned); +#ifdef FEATURE_OBJCMARSHAL + if (!sc->concurrent) + { + ObjCMarshalNative::AfterRefCountedHandleCallbacks(); + } +#endif } void GCToEEInterface::GcDone(int condemned) @@ -699,6 +712,11 @@ void GCToEEInterface::GcDone(int condemned) bool GCToEEInterface::RefCountedHandleCallbacks(Object * pObject) { +#ifdef FEATURE_OBJCMARSHAL + bool isReferenced = false; + if (ObjCMarshalNative::IsTrackedReference(pObject, &isReferenced)) + return isReferenced; +#endif // FEATURE_OBJCMARSHAL return RestrictedCallouts::InvokeRefCountedHandleCallbacks(pObject); } @@ -1153,6 +1171,14 @@ void GCToEEInterface::HandleFatalError(unsigned int exitCode) bool GCToEEInterface::EagerFinalized(Object* obj) { +#ifdef FEATURE_OBJCMARSHAL + if (obj->GetGCSafeMethodTable()->IsTrackedReferenceWithFinalizer()) + { + ObjCMarshalNative::OnEnteredFinalizerQueue(obj); + return false; + } +#endif + if (!obj->GetGCSafeMethodTable()->HasEagerFinalizer()) return false; diff --git a/src/coreclr/nativeaot/Runtime/inc/MethodTable.h b/src/coreclr/nativeaot/Runtime/inc/MethodTable.h index 1ba189e54fe51d..69ab2d78b7b8b0 100644 --- a/src/coreclr/nativeaot/Runtime/inc/MethodTable.h +++ b/src/coreclr/nativeaot/Runtime/inc/MethodTable.h @@ -197,6 +197,7 @@ class MethodTable { HasEagerFinalizerFlag = 0x0001, HasCriticalFinalizerFlag = 0x0002, + IsTrackedReferenceWithFinalizerFlag = 0x0004, }; public: @@ -272,6 +273,11 @@ class MethodTable return (m_uFlags & HasCriticalFinalizerFlag) && !HasComponentSize(); } + bool IsTrackedReferenceWithFinalizer() + { + return (m_uFlags & IsTrackedReferenceWithFinalizerFlag) && !HasComponentSize(); + } + bool HasComponentSize() { static_assert(HasComponentSizeFlag == (MethodTable::Flags)(1 << 31), "we assume that HasComponentSizeFlag matches the sign bit"); diff --git a/src/coreclr/nativeaot/Runtime/interoplibinterface.h b/src/coreclr/nativeaot/Runtime/interoplibinterface.h new file mode 100644 index 00000000000000..03307483213259 --- /dev/null +++ b/src/coreclr/nativeaot/Runtime/interoplibinterface.h @@ -0,0 +1,26 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +#ifdef FEATURE_OBJCMARSHAL + +class ObjCMarshalNative +{ +public: + using TryGetCallback = void*(REDHAWK_CALLCONV *)(void); + using TryGetTaggedMemoryCallback = CLR_BOOL(REDHAWK_CALLCONV *)(_In_ Object *, _Out_ void **); + using BeginEndCallback = void(REDHAWK_CALLCONV *)(void); + using IsReferencedCallback = int(REDHAWK_CALLCONV *)(_In_ void*); + using EnteredFinalizationCallback = void(REDHAWK_CALLCONV *)(_In_ void*); + +public: // Instance inspection + static bool IsTrackedReference(_In_ Object * pObject, _Out_ bool* isReferenced); +public: // GC interaction + static void BeforeRefCountedHandleCallbacks(); + static void AfterRefCountedHandleCallbacks(); + static void OnEnteredFinalizerQueue(_In_ Object * object); + + static bool Initialize(); + static bool RegisterBeginEndCallback(void * callback); +}; + +#endif // FEATURE_OBJCMARSHAL diff --git a/src/coreclr/nativeaot/Runtime/interoplibinterface_objc.cpp b/src/coreclr/nativeaot/Runtime/interoplibinterface_objc.cpp new file mode 100644 index 00000000000000..8acd2aa0234dc5 --- /dev/null +++ b/src/coreclr/nativeaot/Runtime/interoplibinterface_objc.cpp @@ -0,0 +1,187 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +#include "common.h" +#include "CommonTypes.h" +#include "CommonMacros.h" +#include "daccess.h" +#include "PalRedhawkCommon.h" +#include "PalRedhawk.h" +#include "rhassert.h" +#include "slist.h" +#include "holder.h" +#include "gcrhinterface.h" +#include "shash.h" +#include "RWLock.h" +#include "rhbinder.h" +#include "Crst.h" +#include "RuntimeInstance.h" +#include "TypeManager.h" +#include "MethodTable.h" +#include "ObjectLayout.h" +#include "event.h" +#include "varint.h" +#include "regdisplay.h" +#include "StackFrameIterator.h" +#include "thread.h" +#include "threadstore.h" +#include "threadstore.inl" + +#include "interoplibinterface.h" + +#include "MethodTable.inl" + +#ifdef FEATURE_OBJCMARSHAL + +namespace +{ + struct GcBeginEndCallbacks + { + ObjCMarshalNative::BeginEndCallback m_pCallbackFunction; + // May be NULL for the end of the linked list + GcBeginEndCallbacks * m_pNext; + }; + + CrstStatic s_sLock; + GcBeginEndCallbacks * m_pBeginEndCallbacks = nullptr; + + void CallBeginEndCallbacks() + { + GcBeginEndCallbacks * pCallback = m_pBeginEndCallbacks; + while (pCallback != nullptr) + { + ObjCMarshalNative::BeginEndCallback fn = pCallback->m_pCallbackFunction; + fn(); + pCallback = pCallback->m_pNext; + } + } + + bool TryGetTaggedMemory(_In_ Object * obj, _Out_ void ** tagged) + { + void* fn = obj->get_EEType() + ->GetTypeManagerPtr() + ->AsTypeManager() + ->GetClasslibFunction(ClasslibFunctionId::ObjectiveCMarshalTryGetTaggedMemory); + + if (fn == nullptr) + return false; + + auto pTryGetTaggedMemoryCallback = reinterpret_cast(fn); + + // It is illegal for any of the callouts to trigger a GC. + Thread * pThread = ThreadStore::GetCurrentThread(); + pThread->SetDoNotTriggerGc(); + + // Due to the above we have better suppress GC stress. + bool fGcStressWasSuppressed = pThread->IsSuppressGcStressSet(); + if (!fGcStressWasSuppressed) + pThread->SetSuppressGcStress(); + + bool result = pTryGetTaggedMemoryCallback(obj, tagged); + + // Revert GC stress mode if we changed it. + if (!fGcStressWasSuppressed) + pThread->ClearSuppressGcStress(); + + pThread->ClearDoNotTriggerGc(); + + return result; + } + + template + T TryGetCallbackViaClasslib(_In_ Object * object, _In_ ClasslibFunctionId id) + { + void* fn = object->get_EEType() + ->GetTypeManagerPtr() + ->AsTypeManager() + ->GetClasslibFunction(id); + + if (fn != nullptr) + { + fn = ((ObjCMarshalNative::TryGetCallback)fn)(); + } + + return (T)fn; + } +} + +// One time startup initialization. +bool ObjCMarshalNative::Initialize() +{ + s_sLock.Init(CrstObjectiveCMarshalCallouts, CRST_DEFAULT); + + return true; +} + +bool ObjCMarshalNative::RegisterBeginEndCallback(void * callback) +{ + _ASSERTE(callback != nullptr); + + // We must be in Cooperative mode since we are setting callbacks that + // will be used during a GC and we want to ensure a GC isn't occurring + // while they are being set. + _ASSERTE(ThreadStore::GetCurrentThread()->IsCurrentThreadInCooperativeMode()); + + GcBeginEndCallbacks * pCallback = new (nothrow) GcBeginEndCallbacks(); + if (pCallback == NULL) + return false; + + pCallback->m_pCallbackFunction = (ObjCMarshalNative::BeginEndCallback)callback; + + CrstHolder lh(&s_sLock); + + pCallback->m_pNext = m_pBeginEndCallbacks; + m_pBeginEndCallbacks = pCallback; + + return true; +} + +bool ObjCMarshalNative::IsTrackedReference(_In_ Object * object, _Out_ bool* isReferenced) +{ + *isReferenced = false; + + if (!object->GetGCSafeMethodTable()->IsTrackedReferenceWithFinalizer()) + return false; + + auto pIsReferencedCallbackCallback = TryGetCallbackViaClasslib( + object, ClasslibFunctionId::ObjectiveCMarshalGetIsTrackedReferenceCallback); + + if (pIsReferencedCallbackCallback == nullptr) + return false; + + void* taggedMemory; + if (!TryGetTaggedMemory(object, &taggedMemory)) + return false; + + int result = pIsReferencedCallbackCallback(taggedMemory); + + *isReferenced = (result != 0); + return true; +} + +void ObjCMarshalNative::BeforeRefCountedHandleCallbacks() +{ + CallBeginEndCallbacks(); +} + +void ObjCMarshalNative::AfterRefCountedHandleCallbacks() +{ + CallBeginEndCallbacks(); +} + +void ObjCMarshalNative::OnEnteredFinalizerQueue(_In_ Object * object) +{ + auto pOnEnteredFinalizerQueueCallback = TryGetCallbackViaClasslib( + object, ClasslibFunctionId::ObjectiveCMarshalGetOnEnteredFinalizerQueueCallback); + + if (pOnEnteredFinalizerQueueCallback == nullptr) + return; + + void* taggedMemory; + if (!TryGetTaggedMemory(object, &taggedMemory)) + return; + + pOnEnteredFinalizerQueueCallback(taggedMemory); +} + +#endif // FEATURE_OBJCMARSHAL diff --git a/src/coreclr/nativeaot/Runtime/startup.cpp b/src/coreclr/nativeaot/Runtime/startup.cpp index e2b4395da7d8c0..da7a78601bd1a9 100644 --- a/src/coreclr/nativeaot/Runtime/startup.cpp +++ b/src/coreclr/nativeaot/Runtime/startup.cpp @@ -26,6 +26,7 @@ #include "stressLog.h" #include "RestrictedCallouts.h" #include "yieldprocessornormalized.h" +#include "interoplibinterface.h" #ifndef DACCESS_COMPILE @@ -104,6 +105,14 @@ static bool InitDLL(HANDLE hPalInstance) if (!RestrictedCallouts::Initialize()) return false; +#ifdef FEATURE_OBJCMARSHAL + // + // Intialize support for registing GC callbacks for Objective-C Marshal. + // + if (!ObjCMarshalNative::Initialize()) + return false; +#endif + // // Initialize RuntimeInstance state // diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/EETypePtr.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/EETypePtr.cs index 4e2335759056b6..cd6055fdd55c81 100644 --- a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/EETypePtr.cs +++ b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/EETypePtr.cs @@ -277,6 +277,14 @@ internal bool HasCctor } } + internal bool IsTrackedReferenceWithFinalizer + { + get + { + return _value->IsTrackedReferenceWithFinalizer; + } + } + internal EETypePtr NullableType { get diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/ObjectiveCMarshal.NativeAot.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/ObjectiveCMarshal.NativeAot.cs index a7ab0249acbc47..add3038eb4a897 100644 --- a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/ObjectiveCMarshal.NativeAot.cs +++ b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/ObjectiveCMarshal.NativeAot.cs @@ -1,8 +1,8 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -using System; using System.Diagnostics; +using System.Runtime.CompilerServices; using System.Runtime.ExceptionServices; using System.Threading; @@ -11,6 +11,10 @@ namespace System.Runtime.InteropServices.ObjectiveC public static unsafe partial class ObjectiveCMarshal { private static readonly IntPtr[] s_ObjcMessageSendFunctions = new IntPtr[(int)MessageSendFunction.MsgSendSuperStret + 1]; + private static int s_initialized; + private static readonly ConditionalWeakTable s_objects = new(); + private static IntPtr s_IsTrackedReferenceCallback; + private static IntPtr s_OnEnteredFinalizerQueueCallback; [ThreadStatic] private static Exception? t_pendingExceptionObject; @@ -51,12 +55,55 @@ internal static void ThrowPendingExceptionObject() } } + [RuntimeExport("ObjectiveCMarshalTryGetTaggedMemory")] + static bool TryGetTaggedMemory(IntPtr pObj, IntPtr* tagged) + { + // We are paused in the GC, so this is safe. + object obj = Unsafe.AsRef((void*)&pObj); + + if (!s_objects.TryGetValue(obj, out ObjcTrackingInformation? info)) + { + return false; + } + + *tagged = info._memory; + return true; + } + + [RuntimeExport("ObjectiveCMarshalGetIsTrackedReferenceCallback")] + static IntPtr GetIsTrackedReferenceCallback() + { + return s_IsTrackedReferenceCallback; + } + + [RuntimeExport("ObjectiveCMarshalGetOnEnteredFinalizerQueueCallback")] + static IntPtr GetOnEnteredFinalizerQueueCallback() + { + return s_OnEnteredFinalizerQueueCallback; + } + private static bool TryInitializeReferenceTracker( delegate* unmanaged beginEndCallback, delegate* unmanaged isReferencedCallback, delegate* unmanaged trackedObjectEnteredFinalization) { - throw new NotImplementedException(); + if (Interlocked.CompareExchange(ref s_initialized, 1, 0) != 0) + { + return false; + } + + if (!RuntimeImports.RhRegisterObjectiveCMarshalBeginEndCallback((IntPtr)beginEndCallback)) + { + // We failed to allocate unmanaged memory, so no state has been changed. + // Reset s_initialized so initialization can be attempted again. + s_initialized = 0; + throw new OutOfMemoryException(); + } + + s_IsTrackedReferenceCallback = (IntPtr)isReferencedCallback; + s_OnEnteredFinalizerQueueCallback = (IntPtr)trackedObjectEnteredFinalization; + + return true; } private static IntPtr CreateReferenceTrackingHandleInternal( @@ -64,7 +111,74 @@ private static IntPtr CreateReferenceTrackingHandleInternal( out int memInSizeT, out IntPtr mem) { - throw new NotImplementedException(); + if (s_initialized == 0) + { + throw new InvalidOperationException(SR.InvalidOperation_ObjectiveCMarshalNotInitialized); + } + + if (!obj.GetEETypePtr().IsTrackedReferenceWithFinalizer) + { + throw new InvalidOperationException(SR.InvalidOperation_ObjectiveCTypeNoFinalizer); + } + + var trackerInfo = s_objects.GetValue(obj, static o => new ObjcTrackingInformation()); + trackerInfo.EnsureInitialized(obj); + trackerInfo.GetTaggedMemory(out memInSizeT, out mem); + return RuntimeImports.RhHandleAllocRefCounted(obj); + } + + class ObjcTrackingInformation + { + const int TAGGED_MEMORY_SIZE_IN_POINTERS = 2; + + internal IntPtr _memory; + IntPtr _longWeakHandle; + + public ObjcTrackingInformation() + { + _memory = (IntPtr)NativeMemory.AllocZeroed(TAGGED_MEMORY_SIZE_IN_POINTERS * (nuint)IntPtr.Size); + } + + public void GetTaggedMemory(out int memInSizeT, out IntPtr mem) + { + memInSizeT = TAGGED_MEMORY_SIZE_IN_POINTERS; + mem = _memory; + } + + public void EnsureInitialized(object o) + { + // RuntimeImports.RhHandleAlloc + if (_longWeakHandle != IntPtr.Zero) + { + return; + } + + IntPtr newHandle = RuntimeImports.RhHandleAlloc(o, GCHandleType.WeakTrackResurrection); + if (Interlocked.CompareExchange(ref _longWeakHandle, newHandle, IntPtr.Zero) != IntPtr.Zero) + { + RuntimeImports.RhHandleFree(newHandle); + } + } + + ~ObjcTrackingInformation() + { + if (_longWeakHandle != IntPtr.Zero && RuntimeImports.RhHandleGet(_longWeakHandle) != null) + { + GC.ReRegisterForFinalize(this); + return; + } + + if (_memory != IntPtr.Zero) + { + NativeMemory.Free((void*)_memory); + _memory = IntPtr.Zero; + } + if (_longWeakHandle != IntPtr.Zero) + { + RuntimeImports.RhHandleFree(_longWeakHandle); + _longWeakHandle = IntPtr.Zero; + } + } } } } diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/RuntimeImports.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/RuntimeImports.cs index b5f6a17156e55f..869d46e43c05ad 100644 --- a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/RuntimeImports.cs +++ b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/RuntimeImports.cs @@ -242,6 +242,12 @@ internal static IntPtr RhHandleAlloc(object value, GCHandleType type) return h; } + internal static IntPtr RhHandleAllocRefCounted(object value) + { + const int HNDTYPE_REFCOUNTED = 5; + return RhHandleAlloc(value, (GCHandleType)HNDTYPE_REFCOUNTED); + } + // Allocate handle for dependent handle case where a secondary can be set at the same time. [MethodImpl(MethodImplOptions.InternalCall)] [RuntimeImport(RuntimeLibrary, "RhpHandleAllocDependent")] @@ -507,6 +513,12 @@ internal enum GcRestrictedCalloutKind [RuntimeImport(RuntimeLibrary, "RhUnregisterRefCountedHandleCallback")] internal static extern void RhUnregisterRefCountedHandleCallback(IntPtr pCalloutMethod, EETypePtr pTypeFilter); +#if FEATURE_OBJCMARSHAL + [MethodImplAttribute(MethodImplOptions.InternalCall)] + [RuntimeImport(RuntimeLibrary, "RhRegisterObjectiveCMarshalBeginEndCallback")] + internal static extern bool RhRegisterObjectiveCMarshalBeginEndCallback(IntPtr pCalloutMethod); +#endif + // // Blob support // diff --git a/src/coreclr/tools/Common/Internal/Runtime/EETypeBuilderHelpers.cs b/src/coreclr/tools/Common/Internal/Runtime/EETypeBuilderHelpers.cs index fe7fe586dc074b..f7b9ac94e44fbc 100644 --- a/src/coreclr/tools/Common/Internal/Runtime/EETypeBuilderHelpers.cs +++ b/src/coreclr/tools/Common/Internal/Runtime/EETypeBuilderHelpers.cs @@ -113,6 +113,11 @@ mdType.Name is "WeakReference" or "WeakReference`1" && flagsEx |= (ushort)EETypeFlagsEx.HasCriticalFinalizerFlag; } + if (type.Context.Target.IsOSX && IsTrackedReferenceWithFinalizer(type)) + { + flagsEx |= (ushort)EETypeFlagsEx.IsTrackedReferenceWithFinalizerFlag; + } + return flagsEx; } @@ -136,6 +141,23 @@ private static bool HasCriticalFinalizer(TypeDesc type) return false; } + private static bool IsTrackedReferenceWithFinalizer(TypeDesc type) + { + do + { + if (!type.HasFinalizer) + return false; + + if (type is MetadataType mdType && mdType.HasCustomAttribute("System.Runtime.InteropServices.ObjectiveC", "ObjectiveCTrackedTypeAttribute")) + return true; + + type = type.BaseType; + } + while (type != null); + + return false; + } + // These masks and paddings have been chosen so that the ValueTypePadding field can always fit in a byte of data // if the alignment is 8 bytes or less. If the alignment is higher then there may be a need for more bits to hold // the rest of the padding data. diff --git a/src/coreclr/tools/Common/Internal/Runtime/MethodTable.Constants.cs b/src/coreclr/tools/Common/Internal/Runtime/MethodTable.Constants.cs index 0b1db87b84118a..568c56744c88d9 100644 --- a/src/coreclr/tools/Common/Internal/Runtime/MethodTable.Constants.cs +++ b/src/coreclr/tools/Common/Internal/Runtime/MethodTable.Constants.cs @@ -88,6 +88,7 @@ internal enum EETypeFlagsEx : ushort { HasEagerFinalizerFlag = 0x0001, HasCriticalFinalizerFlag = 0x0002, + IsTrackedReferenceWithFinalizerFlag = 0x0004, } internal enum EETypeKind : uint diff --git a/src/tests/Interop/ObjectiveC/ObjectiveCMarshalAPI/Program.cs b/src/tests/Interop/ObjectiveC/ObjectiveCMarshalAPI/Program.cs index 8322f049e93f13..ebb089dc147f2a 100644 --- a/src/tests/Interop/ObjectiveC/ObjectiveCMarshalAPI/Program.cs +++ b/src/tests/Interop/ObjectiveC/ObjectiveCMarshalAPI/Program.cs @@ -285,6 +285,8 @@ class Scenario // Do not call this method from Main as it depends on a previous test for set up. static void _Validate_ExceptionPropagation() { + Console.WriteLine($"Running {nameof(_Validate_ExceptionPropagation)}"); + var delThrowInt = new ThrowExceptionDelegate(DEL_ThrowIntException); var delThrowException = new ThrowExceptionDelegate(DEL_ThrowExceptionException); var scenarios = new[] From a1ded48d8800cf4140350bbd98f0a52c1d3cd7ed Mon Sep 17 00:00:00 2001 From: Austin Wise Date: Sun, 13 Nov 2022 18:29:02 -0800 Subject: [PATCH 02/14] Centralize logic for turning on Objective-C Marshal --- .../src/System.Private.TypeLoader.csproj | 3 +++ .../Internal/Runtime/EETypeBuilderHelpers.cs | 3 ++- .../Common/Internal/Runtime/TargetFeatures.cs | 15 +++++++++++++++ .../TypeSystem/Interop/IL/MarshalHelpers.cs | 5 +++-- .../ILCompiler.Compiler.csproj | 3 +++ .../ILCompiler.ReadyToRun.csproj | 1 + .../ILCompiler.TypeSystem.Tests.csproj | 1 + 7 files changed, 28 insertions(+), 3 deletions(-) create mode 100644 src/coreclr/tools/Common/Internal/Runtime/TargetFeatures.cs diff --git a/src/coreclr/nativeaot/System.Private.TypeLoader/src/System.Private.TypeLoader.csproj b/src/coreclr/nativeaot/System.Private.TypeLoader/src/System.Private.TypeLoader.csproj index fea0a58061282c..eff68a8a488d4a 100644 --- a/src/coreclr/nativeaot/System.Private.TypeLoader/src/System.Private.TypeLoader.csproj +++ b/src/coreclr/nativeaot/System.Private.TypeLoader/src/System.Private.TypeLoader.csproj @@ -47,6 +47,9 @@ MetadataBlob.cs + + TargetFeatures.cs + ArrayBuilder.cs diff --git a/src/coreclr/tools/Common/Internal/Runtime/EETypeBuilderHelpers.cs b/src/coreclr/tools/Common/Internal/Runtime/EETypeBuilderHelpers.cs index f7b9ac94e44fbc..67a76584354b9d 100644 --- a/src/coreclr/tools/Common/Internal/Runtime/EETypeBuilderHelpers.cs +++ b/src/coreclr/tools/Common/Internal/Runtime/EETypeBuilderHelpers.cs @@ -4,6 +4,7 @@ using System.Diagnostics; using Internal.TypeSystem; +using Internal.Runtime; namespace Internal.Runtime { @@ -113,7 +114,7 @@ mdType.Name is "WeakReference" or "WeakReference`1" && flagsEx |= (ushort)EETypeFlagsEx.HasCriticalFinalizerFlag; } - if (type.Context.Target.IsOSX && IsTrackedReferenceWithFinalizer(type)) + if (TargetFeatures.TargetSupportsObjectiveCMarshal(type.Context.Target) && IsTrackedReferenceWithFinalizer(type)) { flagsEx |= (ushort)EETypeFlagsEx.IsTrackedReferenceWithFinalizerFlag; } diff --git a/src/coreclr/tools/Common/Internal/Runtime/TargetFeatures.cs b/src/coreclr/tools/Common/Internal/Runtime/TargetFeatures.cs new file mode 100644 index 00000000000000..18dcf211c6bd96 --- /dev/null +++ b/src/coreclr/tools/Common/Internal/Runtime/TargetFeatures.cs @@ -0,0 +1,15 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using Internal.TypeSystem; + +namespace Internal.Runtime +{ + internal static class TargetFeatures + { + internal static bool TargetSupportsObjectiveCMarshal(TargetDetails target) + { + return target.IsOSX; + } + } +} diff --git a/src/coreclr/tools/Common/TypeSystem/Interop/IL/MarshalHelpers.cs b/src/coreclr/tools/Common/TypeSystem/Interop/IL/MarshalHelpers.cs index 91a9834603764b..4fa7b8b7004d3d 100644 --- a/src/coreclr/tools/Common/TypeSystem/Interop/IL/MarshalHelpers.cs +++ b/src/coreclr/tools/Common/TypeSystem/Interop/IL/MarshalHelpers.cs @@ -5,6 +5,7 @@ using Debug = System.Diagnostics.Debug; using System.Runtime.InteropServices.ObjectiveC; using Internal.TypeSystem.Ecma; +using Internal.Runtime; namespace Internal.TypeSystem.Interop { @@ -925,7 +926,7 @@ internal static MarshallerKind GetDisabledMarshallerKind( internal static bool ShouldCheckForPendingException(TargetDetails target, PInvokeMetadata metadata) { - if (!target.IsOSX) + if (!TargetFeatures.TargetSupportsObjectiveCMarshal(target)) return false; const string ObjectiveCMsgSend = "objc_msgSend"; @@ -942,7 +943,7 @@ internal static bool ShouldCheckForPendingException(TargetDetails target, PInvok internal static int? GetObjectiveCMessageSendFunction(TargetDetails target, string pinvokeModule, string pinvokeFunction) { - if (!target.IsOSX || pinvokeModule != ObjectiveCLibrary) + if (!TargetFeatures.TargetSupportsObjectiveCMarshal(target) || pinvokeModule != ObjectiveCLibrary) return null; #pragma warning disable CA1416 diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/ILCompiler.Compiler.csproj b/src/coreclr/tools/aot/ILCompiler.Compiler/ILCompiler.Compiler.csproj index 5887e5d4ff2a7c..12d05ce0e32a5c 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/ILCompiler.Compiler.csproj +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/ILCompiler.Compiler.csproj @@ -204,6 +204,9 @@ Common\RuntimeConstants.cs + + Common\TargetFeatures.cs + Common\UniversalGenericParameterLayout.cs diff --git a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/ILCompiler.ReadyToRun.csproj b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/ILCompiler.ReadyToRun.csproj index 4d8b7c38dda4e5..37c71907c9fa58 100644 --- a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/ILCompiler.ReadyToRun.csproj +++ b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/ILCompiler.ReadyToRun.csproj @@ -35,6 +35,7 @@ + diff --git a/src/coreclr/tools/aot/ILCompiler.TypeSystem.Tests/ILCompiler.TypeSystem.Tests.csproj b/src/coreclr/tools/aot/ILCompiler.TypeSystem.Tests/ILCompiler.TypeSystem.Tests.csproj index 1f6b33ff18ba39..dbc469e4276b95 100644 --- a/src/coreclr/tools/aot/ILCompiler.TypeSystem.Tests/ILCompiler.TypeSystem.Tests.csproj +++ b/src/coreclr/tools/aot/ILCompiler.TypeSystem.Tests/ILCompiler.TypeSystem.Tests.csproj @@ -39,6 +39,7 @@ + From 0d88691fe2d417e423713a8242e05be507f77c59 Mon Sep 17 00:00:00 2001 From: Austin Wise Date: Mon, 21 Nov 2022 21:30:10 -0800 Subject: [PATCH 03/14] Remove stray comment --- .../Runtime/InteropServices/ObjectiveCMarshal.NativeAot.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/ObjectiveCMarshal.NativeAot.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/ObjectiveCMarshal.NativeAot.cs index add3038eb4a897..533af0fe98d40c 100644 --- a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/ObjectiveCMarshal.NativeAot.cs +++ b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/ObjectiveCMarshal.NativeAot.cs @@ -147,7 +147,6 @@ public void GetTaggedMemory(out int memInSizeT, out IntPtr mem) public void EnsureInitialized(object o) { - // RuntimeImports.RhHandleAlloc if (_longWeakHandle != IntPtr.Zero) { return; From 6f677ad8326f1b148a07433d818f7eddf8c0787c Mon Sep 17 00:00:00 2001 From: Austin Wise Date: Mon, 21 Nov 2022 21:30:32 -0800 Subject: [PATCH 04/14] Revert "Centralize logic for turning on Objective-C Marshal" This reverts commit 2abfe8d394b899810e955d79af0b9303f82c448d. --- .../src/System.Private.TypeLoader.csproj | 3 --- .../Internal/Runtime/EETypeBuilderHelpers.cs | 3 +-- .../Common/Internal/Runtime/TargetFeatures.cs | 15 --------------- .../TypeSystem/Interop/IL/MarshalHelpers.cs | 5 ++--- .../ILCompiler.Compiler.csproj | 3 --- .../ILCompiler.ReadyToRun.csproj | 1 - .../ILCompiler.TypeSystem.Tests.csproj | 1 - 7 files changed, 3 insertions(+), 28 deletions(-) delete mode 100644 src/coreclr/tools/Common/Internal/Runtime/TargetFeatures.cs diff --git a/src/coreclr/nativeaot/System.Private.TypeLoader/src/System.Private.TypeLoader.csproj b/src/coreclr/nativeaot/System.Private.TypeLoader/src/System.Private.TypeLoader.csproj index eff68a8a488d4a..fea0a58061282c 100644 --- a/src/coreclr/nativeaot/System.Private.TypeLoader/src/System.Private.TypeLoader.csproj +++ b/src/coreclr/nativeaot/System.Private.TypeLoader/src/System.Private.TypeLoader.csproj @@ -47,9 +47,6 @@ MetadataBlob.cs - - TargetFeatures.cs - ArrayBuilder.cs diff --git a/src/coreclr/tools/Common/Internal/Runtime/EETypeBuilderHelpers.cs b/src/coreclr/tools/Common/Internal/Runtime/EETypeBuilderHelpers.cs index 67a76584354b9d..f7b9ac94e44fbc 100644 --- a/src/coreclr/tools/Common/Internal/Runtime/EETypeBuilderHelpers.cs +++ b/src/coreclr/tools/Common/Internal/Runtime/EETypeBuilderHelpers.cs @@ -4,7 +4,6 @@ using System.Diagnostics; using Internal.TypeSystem; -using Internal.Runtime; namespace Internal.Runtime { @@ -114,7 +113,7 @@ mdType.Name is "WeakReference" or "WeakReference`1" && flagsEx |= (ushort)EETypeFlagsEx.HasCriticalFinalizerFlag; } - if (TargetFeatures.TargetSupportsObjectiveCMarshal(type.Context.Target) && IsTrackedReferenceWithFinalizer(type)) + if (type.Context.Target.IsOSX && IsTrackedReferenceWithFinalizer(type)) { flagsEx |= (ushort)EETypeFlagsEx.IsTrackedReferenceWithFinalizerFlag; } diff --git a/src/coreclr/tools/Common/Internal/Runtime/TargetFeatures.cs b/src/coreclr/tools/Common/Internal/Runtime/TargetFeatures.cs deleted file mode 100644 index 18dcf211c6bd96..00000000000000 --- a/src/coreclr/tools/Common/Internal/Runtime/TargetFeatures.cs +++ /dev/null @@ -1,15 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -using Internal.TypeSystem; - -namespace Internal.Runtime -{ - internal static class TargetFeatures - { - internal static bool TargetSupportsObjectiveCMarshal(TargetDetails target) - { - return target.IsOSX; - } - } -} diff --git a/src/coreclr/tools/Common/TypeSystem/Interop/IL/MarshalHelpers.cs b/src/coreclr/tools/Common/TypeSystem/Interop/IL/MarshalHelpers.cs index 4fa7b8b7004d3d..91a9834603764b 100644 --- a/src/coreclr/tools/Common/TypeSystem/Interop/IL/MarshalHelpers.cs +++ b/src/coreclr/tools/Common/TypeSystem/Interop/IL/MarshalHelpers.cs @@ -5,7 +5,6 @@ using Debug = System.Diagnostics.Debug; using System.Runtime.InteropServices.ObjectiveC; using Internal.TypeSystem.Ecma; -using Internal.Runtime; namespace Internal.TypeSystem.Interop { @@ -926,7 +925,7 @@ internal static MarshallerKind GetDisabledMarshallerKind( internal static bool ShouldCheckForPendingException(TargetDetails target, PInvokeMetadata metadata) { - if (!TargetFeatures.TargetSupportsObjectiveCMarshal(target)) + if (!target.IsOSX) return false; const string ObjectiveCMsgSend = "objc_msgSend"; @@ -943,7 +942,7 @@ internal static bool ShouldCheckForPendingException(TargetDetails target, PInvok internal static int? GetObjectiveCMessageSendFunction(TargetDetails target, string pinvokeModule, string pinvokeFunction) { - if (!TargetFeatures.TargetSupportsObjectiveCMarshal(target) || pinvokeModule != ObjectiveCLibrary) + if (!target.IsOSX || pinvokeModule != ObjectiveCLibrary) return null; #pragma warning disable CA1416 diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/ILCompiler.Compiler.csproj b/src/coreclr/tools/aot/ILCompiler.Compiler/ILCompiler.Compiler.csproj index 12d05ce0e32a5c..5887e5d4ff2a7c 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/ILCompiler.Compiler.csproj +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/ILCompiler.Compiler.csproj @@ -204,9 +204,6 @@ Common\RuntimeConstants.cs - - Common\TargetFeatures.cs - Common\UniversalGenericParameterLayout.cs diff --git a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/ILCompiler.ReadyToRun.csproj b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/ILCompiler.ReadyToRun.csproj index 37c71907c9fa58..4d8b7c38dda4e5 100644 --- a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/ILCompiler.ReadyToRun.csproj +++ b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/ILCompiler.ReadyToRun.csproj @@ -35,7 +35,6 @@ - diff --git a/src/coreclr/tools/aot/ILCompiler.TypeSystem.Tests/ILCompiler.TypeSystem.Tests.csproj b/src/coreclr/tools/aot/ILCompiler.TypeSystem.Tests/ILCompiler.TypeSystem.Tests.csproj index dbc469e4276b95..1f6b33ff18ba39 100644 --- a/src/coreclr/tools/aot/ILCompiler.TypeSystem.Tests/ILCompiler.TypeSystem.Tests.csproj +++ b/src/coreclr/tools/aot/ILCompiler.TypeSystem.Tests/ILCompiler.TypeSystem.Tests.csproj @@ -39,7 +39,6 @@ - From 0dd4e621f85b66be49a81b8cbe2667bac2a04905 Mon Sep 17 00:00:00 2001 From: Austin Wise Date: Tue, 22 Nov 2022 20:42:24 -0800 Subject: [PATCH 05/14] Respond to feedback, plus add missing GC trigger disable Specifically: * Remove support for registering multiple begin/end callbacks, since the API was not designed with multiple managed worlds in mind. * Use more asserts where it should not be possible for there to be no callback. * Add more comments. * Add missing GC-trigger disable when calling managed callouts. --- .../nativeaot/Runtime/interoplibinterface.h | 4 +- .../Runtime/interoplibinterface_objc.cpp | 144 +++++++++--------- src/coreclr/nativeaot/Runtime/startup.cpp | 8 - .../ObjectiveCMarshal.NativeAot.cs | 5 +- 4 files changed, 71 insertions(+), 90 deletions(-) diff --git a/src/coreclr/nativeaot/Runtime/interoplibinterface.h b/src/coreclr/nativeaot/Runtime/interoplibinterface.h index 03307483213259..da57618b3f7896 100644 --- a/src/coreclr/nativeaot/Runtime/interoplibinterface.h +++ b/src/coreclr/nativeaot/Runtime/interoplibinterface.h @@ -15,12 +15,10 @@ class ObjCMarshalNative public: // Instance inspection static bool IsTrackedReference(_In_ Object * pObject, _Out_ bool* isReferenced); public: // GC interaction + static bool RegisterBeginEndCallback(void * callback); static void BeforeRefCountedHandleCallbacks(); static void AfterRefCountedHandleCallbacks(); static void OnEnteredFinalizerQueue(_In_ Object * object); - - static bool Initialize(); - static bool RegisterBeginEndCallback(void * callback); }; #endif // FEATURE_OBJCMARSHAL diff --git a/src/coreclr/nativeaot/Runtime/interoplibinterface_objc.cpp b/src/coreclr/nativeaot/Runtime/interoplibinterface_objc.cpp index 8acd2aa0234dc5..07bfc9980ef7a7 100644 --- a/src/coreclr/nativeaot/Runtime/interoplibinterface_objc.cpp +++ b/src/coreclr/nativeaot/Runtime/interoplibinterface_objc.cpp @@ -35,105 +35,91 @@ namespace { - struct GcBeginEndCallbacks + // TODO: Support registering multiple begin/end callbacks. + // NativeAOT support the concept of multiple managed "worlds", + // each with their own object heirarchy. Each of these worlds + // could potentially have its own set of callbacks. + // However this Objective-C Marshal API was not designed with such a + // possibility in mind. + ObjCMarshalNative::BeginEndCallback s_pBeginEndCallback = nullptr; + + struct DisableTriggerGcWhileCallingManagedCode { - ObjCMarshalNative::BeginEndCallback m_pCallbackFunction; - // May be NULL for the end of the linked list - GcBeginEndCallbacks * m_pNext; - }; + Thread * m_pThread; + bool m_fGcStressWasSuppressed; - CrstStatic s_sLock; - GcBeginEndCallbacks * m_pBeginEndCallbacks = nullptr; + DisableTriggerGcWhileCallingManagedCode() + { + // It is illegal for any of the callouts to trigger a GC. + m_pThread = ThreadStore::GetCurrentThread(); + m_pThread->SetDoNotTriggerGc(); + + // Due to the above we have better suppress GC stress. + m_fGcStressWasSuppressed = m_pThread->IsSuppressGcStressSet(); + if (!m_fGcStressWasSuppressed) + m_pThread->SetSuppressGcStress(); + } - void CallBeginEndCallbacks() - { - GcBeginEndCallbacks * pCallback = m_pBeginEndCallbacks; - while (pCallback != nullptr) + ~DisableTriggerGcWhileCallingManagedCode() { - ObjCMarshalNative::BeginEndCallback fn = pCallback->m_pCallbackFunction; - fn(); - pCallback = pCallback->m_pNext; + // Revert GC stress mode if we changed it. + if (!m_fGcStressWasSuppressed) + m_pThread->ClearSuppressGcStress(); + + m_pThread->ClearDoNotTriggerGc(); } - } + }; bool TryGetTaggedMemory(_In_ Object * obj, _Out_ void ** tagged) { void* fn = obj->get_EEType() - ->GetTypeManagerPtr() - ->AsTypeManager() - ->GetClasslibFunction(ClasslibFunctionId::ObjectiveCMarshalTryGetTaggedMemory); - - if (fn == nullptr) - return false; - - auto pTryGetTaggedMemoryCallback = reinterpret_cast(fn); + ->GetTypeManagerPtr() + ->AsTypeManager() + ->GetClasslibFunction(ClasslibFunctionId::ObjectiveCMarshalTryGetTaggedMemory); + + ASSERT(fn != nullptr); - // It is illegal for any of the callouts to trigger a GC. - Thread * pThread = ThreadStore::GetCurrentThread(); - pThread->SetDoNotTriggerGc(); + auto pTryGetTaggedMemoryCallback = reinterpret_cast(fn); - // Due to the above we have better suppress GC stress. - bool fGcStressWasSuppressed = pThread->IsSuppressGcStressSet(); - if (!fGcStressWasSuppressed) - pThread->SetSuppressGcStress(); + DisableTriggerGcWhileCallingManagedCode disabler{}; bool result = pTryGetTaggedMemoryCallback(obj, tagged); - // Revert GC stress mode if we changed it. - if (!fGcStressWasSuppressed) - pThread->ClearSuppressGcStress(); - - pThread->ClearDoNotTriggerGc(); - return result; } + // Calls a managed classlib function to potentially get an unmanaged callback. + // Not for use with ObjectiveCMarshalTryGetTaggedMemory. template - T TryGetCallbackViaClasslib(_In_ Object * object, _In_ ClasslibFunctionId id) + T GetCallbackViaClasslibCallback(_In_ Object * object, _In_ ClasslibFunctionId id) { void* fn = object->get_EEType() ->GetTypeManagerPtr() ->AsTypeManager() ->GetClasslibFunction(id); - if (fn != nullptr) - { - fn = ((ObjCMarshalNative::TryGetCallback)fn)(); - } + ASSERT(fn != nullptr); - return (T)fn; - } -} + DisableTriggerGcWhileCallingManagedCode disabler{}; -// One time startup initialization. -bool ObjCMarshalNative::Initialize() -{ - s_sLock.Init(CrstObjectiveCMarshalCallouts, CRST_DEFAULT); + fn = ((ObjCMarshalNative::TryGetCallback)fn)(); - return true; + ASSERT(fn != nullptr); + + return (T)fn; + } } bool ObjCMarshalNative::RegisterBeginEndCallback(void * callback) { - _ASSERTE(callback != nullptr); + ASSERT(callback != nullptr); - // We must be in Cooperative mode since we are setting callbacks that - // will be used during a GC and we want to ensure a GC isn't occurring - // while they are being set. - _ASSERTE(ThreadStore::GetCurrentThread()->IsCurrentThreadInCooperativeMode()); - - GcBeginEndCallbacks * pCallback = new (nothrow) GcBeginEndCallbacks(); - if (pCallback == NULL) - return false; - - pCallback->m_pCallbackFunction = (ObjCMarshalNative::BeginEndCallback)callback; - - CrstHolder lh(&s_sLock); + // We must be in Cooperative mode since we are setting callbacks that + // will be used during a GC and we want to ensure a GC isn't occurring + // while they are being set. + ASSERT(ThreadStore::GetCurrentThread()->IsCurrentThreadInCooperativeMode()); - pCallback->m_pNext = m_pBeginEndCallbacks; - m_pBeginEndCallbacks = pCallback; - - return true; + return PalInterlockedCompareExchangePointer(&s_pBeginEndCallback, callback, nullptr) == nullptr; } bool ObjCMarshalNative::IsTrackedReference(_In_ Object * object, _Out_ bool* isReferenced) @@ -143,15 +129,17 @@ bool ObjCMarshalNative::IsTrackedReference(_In_ Object * object, _Out_ bool* isR if (!object->GetGCSafeMethodTable()->IsTrackedReferenceWithFinalizer()) return false; - auto pIsReferencedCallbackCallback = TryGetCallbackViaClasslib( + auto pIsReferencedCallbackCallback = GetCallbackViaClasslibCallback( object, ClasslibFunctionId::ObjectiveCMarshalGetIsTrackedReferenceCallback); - if (pIsReferencedCallbackCallback == nullptr) - return false; - void* taggedMemory; if (!TryGetTaggedMemory(object, &taggedMemory)) + { + // It should not be possible to create a ref-counted handle + // without setting up the tagged memory first. + ASSERT(false); return false; + } int result = pIsReferencedCallbackCallback(taggedMemory); @@ -161,25 +149,29 @@ bool ObjCMarshalNative::IsTrackedReference(_In_ Object * object, _Out_ bool* isR void ObjCMarshalNative::BeforeRefCountedHandleCallbacks() { - CallBeginEndCallbacks(); + if (s_pBeginEndCallback != nullptr) + s_pBeginEndCallback(); } void ObjCMarshalNative::AfterRefCountedHandleCallbacks() { - CallBeginEndCallbacks(); + if (s_pBeginEndCallback != nullptr) + s_pBeginEndCallback(); } void ObjCMarshalNative::OnEnteredFinalizerQueue(_In_ Object * object) { - auto pOnEnteredFinalizerQueueCallback = TryGetCallbackViaClasslib( + auto pOnEnteredFinalizerQueueCallback = GetCallbackViaClasslibCallback( object, ClasslibFunctionId::ObjectiveCMarshalGetOnEnteredFinalizerQueueCallback); - if (pOnEnteredFinalizerQueueCallback == nullptr) - return; - void* taggedMemory; if (!TryGetTaggedMemory(object, &taggedMemory)) + { + // Its possible to create an object that supports reference tracking + // without ever creating a ref-counted handle for it. In this case, + // there will be no tagged memory. return; + } pOnEnteredFinalizerQueueCallback(taggedMemory); } diff --git a/src/coreclr/nativeaot/Runtime/startup.cpp b/src/coreclr/nativeaot/Runtime/startup.cpp index da7a78601bd1a9..e97b59c746dd62 100644 --- a/src/coreclr/nativeaot/Runtime/startup.cpp +++ b/src/coreclr/nativeaot/Runtime/startup.cpp @@ -105,14 +105,6 @@ static bool InitDLL(HANDLE hPalInstance) if (!RestrictedCallouts::Initialize()) return false; -#ifdef FEATURE_OBJCMARSHAL - // - // Intialize support for registing GC callbacks for Objective-C Marshal. - // - if (!ObjCMarshalNative::Initialize()) - return false; -#endif - // // Initialize RuntimeInstance state // diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/ObjectiveCMarshal.NativeAot.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/ObjectiveCMarshal.NativeAot.cs index 533af0fe98d40c..f5df2c939d2bb1 100644 --- a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/ObjectiveCMarshal.NativeAot.cs +++ b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/ObjectiveCMarshal.NativeAot.cs @@ -94,10 +94,9 @@ private static bool TryInitializeReferenceTracker( if (!RuntimeImports.RhRegisterObjectiveCMarshalBeginEndCallback((IntPtr)beginEndCallback)) { - // We failed to allocate unmanaged memory, so no state has been changed. - // Reset s_initialized so initialization can be attempted again. + // Prevent the creation of tracking handles by resetting the initialized state. s_initialized = 0; - throw new OutOfMemoryException(); + return false; } s_IsTrackedReferenceCallback = (IntPtr)isReferencedCallback; From 3da32c9beaa96c1935c811954f3cf7e080a2c497 Mon Sep 17 00:00:00 2001 From: Austin Wise Date: Tue, 22 Nov 2022 20:49:50 -0800 Subject: [PATCH 06/14] Remove some extraneous spaces and unneeded changes --- src/coreclr/nativeaot/Runtime/interoplibinterface_objc.cpp | 6 +++--- src/coreclr/nativeaot/Runtime/startup.cpp | 1 - 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/coreclr/nativeaot/Runtime/interoplibinterface_objc.cpp b/src/coreclr/nativeaot/Runtime/interoplibinterface_objc.cpp index 07bfc9980ef7a7..b20a7c7efd4b06 100644 --- a/src/coreclr/nativeaot/Runtime/interoplibinterface_objc.cpp +++ b/src/coreclr/nativeaot/Runtime/interoplibinterface_objc.cpp @@ -76,7 +76,7 @@ namespace ->GetTypeManagerPtr() ->AsTypeManager() ->GetClasslibFunction(ClasslibFunctionId::ObjectiveCMarshalTryGetTaggedMemory); - + ASSERT(fn != nullptr); auto pTryGetTaggedMemoryCallback = reinterpret_cast(fn); @@ -128,7 +128,7 @@ bool ObjCMarshalNative::IsTrackedReference(_In_ Object * object, _Out_ bool* isR if (!object->GetGCSafeMethodTable()->IsTrackedReferenceWithFinalizer()) return false; - + auto pIsReferencedCallbackCallback = GetCallbackViaClasslibCallback( object, ClasslibFunctionId::ObjectiveCMarshalGetIsTrackedReferenceCallback); @@ -140,7 +140,7 @@ bool ObjCMarshalNative::IsTrackedReference(_In_ Object * object, _Out_ bool* isR ASSERT(false); return false; } - + int result = pIsReferencedCallbackCallback(taggedMemory); *isReferenced = (result != 0); diff --git a/src/coreclr/nativeaot/Runtime/startup.cpp b/src/coreclr/nativeaot/Runtime/startup.cpp index e97b59c746dd62..e2b4395da7d8c0 100644 --- a/src/coreclr/nativeaot/Runtime/startup.cpp +++ b/src/coreclr/nativeaot/Runtime/startup.cpp @@ -26,7 +26,6 @@ #include "stressLog.h" #include "RestrictedCallouts.h" #include "yieldprocessornormalized.h" -#include "interoplibinterface.h" #ifndef DACCESS_COMPILE From 2ec8884df2a01605ef4ed8c7601a8c719a757fb6 Mon Sep 17 00:00:00 2001 From: Austin Wise Date: Tue, 22 Nov 2022 20:56:27 -0800 Subject: [PATCH 07/14] Update src/coreclr/tools/Common/Internal/Runtime/EETypeBuilderHelpers.cs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Michal Strehovský --- .../tools/Common/Internal/Runtime/EETypeBuilderHelpers.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/tools/Common/Internal/Runtime/EETypeBuilderHelpers.cs b/src/coreclr/tools/Common/Internal/Runtime/EETypeBuilderHelpers.cs index f7b9ac94e44fbc..6ff1658955d43a 100644 --- a/src/coreclr/tools/Common/Internal/Runtime/EETypeBuilderHelpers.cs +++ b/src/coreclr/tools/Common/Internal/Runtime/EETypeBuilderHelpers.cs @@ -148,7 +148,7 @@ private static bool IsTrackedReferenceWithFinalizer(TypeDesc type) if (!type.HasFinalizer) return false; - if (type is MetadataType mdType && mdType.HasCustomAttribute("System.Runtime.InteropServices.ObjectiveC", "ObjectiveCTrackedTypeAttribute")) + if ((MetadataType)type).HasCustomAttribute("System.Runtime.InteropServices.ObjectiveC", "ObjectiveCTrackedTypeAttribute")) return true; type = type.BaseType; From 6a06e1b8665ba16a533d261f4953efd0f1b0c3a3 Mon Sep 17 00:00:00 2001 From: Austin Wise Date: Tue, 29 Nov 2022 23:17:03 +0000 Subject: [PATCH 08/14] Fix build --- .../tools/Common/Internal/Runtime/EETypeBuilderHelpers.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/tools/Common/Internal/Runtime/EETypeBuilderHelpers.cs b/src/coreclr/tools/Common/Internal/Runtime/EETypeBuilderHelpers.cs index 6ff1658955d43a..5f45795ec49598 100644 --- a/src/coreclr/tools/Common/Internal/Runtime/EETypeBuilderHelpers.cs +++ b/src/coreclr/tools/Common/Internal/Runtime/EETypeBuilderHelpers.cs @@ -148,7 +148,7 @@ private static bool IsTrackedReferenceWithFinalizer(TypeDesc type) if (!type.HasFinalizer) return false; - if ((MetadataType)type).HasCustomAttribute("System.Runtime.InteropServices.ObjectiveC", "ObjectiveCTrackedTypeAttribute")) + if (((MetadataType)type).HasCustomAttribute("System.Runtime.InteropServices.ObjectiveC", "ObjectiveCTrackedTypeAttribute")) return true; type = type.BaseType; From b1eb66d5508e912d725e71d4fa2b52515d7ce8f3 Mon Sep 17 00:00:00 2001 From: Austin Wise Date: Sat, 3 Dec 2022 21:50:01 -0800 Subject: [PATCH 09/14] Simplify initialization logic --- .../InteropServices/ObjectiveCMarshal.NativeAot.cs | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/ObjectiveCMarshal.NativeAot.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/ObjectiveCMarshal.NativeAot.cs index f5df2c939d2bb1..b324135b3425d3 100644 --- a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/ObjectiveCMarshal.NativeAot.cs +++ b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/ObjectiveCMarshal.NativeAot.cs @@ -11,7 +11,7 @@ namespace System.Runtime.InteropServices.ObjectiveC public static unsafe partial class ObjectiveCMarshal { private static readonly IntPtr[] s_ObjcMessageSendFunctions = new IntPtr[(int)MessageSendFunction.MsgSendSuperStret + 1]; - private static int s_initialized; + private static bool s_initialized; private static readonly ConditionalWeakTable s_objects = new(); private static IntPtr s_IsTrackedReferenceCallback; private static IntPtr s_OnEnteredFinalizerQueueCallback; @@ -87,20 +87,14 @@ private static bool TryInitializeReferenceTracker( delegate* unmanaged isReferencedCallback, delegate* unmanaged trackedObjectEnteredFinalization) { - if (Interlocked.CompareExchange(ref s_initialized, 1, 0) != 0) - { - return false; - } - if (!RuntimeImports.RhRegisterObjectiveCMarshalBeginEndCallback((IntPtr)beginEndCallback)) { - // Prevent the creation of tracking handles by resetting the initialized state. - s_initialized = 0; return false; } s_IsTrackedReferenceCallback = (IntPtr)isReferencedCallback; s_OnEnteredFinalizerQueueCallback = (IntPtr)trackedObjectEnteredFinalization; + s_initialized = true; return true; } @@ -110,7 +104,7 @@ private static IntPtr CreateReferenceTrackingHandleInternal( out int memInSizeT, out IntPtr mem) { - if (s_initialized == 0) + if (!s_initialized) { throw new InvalidOperationException(SR.InvalidOperation_ObjectiveCMarshalNotInitialized); } From a7a6a9bc5d521ccd54c5332306f0cda7edfc5aec Mon Sep 17 00:00:00 2001 From: Austin Wise Date: Sun, 4 Dec 2022 16:08:57 -0800 Subject: [PATCH 10/14] Respond to feedback * Add comments to CoreCLR and NativeAOT about constants that should have the same size. * Remove superfluous use of C++ templates. * Use the C# unmanaged function pointer types in more places, improving type safety --- .../nativeaot/Runtime/interoplibinterface_objc.cpp | 9 ++++----- .../InteropServices/ObjectiveCMarshal.NativeAot.cs | 14 ++++++++------ src/coreclr/vm/syncblk.h | 4 ++++ 3 files changed, 16 insertions(+), 11 deletions(-) diff --git a/src/coreclr/nativeaot/Runtime/interoplibinterface_objc.cpp b/src/coreclr/nativeaot/Runtime/interoplibinterface_objc.cpp index b20a7c7efd4b06..5c737082302a65 100644 --- a/src/coreclr/nativeaot/Runtime/interoplibinterface_objc.cpp +++ b/src/coreclr/nativeaot/Runtime/interoplibinterface_objc.cpp @@ -90,8 +90,7 @@ namespace // Calls a managed classlib function to potentially get an unmanaged callback. // Not for use with ObjectiveCMarshalTryGetTaggedMemory. - template - T GetCallbackViaClasslibCallback(_In_ Object * object, _In_ ClasslibFunctionId id) + void * GetCallbackViaClasslibCallback(_In_ Object * object, _In_ ClasslibFunctionId id) { void* fn = object->get_EEType() ->GetTypeManagerPtr() @@ -106,7 +105,7 @@ namespace ASSERT(fn != nullptr); - return (T)fn; + return fn; } } @@ -129,7 +128,7 @@ bool ObjCMarshalNative::IsTrackedReference(_In_ Object * object, _Out_ bool* isR if (!object->GetGCSafeMethodTable()->IsTrackedReferenceWithFinalizer()) return false; - auto pIsReferencedCallbackCallback = GetCallbackViaClasslibCallback( + auto pIsReferencedCallbackCallback = (ObjCMarshalNative::IsReferencedCallback)GetCallbackViaClasslibCallback( object, ClasslibFunctionId::ObjectiveCMarshalGetIsTrackedReferenceCallback); void* taggedMemory; @@ -161,7 +160,7 @@ void ObjCMarshalNative::AfterRefCountedHandleCallbacks() void ObjCMarshalNative::OnEnteredFinalizerQueue(_In_ Object * object) { - auto pOnEnteredFinalizerQueueCallback = GetCallbackViaClasslibCallback( + auto pOnEnteredFinalizerQueueCallback = (ObjCMarshalNative::EnteredFinalizationCallback)GetCallbackViaClasslibCallback( object, ClasslibFunctionId::ObjectiveCMarshalGetOnEnteredFinalizerQueueCallback); void* taggedMemory; diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/ObjectiveCMarshal.NativeAot.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/ObjectiveCMarshal.NativeAot.cs index b324135b3425d3..94debcc4ceca95 100644 --- a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/ObjectiveCMarshal.NativeAot.cs +++ b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/ObjectiveCMarshal.NativeAot.cs @@ -13,8 +13,8 @@ public static unsafe partial class ObjectiveCMarshal private static readonly IntPtr[] s_ObjcMessageSendFunctions = new IntPtr[(int)MessageSendFunction.MsgSendSuperStret + 1]; private static bool s_initialized; private static readonly ConditionalWeakTable s_objects = new(); - private static IntPtr s_IsTrackedReferenceCallback; - private static IntPtr s_OnEnteredFinalizerQueueCallback; + private static delegate* unmanaged s_IsTrackedReferenceCallback; + private static delegate* unmanaged s_OnEnteredFinalizerQueueCallback; [ThreadStatic] private static Exception? t_pendingExceptionObject; @@ -71,13 +71,13 @@ static bool TryGetTaggedMemory(IntPtr pObj, IntPtr* tagged) } [RuntimeExport("ObjectiveCMarshalGetIsTrackedReferenceCallback")] - static IntPtr GetIsTrackedReferenceCallback() + static delegate* unmanaged GetIsTrackedReferenceCallback() { return s_IsTrackedReferenceCallback; } [RuntimeExport("ObjectiveCMarshalGetOnEnteredFinalizerQueueCallback")] - static IntPtr GetOnEnteredFinalizerQueueCallback() + static delegate* unmanaged GetOnEnteredFinalizerQueueCallback() { return s_OnEnteredFinalizerQueueCallback; } @@ -92,8 +92,8 @@ private static bool TryInitializeReferenceTracker( return false; } - s_IsTrackedReferenceCallback = (IntPtr)isReferencedCallback; - s_OnEnteredFinalizerQueueCallback = (IntPtr)trackedObjectEnteredFinalization; + s_IsTrackedReferenceCallback = isReferencedCallback; + s_OnEnteredFinalizerQueueCallback = trackedObjectEnteredFinalization; s_initialized = true; return true; @@ -122,6 +122,8 @@ private static IntPtr CreateReferenceTrackingHandleInternal( class ObjcTrackingInformation { + // This matches the CoreCLR implementation. See + // InteropSyncBlockInfo::m_taggedAlloc in syncblk.h . const int TAGGED_MEMORY_SIZE_IN_POINTERS = 2; internal IntPtr _memory; diff --git a/src/coreclr/vm/syncblk.h b/src/coreclr/vm/syncblk.h index a9008132ad4951..32ad8c522076d0 100644 --- a/src/coreclr/vm/syncblk.h +++ b/src/coreclr/vm/syncblk.h @@ -957,6 +957,10 @@ class InteropSyncBlockInfo // Two pointers worth of bytes of the requirement for // the current consuming implementation so that is what // is being allocated. + // If the size of this array is changed, the NativeAOT version + // should be updated as well. + // See the TAGGED_MEMORY_SIZE_IN_POINTERS constant in + // ObjectiveCMarshal.NativeAot.cs BYTE m_taggedAlloc[2 * sizeof(void*)]; #endif // FEATURE_OBJCMARSHAL }; From 7cbf935e0079aba4fa486daa8bb00511087e23e3 Mon Sep 17 00:00:00 2001 From: Austin Wise Date: Sun, 4 Dec 2022 16:25:19 -0800 Subject: [PATCH 11/14] Apply suggestions from code review Co-authored-by: Aaron Robinson --- .../InteropServices/ObjectiveCMarshal.NativeAot.cs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/ObjectiveCMarshal.NativeAot.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/ObjectiveCMarshal.NativeAot.cs index 94debcc4ceca95..b79d0849ed545b 100644 --- a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/ObjectiveCMarshal.NativeAot.cs +++ b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/ObjectiveCMarshal.NativeAot.cs @@ -13,8 +13,8 @@ public static unsafe partial class ObjectiveCMarshal private static readonly IntPtr[] s_ObjcMessageSendFunctions = new IntPtr[(int)MessageSendFunction.MsgSendSuperStret + 1]; private static bool s_initialized; private static readonly ConditionalWeakTable s_objects = new(); - private static delegate* unmanaged s_IsTrackedReferenceCallback; - private static delegate* unmanaged s_OnEnteredFinalizerQueueCallback; + private static delegate* unmanaged s_IsTrackedReferenceCallback; + private static delegate* unmanaged s_OnEnteredFinalizerQueueCallback; [ThreadStatic] private static Exception? t_pendingExceptionObject; @@ -71,21 +71,21 @@ static bool TryGetTaggedMemory(IntPtr pObj, IntPtr* tagged) } [RuntimeExport("ObjectiveCMarshalGetIsTrackedReferenceCallback")] - static delegate* unmanaged GetIsTrackedReferenceCallback() + static delegate* unmanaged GetIsTrackedReferenceCallback() { return s_IsTrackedReferenceCallback; } [RuntimeExport("ObjectiveCMarshalGetOnEnteredFinalizerQueueCallback")] - static delegate* unmanaged GetOnEnteredFinalizerQueueCallback() + static delegate* unmanaged GetOnEnteredFinalizerQueueCallback() { return s_OnEnteredFinalizerQueueCallback; } private static bool TryInitializeReferenceTracker( delegate* unmanaged beginEndCallback, - delegate* unmanaged isReferencedCallback, - delegate* unmanaged trackedObjectEnteredFinalization) + delegate* unmanaged isReferencedCallback, + delegate* unmanaged trackedObjectEnteredFinalization) { if (!RuntimeImports.RhRegisterObjectiveCMarshalBeginEndCallback((IntPtr)beginEndCallback)) { From 916cf0881b11c651d5fc23fe446f34317abdae6b Mon Sep 17 00:00:00 2001 From: Austin Wise Date: Sun, 4 Dec 2022 16:33:31 -0800 Subject: [PATCH 12/14] Revert "Apply suggestions from code review" This reverts commit 7cbf935e0079aba4fa486daa8bb00511087e23e3. --- .../InteropServices/ObjectiveCMarshal.NativeAot.cs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/ObjectiveCMarshal.NativeAot.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/ObjectiveCMarshal.NativeAot.cs index b79d0849ed545b..94debcc4ceca95 100644 --- a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/ObjectiveCMarshal.NativeAot.cs +++ b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/ObjectiveCMarshal.NativeAot.cs @@ -13,8 +13,8 @@ public static unsafe partial class ObjectiveCMarshal private static readonly IntPtr[] s_ObjcMessageSendFunctions = new IntPtr[(int)MessageSendFunction.MsgSendSuperStret + 1]; private static bool s_initialized; private static readonly ConditionalWeakTable s_objects = new(); - private static delegate* unmanaged s_IsTrackedReferenceCallback; - private static delegate* unmanaged s_OnEnteredFinalizerQueueCallback; + private static delegate* unmanaged s_IsTrackedReferenceCallback; + private static delegate* unmanaged s_OnEnteredFinalizerQueueCallback; [ThreadStatic] private static Exception? t_pendingExceptionObject; @@ -71,21 +71,21 @@ static bool TryGetTaggedMemory(IntPtr pObj, IntPtr* tagged) } [RuntimeExport("ObjectiveCMarshalGetIsTrackedReferenceCallback")] - static delegate* unmanaged GetIsTrackedReferenceCallback() + static delegate* unmanaged GetIsTrackedReferenceCallback() { return s_IsTrackedReferenceCallback; } [RuntimeExport("ObjectiveCMarshalGetOnEnteredFinalizerQueueCallback")] - static delegate* unmanaged GetOnEnteredFinalizerQueueCallback() + static delegate* unmanaged GetOnEnteredFinalizerQueueCallback() { return s_OnEnteredFinalizerQueueCallback; } private static bool TryInitializeReferenceTracker( delegate* unmanaged beginEndCallback, - delegate* unmanaged isReferencedCallback, - delegate* unmanaged trackedObjectEnteredFinalization) + delegate* unmanaged isReferencedCallback, + delegate* unmanaged trackedObjectEnteredFinalization) { if (!RuntimeImports.RhRegisterObjectiveCMarshalBeginEndCallback((IntPtr)beginEndCallback)) { From 1ec0fa9c4ad03f5b34f8262121e0eaa5698c299d Mon Sep 17 00:00:00 2001 From: Austin Wise Date: Sun, 4 Dec 2022 22:56:24 -0800 Subject: [PATCH 13/14] Revert "Revert "Apply suggestions from code review"" This reverts commit 916cf0881b11c651d5fc23fe446f34317abdae6b. --- .../InteropServices/ObjectiveCMarshal.NativeAot.cs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/ObjectiveCMarshal.NativeAot.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/ObjectiveCMarshal.NativeAot.cs index 94debcc4ceca95..b79d0849ed545b 100644 --- a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/ObjectiveCMarshal.NativeAot.cs +++ b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/ObjectiveCMarshal.NativeAot.cs @@ -13,8 +13,8 @@ public static unsafe partial class ObjectiveCMarshal private static readonly IntPtr[] s_ObjcMessageSendFunctions = new IntPtr[(int)MessageSendFunction.MsgSendSuperStret + 1]; private static bool s_initialized; private static readonly ConditionalWeakTable s_objects = new(); - private static delegate* unmanaged s_IsTrackedReferenceCallback; - private static delegate* unmanaged s_OnEnteredFinalizerQueueCallback; + private static delegate* unmanaged s_IsTrackedReferenceCallback; + private static delegate* unmanaged s_OnEnteredFinalizerQueueCallback; [ThreadStatic] private static Exception? t_pendingExceptionObject; @@ -71,21 +71,21 @@ static bool TryGetTaggedMemory(IntPtr pObj, IntPtr* tagged) } [RuntimeExport("ObjectiveCMarshalGetIsTrackedReferenceCallback")] - static delegate* unmanaged GetIsTrackedReferenceCallback() + static delegate* unmanaged GetIsTrackedReferenceCallback() { return s_IsTrackedReferenceCallback; } [RuntimeExport("ObjectiveCMarshalGetOnEnteredFinalizerQueueCallback")] - static delegate* unmanaged GetOnEnteredFinalizerQueueCallback() + static delegate* unmanaged GetOnEnteredFinalizerQueueCallback() { return s_OnEnteredFinalizerQueueCallback; } private static bool TryInitializeReferenceTracker( delegate* unmanaged beginEndCallback, - delegate* unmanaged isReferencedCallback, - delegate* unmanaged trackedObjectEnteredFinalization) + delegate* unmanaged isReferencedCallback, + delegate* unmanaged trackedObjectEnteredFinalization) { if (!RuntimeImports.RhRegisterObjectiveCMarshalBeginEndCallback((IntPtr)beginEndCallback)) { From 9163e9533555ed87dca12ffd983a3e958f9796f2 Mon Sep 17 00:00:00 2001 From: Austin Wise Date: Sun, 4 Dec 2022 23:03:34 -0800 Subject: [PATCH 14/14] Add SuppressGCTransition and fix compile error by adding cast --- .../ObjectiveCMarshal.NativeAot.cs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/ObjectiveCMarshal.NativeAot.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/ObjectiveCMarshal.NativeAot.cs index b79d0849ed545b..831edffbd1a354 100644 --- a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/ObjectiveCMarshal.NativeAot.cs +++ b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/ObjectiveCMarshal.NativeAot.cs @@ -13,8 +13,8 @@ public static unsafe partial class ObjectiveCMarshal private static readonly IntPtr[] s_ObjcMessageSendFunctions = new IntPtr[(int)MessageSendFunction.MsgSendSuperStret + 1]; private static bool s_initialized; private static readonly ConditionalWeakTable s_objects = new(); - private static delegate* unmanaged s_IsTrackedReferenceCallback; - private static delegate* unmanaged s_OnEnteredFinalizerQueueCallback; + private static delegate* unmanaged[SuppressGCTransition] s_IsTrackedReferenceCallback; + private static delegate* unmanaged[SuppressGCTransition] s_OnEnteredFinalizerQueueCallback; [ThreadStatic] private static Exception? t_pendingExceptionObject; @@ -71,29 +71,29 @@ static bool TryGetTaggedMemory(IntPtr pObj, IntPtr* tagged) } [RuntimeExport("ObjectiveCMarshalGetIsTrackedReferenceCallback")] - static delegate* unmanaged GetIsTrackedReferenceCallback() + static delegate* unmanaged[SuppressGCTransition] GetIsTrackedReferenceCallback() { return s_IsTrackedReferenceCallback; } [RuntimeExport("ObjectiveCMarshalGetOnEnteredFinalizerQueueCallback")] - static delegate* unmanaged GetOnEnteredFinalizerQueueCallback() + static delegate* unmanaged[SuppressGCTransition] GetOnEnteredFinalizerQueueCallback() { return s_OnEnteredFinalizerQueueCallback; } private static bool TryInitializeReferenceTracker( delegate* unmanaged beginEndCallback, - delegate* unmanaged isReferencedCallback, - delegate* unmanaged trackedObjectEnteredFinalization) + delegate* unmanaged isReferencedCallback, + delegate* unmanaged trackedObjectEnteredFinalization) { if (!RuntimeImports.RhRegisterObjectiveCMarshalBeginEndCallback((IntPtr)beginEndCallback)) { return false; } - s_IsTrackedReferenceCallback = isReferencedCallback; - s_OnEnteredFinalizerQueueCallback = trackedObjectEnteredFinalization; + s_IsTrackedReferenceCallback = (delegate* unmanaged[SuppressGCTransition])isReferencedCallback; + s_OnEnteredFinalizerQueueCallback = (delegate* unmanaged[SuppressGCTransition])trackedObjectEnteredFinalization; s_initialized = true; return true;