Update API surface to match approved specs from API review

src/coreclr/System.Private.CoreLib/src/System/Runtime/DependentHandle.cs

@@ -70,9 +70,12 @@ public DependentHandle(object? target, object? dependent)
         public bool IsAllocated => (nint)_handle != 0;
 
         /// <summary>
-        /// Gets the target object instance for the current handle.
+        /// Gets or sets the target object instance for the current handle. The target can only be set to a <see langword="null"/> value
+        /// once the <see cref="DependentHandle"/> instance has been created. Doing so will cause <see cref="Dependent"/> to start
+        /// returning <see langword="null"/> as well, and to become eligible for collection even if the previous target is still alive.
         /// </summary>
-        /// <exception cref="InvalidOperationException">Thrown if <see cref="IsAllocated"/> is <see langword="false"/>.</exception>
+        /// <exception cref="InvalidOperationException">
+        /// Thrown if <see cref="IsAllocated"/> is <see langword="false"/> or if the input value is not <see langword="null"/>.</exception>
         /// <remarks>This property is thread-safe.</remarks>
         public object? Target
         {
@@ -87,6 +90,17 @@ public object? Target
 
                 return InternalGetTarget(handle);
             }
+            set
+            {
+                IntPtr handle = _handle;
+
+                if ((nint)handle == 0 || value is not null)
+                {
+                    ThrowHelper.ThrowInvalidOperationException();
+                }
+
+                InternalSetTargetToNull(handle);
+            }
         }
 
         /// <summary>
@@ -127,49 +141,29 @@ public object? Dependent
         }
 
         /// <summary>
-        /// Atomically retrieves the values of both <see cref="Target"/> and <see cref="Dependent"/>, if available.
+        /// Gets the values of both <see cref="Target"/> and <see cref="Dependent"/> (if available) as an atomic operation.
         /// That is, even if <see cref="Target"/> is concurrently set to <see langword="null"/>, calling this method
         /// will either return <see langword="null"/> for both target and dependent, or return both previous values.
         /// If <see cref="Target"/> and <see cref="Dependent"/> were used sequentially in this scenario instead, it's
         /// could be possible to sometimes successfully retrieve the previous target, but then fail to get the dependent.
         /// </summary>
         /// <returns>The values of <see cref="Target"/> and <see cref="Dependent"/>.</returns>
         /// <exception cref="InvalidOperationException">Thrown if <see cref="IsAllocated"/> is <see langword="false"/>.</exception>
-        public (object? Target, object? Dependent) GetTargetAndDependent()
+        public (object? Target, object? Dependent) TargetAndDependent
         {
-            IntPtr handle = _handle;
-
-            if ((nint)handle == 0)
+            get
             {
-                ThrowHelper.ThrowInvalidOperationException();
-            }
-
-            object? target = InternalGetTargetAndDependent(handle, out object? dependent);
+                IntPtr handle = _handle;
 
-            return (target, dependent);
-        }
+                if ((nint)handle == 0)
+                {
+                    ThrowHelper.ThrowInvalidOperationException();
+                }
 
-        /// <summary>
-        /// Stops tracking the target and dependent objects in the current <see cref="DependentHandle"/> instance. Once this method
-        /// is invoked, calling other APIs is still allowed, but <see cref="Target"/> and <see cref="Dependent"/> will always just
-        /// return <see langword="null"/>. Additionally, since the dependent instance will no longer be tracked, it will also
-        /// immediately become eligible for collection if there are no other roots for it, even if the original target is still alive.
-        /// </summary>
-        /// <exception cref="InvalidOperationException">Thrown if <see cref="IsAllocated"/> is <see langword="false"/>.</exception>
-        /// <remarks>
-        /// This method does not dispose the current <see cref="DependentHandle"/> instance, which means that after calling it will not
-        /// affect the value of <see cref="IsAllocated"/>, and <see cref="Dispose"/> will still need to be called to release resources.
-        /// </remarks>
-        public void StopTracking()
-        {
-            IntPtr handle = _handle;
+                object? target = InternalGetTargetAndDependent(handle, out object? dependent);
 
-            if ((nint)handle == 0)
-            {
-                ThrowHelper.ThrowInvalidOperationException();
+                return (target, dependent);
             }
-
-            InternalStopTracking(handle);
         }
 
         /// <summary>
@@ -188,7 +182,7 @@ public void StopTracking()
         /// <param name="dependent">The dependent instance, if available.</param>
         /// <returns>The values of <see cref="Target"/> and <see cref="Dependent"/>.</returns>
         /// <remarks>
-        /// This method mirrors <see cref="GetTargetAndDependent"/>, but without the allocation check.
+        /// This method mirrors the <see cref="TargetAndDependent"/> property, but without the allocation check.
         /// The signature is also kept the same as the one for the internal call, to improve the codegen.
         /// Note that <paramref name="dependent"/> is required to be on the stack (or it might not be tracked).
         /// </remarks>
@@ -198,22 +192,21 @@ public void StopTracking()
         }
 
         /// <summary>
