Make DependentHandle public (#54246)

* Move DependentHandle to System.Runtime

* Update DependentHandle APIs to follow review

* Make DependentHandle type public

* Update DependentHandle on Mono runtime

* Add allocation checks to DependentHandle APIs

This avoids throwing ExecutionEngineException-s if one of the public APIs is called on a non-allocated DependentHandle instance

* Add more unit tests for new public DependentHandle APIs

* Add faster, unsafe internal APIs versions to DependentHandle

* Naming improvements to Ephemeron type

The ephemeron type is checked in the Mono runtime in "object.c" as follows:

m_class_get_image (klass) == mono_defaults.corlib && !strcmp ("Ephemeron", m_class_get_name (klass))

As such, the namespace it belongs to in the managed runtime doesn't matter: the VM will just check
that the type name matches, and that the type is in fact defined in corelib. This means we can
just move it to System.Runtime without worrying about it being properly managed in the VM.
Additionally, the type is defined in "sgen-mono.c" as follows:

typedef struct {
    GCObject* key;
    GCObject* value;
} Ephemeron;

So as long as the layout matches the one of the type defined in C# (which it does), we're also free
to rename the fields to better follow the naming guidelines, and the VM will have no issues with it.

* Code style tweaks, improved nullability annotations

* Remove incorrect DependentHandle comment on Mono

* Add default Dispose test for DependentHandle

Co-authored-by: Stephen Toub <stoub@microsoft.com>

* Fix race condition in DependentHandle on Mono

* Optimize DependentHandle.nGetPrimary on CoreCLR

Removed internal call, same optimization as GCHandle

* Small IL codegen improvement in DependentHandle.nGetPrimary

* Simplify comments, add #ifdef for using directive

* Minor code style tweaks

* Change nGetPrimaryAndSecondary to nGetSecondary

* Minor code refactoring to DependentHandle on Mono

* Rename DependentHandle FCalls

* Remove DependentHandle.UnsafeGetTargetAndDependent

* Remove DependentHandle.GetTargetAndDependent

* Fix FCall path for internal DependentHandle APIs

* Add more DependentHandle unit tests

* Reintroduce DependentHandle.GetTargetAndDependent()

This fixes a bug due to a race condition in ConditionalWeakTable<K, V>, which relies on this method which atomically retrieves both target and dependent with respect to target being set to null concurrently by other threads. This also exposes the same API publically to allow consumers to potentially implement custom conditional weak tables in the same manner.

* Minor IL tweaks to produce smaller IR in the JIT

* Add DependentHandle.StopTracking() API

This also fixes two potential GC holes when setting DependentHandle.Target (see conversation from https://github.com/dotnet/runtime/pull/54246#issuecomment-863285327 onwards)

* Rename InternalSetTarget to StopTracking, remove redundant param

* Remove FCUnique from InternalStopTracking

This was added in https://github.com/dotnet/runtime/pull/39810 to avoid a collision with MarshalNative::GCHandleInternalSet, as the two FCalls had identical implementations and their entry points were not unique. This should no longer be needed after 099fc478551f46cc54e7a18a32d9a9ac73727c73, as that changed both the signature and the implementation of this FCall.

* Update API surface to match approved specs from API review

* Update DependentHandle XML docs

Co-authored-by: Stephen Toub <stoub@microsoft.com>

3 files changed, 62 insertions, 55 deletions

diff --git a/src/coreclr/vm/comdependenthandle.cpp b/src/coreclr/vm/comdependenthandle.cpp
index e261f16064a..b2221908102 100644
--- a/src/coreclr/vm/comdependenthandle.cpp
+++ b/src/coreclr/vm/comdependenthandle.cpp
@@ -14,20 +14,18 @@
 #include "common.h"
 #include "comdependenthandle.h"
 
-
-
-FCIMPL2(OBJECTHANDLE, DependentHandle::nInitialize, Object *_primary, Object *_secondary)
+FCIMPL2(OBJECTHANDLE, DependentHandle::InternalInitialize, Object *_target, Object *_dependent)
 {
     FCALL_CONTRACT;
 
-    OBJECTREF primary(_primary);
-    OBJECTREF secondary(_secondary);
+    OBJECTREF target(_target);
+    OBJECTREF dependent(_dependent);
     OBJECTHANDLE result = NULL;
 
     HELPER_METHOD_FRAME_BEGIN_RET_NOPOLL();
 
     // Create the handle.
-    result = GetAppDomain()->CreateDependentHandle(primary, secondary);
+    result = GetAppDomain()->CreateDependentHandle(target, dependent);
 
     HELPER_METHOD_FRAME_END_POLL();
 
@@ -35,72 +33,80 @@ FCIMPL2(OBJECTHANDLE, DependentHandle::nInitialize, Object *_primary, Object *_s
 }
 FCIMPLEND
 
+FCIMPL1(Object*, DependentHandle::InternalGetTarget, OBJECTHANDLE handle)
+{
+    FCALL_CONTRACT;
+    FCUnique(0x54);
 
+    _ASSERTE(handle != NULL);
+
+    return OBJECTREFToObject(ObjectFromHandle(handle));
+}
+FCIMPLEND
 
-FCIMPL1(VOID, DependentHandle::nFree, OBJECTHANDLE handle)
+FCIMPL1(Object*, DependentHandle::InternalGetDependent, OBJECTHANDLE handle)
 {
     FCALL_CONTRACT;
 
     _ASSERTE(handle != NULL);
 
-    HELPER_METHOD_FRAME_BEGIN_0();
+    OBJECTREF target = ObjectFromHandle(handle);
 
-    DestroyDependentHandle(handle);
-
-    HELPER_METHOD_FRAME_END();
+    IGCHandleManager *mgr = GCHandleUtilities::GetGCHandleManager();
 
+    // The dependent is tracked only if target is non-null
+    return (target != NULL) ? mgr->GetDependentHandleSecondary(handle) : NULL;
 }
 FCIMPLEND
 
-
-
-FCIMPL1(Object*, DependentHandle::nGetPrimary, OBJECTHANDLE handle)
+FCIMPL2(Object*, DependentHandle::InternalGetTargetAndDependent, OBJECTHANDLE handle, Object **outDependent)
 {
     FCALL_CONTRACT;
-    FCUnique(0x54);
-    _ASSERTE(handle != NULL);
-    return OBJECTREFToObject(ObjectFromHandle(handle));
-}
-FCIMPLEND
 
+    _ASSERTE(handle != NULL && outDependent != NULL);
 
+    OBJECTREF target = ObjectFromHandle(handle);
+    IGCHandleManager *mgr = GCHandleUtilities::GetGCHandleManager();
+
+    // The dependent is tracked only if target is non-null
+    *outDependent = (target != NULL) ? mgr->GetDependentHandleSecondary(handle) : NULL;
+
+    return OBJECTREFToObject(target);
+}
+FCIMPLEND
 
-FCIMPL2(Object*, DependentHandle::nGetPrimaryAndSecondary, OBJECTHANDLE handle, Object **outSecondary)
+FCIMPL1(VOID, DependentHandle::InternalSetTargetToNull, OBJECTHANDLE handle)
 {
     FCALL_CONTRACT;
-    _ASSERTE(handle != NULL && outSecondary != NULL);
 
-    OBJECTREF primary = ObjectFromHandle(handle);
+    _ASSERTE(handle != NULL);
 
     IGCHandleManager *mgr = GCHandleUtilities::GetGCHandleManager();
-    // Secondary is tracked only if primary is non-null
-    *outSecondary = (primary != NULL) ? mgr->GetDependentHandleSecondary(handle) : NULL;
-
-    return OBJECTREFToObject(primary);
+    mgr->StoreObjectInHandle(handle, NULL);
 }
 FCIMPLEND
 
-FCIMPL2(VOID, DependentHandle::nSetPrimary, OBJECTHANDLE handle, Object *_primary)
+FCIMPL2(VOID, DependentHandle::InternalSetDependent, OBJECTHANDLE handle, Object *_dependent)
 {
     FCALL_CONTRACT;
 
     _ASSERTE(handle != NULL);
 
-    // Avoid collision with MarshalNative::GCHandleInternalSet
-    FCUnique(0x12);
-
     IGCHandleManager *mgr = GCHandleUtilities::GetGCHandleManager();
-    mgr->StoreObjectInHandle(handle, _primary);
+    mgr->SetDependentHandleSecondary(handle, _dependent);
 }
 FCIMPLEND
 
-FCIMPL2(VOID, DependentHandle::nSetSecondary, OBJECTHANDLE handle, Object *_secondary)
+FCIMPL1(VOID, DependentHandle::InternalFree, OBJECTHANDLE handle)
 {
     FCALL_CONTRACT;
 
     _ASSERTE(handle != NULL);
 
-    IGCHandleManager *mgr = GCHandleUtilities::GetGCHandleManager();
-    mgr->SetDependentHandleSecondary(handle, _secondary);
+    HELPER_METHOD_FRAME_BEGIN_0();
+
+    DestroyDependentHandle(handle);
+
+    HELPER_METHOD_FRAME_END();
 }
 FCIMPLEND
diff --git a/src/coreclr/vm/comdependenthandle.h b/src/coreclr/vm/comdependenthandle.h
index 7c80d0bebe4..06786e62577 100644
--- a/src/coreclr/vm/comdependenthandle.h
+++ b/src/coreclr/vm/comdependenthandle.h
@@ -16,16 +16,16 @@
 
 // A dependent handle is conceputally a tuple containing two object reference
 //
-//     * A Primary object (think key)
-//     * A Secondary Object (think value)
+//     * A Target object (think key)
+//     * A Dependent Object (think value)
 //
-// The reference to both the primary object is (long) weak (will not keep the object alive). However the
-// reference to the secondary object is (long) weak if the primary object is dead, and strong if the primary
-// object is alive. (Hence it is a 'Dependent' handle since the strength of the secondary reference depends
-// on the primary).
+// The reference to both the target object is (long) weak (will not keep the object alive). However the
+// reference to the dependent object is (long) weak if the target object is dead, and strong if the target
+// object is alive. (Hence it is a 'Dependent' handle since the strength of the dependent reference depends
+// on the target).
 //
 // The effect of this semantics is that it seems that while the DependentHandle exists, the system behaves as
-// if there was a normal strong reference from the primary object to the secondary one.
+// if there was a normal strong reference from the target object to the dependent one.
 //
 // The usefulness of a DependentHandle is to allow other objects to be 'attached' to a given object. By
 // having a hash table where the entries are dependent handles you can attach arbitrary objects to another
@@ -40,13 +40,13 @@
 class DependentHandle
 {
 public:
-    static FCDECL2(OBJECTHANDLE, nInitialize, Object *primary, Object *secondary);
-    static FCDECL1(Object *, nGetPrimary, OBJECTHANDLE handle);
-    static FCDECL2(Object *, nGetPrimaryAndSecondary, OBJECTHANDLE handle, Object **outSecondary);
-    static FCDECL1(VOID, nFree, OBJECTHANDLE handle);
-    static FCDECL2(VOID, nSetPrimary, OBJECTHANDLE handle, Object *primary);
-    static FCDECL2(VOID, nSetSecondary, OBJECTHANDLE handle, Object *secondary);
+    static FCDECL2(OBJECTHANDLE, InternalInitialize, Object *target, Object *dependent);
+    static FCDECL1(Object *, InternalGetTarget, OBJECTHANDLE handle);
+    static FCDECL1(Object *, InternalGetDependent, OBJECTHANDLE handle);
+    static FCDECL2(Object *, InternalGetTargetAndDependent, OBJECTHANDLE handle, Object **outDependent);
+    static FCDECL1(VOID, InternalSetTargetToNull, OBJECTHANDLE handle);
+    static FCDECL2(VOID, InternalSetDependent, OBJECTHANDLE handle, Object *dependent);
+    static FCDECL1(VOID, InternalFree, OBJECTHANDLE handle);
 };
 
 #endif
-
diff --git a/src/coreclr/vm/ecalllist.h b/src/coreclr/vm/ecalllist.h
index 5cc1d5827fb..f77dc75c80b 100644
--- a/src/coreclr/vm/ecalllist.h
+++ b/src/coreclr/vm/ecalllist.h
@@ -61,12 +61,13 @@
 
 
 FCFuncStart(gDependentHandleFuncs)
-    FCFuncElement("nInitialize",             DependentHandle::nInitialize)
-    FCFuncElement("nGetPrimary",             DependentHandle::nGetPrimary)
-    FCFuncElement("nGetPrimaryAndSecondary", DependentHandle::nGetPrimaryAndSecondary)
-    FCFuncElement("nFree",                   DependentHandle::nFree)
-    FCFuncElement("nSetPrimary",             DependentHandle::nSetPrimary)
-    FCFuncElement("nSetSecondary",           DependentHandle::nSetSecondary)
+    FCFuncElement("InternalInitialize",            DependentHandle::InternalInitialize)
+    FCFuncElement("InternalGetTarget",             DependentHandle::InternalGetTarget)
+    FCFuncElement("InternalGetDependent",          DependentHandle::InternalGetDependent)
+    FCFuncElement("InternalGetTargetAndDependent", DependentHandle::InternalGetTargetAndDependent)
+    FCFuncElement("InternalSetTargetToNull",       DependentHandle::InternalSetTargetToNull)
+    FCFuncElement("InternalSetDependent",          DependentHandle::InternalSetDependent)
+    FCFuncElement("InternalFree",                  DependentHandle::InternalFree)
 FCFuncEnd()
 
 
@@ -1139,7 +1140,7 @@ FCClassElement("CustomAttribute", "System.Reflection", gCOMCustomAttributeFuncs)
 FCClassElement("CustomAttributeEncodedArgument", "System.Reflection", gCustomAttributeEncodedArgument)
 FCClassElement("Debugger", "System.Diagnostics", gDiagnosticsDebugger)
 FCClassElement("Delegate", "System", gDelegateFuncs)
-FCClassElement("DependentHandle", "System.Runtime.CompilerServices", gDependentHandleFuncs)
+FCClassElement("DependentHandle", "System.Runtime", gDependentHandleFuncs)
 FCClassElement("Enum", "System", gEnumFuncs)
 FCClassElement("Environment", "System", gEnvironmentFuncs)
 #if defined(FEATURE_PERFTRACING)