-        /// Sets the dependent object instance for the current handle.
+        /// Sets the dependent object instance for the current handle to <see langword="null"/>.
         /// </summary>
-        /// <remarks>This method mirrors <see cref="Dependent"/>, but without the allocation check.</remarks>
-        internal void UnsafeSetDependent(object? dependent)
+        /// <remarks>This method mirrors the <see cref="Target"/> setter, but without allocation and input checks.</remarks>
+        internal void UnsafeSetTargetToNull()
         {
-            InternalSetDependent(_handle, dependent);
+            InternalSetTargetToNull(_handle);
         }
 
         /// <summary>
-        /// Stops tracking the target and dependent objects in the current <see cref="DependentHandle"/> instance.
+        /// Sets the dependent object instance for the current handle.
         /// </summary>
-        /// <exception cref="InvalidOperationException">Thrown if <see cref="IsAllocated"/> is <see langword="false"/>.</exception>
-        /// <remarks>This method mirrors <see cref="StopTracking"/>, but without the allocation check.</remarks>
-        internal void UnsafeStopTracking()
+        /// <remarks>This method mirrors <see cref="Dependent"/>, but without the allocation check.</remarks>
+        internal void UnsafeSetDependent(object? dependent)
         {
-            InternalStopTracking(_handle);
+            InternalSetDependent(_handle, dependent);
         }
 
         /// <inheritdoc cref="IDisposable.Dispose"/>
@@ -258,7 +251,7 @@ public void Dispose()
         private static extern void InternalSetDependent(IntPtr dependentHandle, object? dependent);
 
         [MethodImpl(MethodImplOptions.InternalCall)]
-        private static extern void InternalStopTracking(IntPtr dependentHandle);
+        private static extern void InternalSetTargetToNull(IntPtr dependentHandle);
 
         [MethodImpl(MethodImplOptions.InternalCall)]
         private static extern void InternalFree(IntPtr dependentHandle);

src/coreclr/vm/comdependenthandle.cpp

@@ -75,25 +75,25 @@ FCIMPL2(Object*, DependentHandle::InternalGetTargetAndDependent, OBJECTHANDLE ha
 }
 FCIMPLEND
 
-FCIMPL2(VOID, DependentHandle::InternalSetDependent, OBJECTHANDLE handle, Object *_dependent)
+FCIMPL1(VOID, DependentHandle::InternalSetTargetToNull, OBJECTHANDLE handle)
 {
     FCALL_CONTRACT;
 
     _ASSERTE(handle != NULL);
 
     IGCHandleManager *mgr = GCHandleUtilities::GetGCHandleManager();
-    mgr->SetDependentHandleSecondary(handle, _dependent);
+    mgr->StoreObjectInHandle(handle, NULL);
 }
 FCIMPLEND
 
-FCIMPL1(VOID, DependentHandle::InternalStopTracking, OBJECTHANDLE handle)
+FCIMPL2(VOID, DependentHandle::InternalSetDependent, OBJECTHANDLE handle, Object *_dependent)
 {
     FCALL_CONTRACT;
 
     _ASSERTE(handle != NULL);
 
     IGCHandleManager *mgr = GCHandleUtilities::GetGCHandleManager();
-    mgr->StoreObjectInHandle(handle, NULL);
+    mgr->SetDependentHandleSecondary(handle, _dependent);
 }
 FCIMPLEND
 

src/coreclr/vm/comdependenthandle.h

@@ -44,8 +44,8 @@ class DependentHandle
     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, InternalStopTracking, OBJECTHANDLE handle);
     static FCDECL1(VOID, InternalFree, OBJECTHANDLE handle);
 };
 

src/coreclr/vm/ecalllist.h

@@ -65,8 +65,8 @@ FCFuncStart(gDependentHandleFuncs)
     FCFuncElement("InternalGetTarget",             DependentHandle::InternalGetTarget)
     FCFuncElement("InternalGetDependent",          DependentHandle::InternalGetDependent)
     FCFuncElement("InternalGetTargetAndDependent", DependentHandle::InternalGetTargetAndDependent)
+    FCFuncElement("InternalSetTargetToNull",       DependentHandle::InternalSetTargetToNull)
     FCFuncElement("InternalSetDependent",          DependentHandle::InternalSetDependent)
-    FCFuncElement("InternalStopTracking",          DependentHandle::InternalStopTracking)
     FCFuncElement("InternalFree",                  DependentHandle::InternalFree)
 FCFuncEnd()
 

src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/ConditionalWeakTable.cs

@@ -595,7 +595,7 @@ private void RemoveIndex(int entryIndex)
                 Volatile.Write(ref entry.HashCode, -1);
 
                 // Also, clear the key to allow GC to collect objects pointed to by the entry
-                entry.depHnd.UnsafeStopTracking();
+                entry.depHnd.UnsafeSetTargetToNull();
             }
 
             internal void UpdateValue(int entryIndex, TValue newValue)

src/libraries/System.Runtime/ref/System.Runtime.cs

@@ -9600,10 +9600,9 @@ public partial struct DependentHandle : System.IDisposable
         public DependentHandle(object? target, object? dependent) { throw null; }
         public object? Dependent { get { throw null; } set { } }
         public bool IsAllocated { get { throw null; } }
-        public object? Target { get { throw null; } }
+        public object? Target { get { throw null; } set { } }
+        public (object? Target, object? Dependent) TargetAndDependent { get { throw null; } }
         public void Dispose() { }
-        public (object? Target, object? Dependent) GetTargetAndDependent() { throw null; }
-        public void StopTracking() { throw null; }
     }
     public enum GCLargeObjectHeapCompactionMode
     {

src/libraries/System.Runtime/tests/System/Runtime/DependentHandleTests.cs

@@ -37,6 +37,48 @@ public void GetNotNullTarget()
             handle.Dispose();
         }
 
+        [Fact]
+        public void SetTargetToNull_StateIsConsistent()
+        {
+            object target = new(), dependent = new();
+            DependentHandle handle = new(target, dependent);
+
+            Assert.True(handle.IsAllocated);
+            Assert.Same(handle.Target, target);
+            Assert.Same(handle.Dependent, dependent);
+
+            handle.Target = null;
+
+            Assert.True(handle.IsAllocated);
+            Assert.Null(handle.Target);
+            Assert.Null(handle.Dependent);
+
+            handle.Dispose();
+        }
+
+        [Fact]
+        public void SetTargetToNull_RepeatedCallsAreFine()
+        {
+            object target = new(), dependent = new();
+            DependentHandle handle = new(target, dependent);
+
+            handle.Target = null;
+
+            Assert.True(handle.IsAllocated);
+            Assert.Null(handle.Target);
+            Assert.Null(handle.Dependent);
+
+            handle.Target = null;
+            handle.Target = null;
+            handle.Target = null;
+
+            Assert.True(handle.IsAllocated);
+            Assert.Null(handle.Target);
+            Assert.Null(handle.Dependent);
+
+            handle.Dispose();
+        }
+
         [Fact]
         public void GetSetDependent()
         {
@@ -175,50 +217,8 @@ private sealed class ObjectWithReference
             public object Reference;
         }
 
-        [Fact]
-        public void StopTracking_RepeatedCallsAreFine()
-        {
-            object target = new(), dependent = new();
-            DependentHandle handle = new(target, dependent);
-
-            handle.StopTracking();
-
-            Assert.True(handle.IsAllocated);
-            Assert.Null(handle.Target);
-            Assert.Null(handle.Dependent);
-
-            handle.StopTracking();
-            handle.StopTracking();
-            handle.StopTracking();
-
-            Assert.True(handle.IsAllocated);
-            Assert.Null(handle.Target);
-            Assert.Null(handle.Dependent);
-
-            handle.Dispose();
-        }
-
-        [Fact]
-        public void StopTracking_StateIsConsistent()
-        {
-            object target = new(), dependent = new();
-            DependentHandle handle = new(target, dependent);
-
-            Assert.True(handle.IsAllocated);
-            Assert.Same(handle.Target, target);
-            Assert.Same(handle.Dependent, dependent);
-
-            handle.StopTracking();
-
-            Assert.True(handle.IsAllocated);
-            Assert.Null(handle.Target);
-            Assert.Null(handle.Dependent);
-
-            handle.Dispose();
-        }
-
         [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsPreciseGcSupported))]
-        public void DependentIsCollectedAfterStopTracking()
+        public void DependentIsCollectedAfterTargetIsSetToNull()
         {
             [MethodImpl(MethodImplOptions.NoInlining)]
             static DependentHandle Initialize(out object target, out WeakReference weakDependent)
@@ -234,7 +234,7 @@ static DependentHandle Initialize(out object target, out WeakReference weakDepen
 
             DependentHandle handle = Initialize(out object target, out WeakReference dependent);
 
-            handle.StopTracking();
+            handle.Target = null;
 
             GC.Collect();
 
@@ -263,19 +263,41 @@ public void GetDependent_ThrowsInvalidOperationException()
         }
 
         [Fact]
-        public void SetDependent_ThrowsInvalidOperationException()
+        public void SetTarget_NotAllocated_ThrowsInvalidOperationException()
         {
             Assert.Throws<InvalidOperationException>(() =>
             {
                 DependentHandle handle = default;
-                handle.Dependent = new();
+                handle.Target = new();
             });
         }
 
         [Fact]
-        public void StopTracking_ThrowsInvalidOperationException()
+        public void SetTarget_NotNullObject_ThrowsInvalidOperationException()
         {
-            Assert.Throws<InvalidOperationException>(() => default(DependentHandle).StopTracking());
+            Assert.Throws<InvalidOperationException>(() =>
+            {
+                DependentHandle handle = default;
+
+                try
+                {
+                    handle.Target = new();
+                }
+                finally
+                {
+                    handle.Dispose();
+                }                
+            });
+        }
+
+        [Fact]
+        public void SetDependent_ThrowsInvalidOperationException()
+        {
+            Assert.Throws<InvalidOperationException>(() =>
+            {
+                DependentHandle handle = default;
+                handle.Dependent = new();
+            });
         }
 
         [Fact]