Skip to content

Commit 02f70d0

Browse files
Sergio0694stephentoub
Sergio0694
and
stephentoub
authored
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 #54246 (comment) onwards) * Rename InternalSetTarget to StopTracking, remove redundant param * Remove FCUnique from InternalStopTracking This was added in #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>
1 parent 2abd487 commit 02f70d0

File tree

16 files changed

+851
-229
lines changed

16 files changed

+851
-229
lines changed

‎src/coreclr/System.Private.CoreLib/System.Private.CoreLib.csproj‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -206,13 +206,13 @@
206206
<Compile Include="$(BclSourcesRoot)\System\Reflection\Metadata\RuntimeTypeMetadataUpdateHandler.cs" />
207207
<Compile Include="$(BclSourcesRoot)\System\Resources\ManifestBasedResourceGroveler.CoreCLR.cs" />
208208
<Compile Include="$(BclSourcesRoot)\System\Runtime\CompilerServices\CrossLoaderAllocatorHashHelpers.cs" />
209-
<Compile Include="$(BclSourcesRoot)\System\Runtime\CompilerServices\DependentHandle.cs" />
210209
<Compile Include="$(BclSourcesRoot)\System\Runtime\CompilerServices\GCHeapHash.cs" />
211210
<Compile Include="$(BclSourcesRoot)\System\Runtime\CompilerServices\CastHelpers.cs" />
212211
<Compile Include="$(BclSourcesRoot)\System\Runtime\CompilerServices\ICastableHelpers.cs" />
213212
<Compile Include="$(BclSourcesRoot)\System\Runtime\CompilerServices\RuntimeFeature.CoreCLR.cs" />
214213
<Compile Include="$(BclSourcesRoot)\System\Runtime\CompilerServices\RuntimeHelpers.CoreCLR.cs" />
215214
<Compile Include="$(BclSourcesRoot)\System\Runtime\CompilerServices\TypeDependencyAttribute.cs" />
215+
<Compile Include="$(BclSourcesRoot)\System\Runtime\DependentHandle.cs" />
216216
<Compile Include="$(BclSourcesRoot)\System\Runtime\GCSettings.CoreCLR.cs" />
217217
<Compile Include="$(BclSourcesRoot)\System\Runtime\InteropServices\ComTypes\IEnumerable.cs" />
218218
<Compile Include="$(BclSourcesRoot)\System\Runtime\InteropServices\ComTypes\IEnumerator.cs" />

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

Lines changed: 0 additions & 84 deletions
This file was deleted.

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

Lines changed: 267 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,267 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
4+
using System.Runtime.CompilerServices;
5+
#if !DEBUG
6+
using Internal.Runtime.CompilerServices;
7+
#endif
8+
9+
namespace System.Runtime
10+
{
11+
/// <summary>
12+
/// Represents a dependent GC handle, which will conditionally keep a dependent object instance alive as long as
13+
/// a target object instance is alive as well, without representing a strong reference to the target instance.
14+
/// </summary>
15+
/// <remarks>
16+
/// A <see cref="DependentHandle"/> value with a given object instance as target will not cause the target
17+
/// to be kept alive if there are no other strong references to it, but it will do so for the dependent
18+
/// object instance as long as the target is alive.
19+
/// <para>
20+
/// Using this type is conceptually equivalent to having a weak reference to a given target object instance A,
21+
/// with that object having a field or property (or some other strong reference) to a dependent object instance B.
22+
/// </para>
23+
/// <para>
24+
/// The <see cref="DependentHandle"/> type is not thread-safe, and consumers are responsible for ensuring that
25+
/// <see cref="Dispose"/> is not called concurrently with other APIs. Not doing so results in undefined behavior.
26+
/// </para>
27+
/// <para>
28+
/// The <see cref="IsAllocated"/>, <see cref="Target"/>, <see cref="Dependent"/> and <see cref="TargetAndDependent"/>
29+
/// properties are instead thread-safe, and safe to use if <see cref="Dispose"/> is not concurrently invoked as well.
30+
/// </para>
31+
/// </remarks>
32+
public struct DependentHandle : IDisposable
33+
{
34+
// =========================================================================================
35+
// This struct collects all operations on native DependentHandles. The DependentHandle
36+
// merely wraps an IntPtr so this struct serves mainly as a "managed typedef."
37+
//
38+
// DependentHandles exist in one of two states:
39+
//
40+
// IsAllocated == false
41+
// No actual handle is allocated underneath. Illegal to get Target, Dependent
42+
// or GetTargetAndDependent(). Ok to call Dispose().
43+
//
44+
// Initializing a DependentHandle using the nullary ctor creates a DependentHandle
45+
// that's in the !IsAllocated state.
46+
// (! Right now, we get this guarantee for free because (IntPtr)0 == NULL unmanaged handle.
47+
// ! If that assertion ever becomes false, we'll have to add an _isAllocated field
48+
// ! to compensate.)
49+
//
50+
//
51+
// IsAllocated == true
52+
// There's a handle allocated underneath. You must call Dispose() on this eventually
53+
// or you cause a native handle table leak.
54+
//
55+
// This struct intentionally does no self-synchronization. It's up to the caller to
56+
// to use DependentHandles in a thread-safe way.
57+
// =========================================================================================
58+
59+
private IntPtr _handle;
60+
61+
/// <summary>
62+
/// Initializes a new instance of the <see cref="DependentHandle"/> struct with the specified arguments.
63+
/// </summary>
64+
/// <param name="target">The target object instance to track.</param>
65+
/// <param name="dependent">The dependent object instance to associate with <paramref name="target"/>.</param>
66+
public DependentHandle(object? target, object? dependent)
67+
{
68+
// no need to check for null result: InternalInitialize expected to throw OOM.
69+
_handle = InternalInitialize(target, dependent);
70+
}
71+
72+
/// <summary>
73+
/// Gets a value indicating whether this instance was constructed with
74+
/// <see cref="DependentHandle(object?, object?)"/> and has not yet been disposed.
75+
/// </summary>
76+
/// <remarks>This property is thread-safe.</remarks>
77+
public bool IsAllocated => (nint)_handle != 0;
78+
79+
/// <summary>
80+
/// Gets or sets the target object instance for the current handle. The target can only be set to a <see langword="null"/> value
81+
/// once the <see cref="DependentHandle"/> instance has been created. Doing so will cause <see cref="Dependent"/> to start
82+
/// returning <see langword="null"/> as well, and to become eligible for collection even if the previous target is still alive.
83+
/// </summary>
84+
/// <exception cref="InvalidOperationException">
85+
/// Thrown if <see cref="IsAllocated"/> is <see langword="false"/> or if the input value is not <see langword="null"/>.</exception>
86+
/// <remarks>This property is thread-safe.</remarks>
87+
public object? Target
88+
{
89+
get
90+
{
91+
IntPtr handle = _handle;
92+
93+
if ((nint)handle == 0)
94+
{
95+
ThrowHelper.ThrowInvalidOperationException();
96+
}
97+
98+
return InternalGetTarget(handle);
99+
}
100+
set
101+
{
102+
IntPtr handle = _handle;
103+
104+
if ((nint)handle == 0 || value is not null)
105+
{
106+
ThrowHelper.ThrowInvalidOperationException();
107+
}
108+
109+
InternalSetTargetToNull(handle);
110+
}
111+
}
112+
113+
/// <summary>
114+
/// Gets or sets the dependent object instance for the current handle.
115+
/// </summary>
116+
/// <remarks>
117+
/// If it is needed to retrieve both <see cref="Target"/> and <see cref="Dependent"/>, it is necessary
118+
/// to ensure that the returned instance from <see cref="Target"/> will be kept alive until <see cref="Dependent"/>
119+
/// is retrieved as well, or it might be collected and result in unexpected behavior. This can be done by storing the
120+
/// target in a local and calling <see cref="GC.KeepAlive(object)"/> on it after <see cref="Dependent"/> is accessed.
121+
/// </remarks>
122+
/// <exception cref="InvalidOperationException">Thrown if <see cref="IsAllocated"/> is <see langword="false"/>.</exception>
123+
/// <remarks>This property is thread-safe.</remarks>
124+
public object? Dependent
125+
{
126+
get
127+
{
128+
IntPtr handle = _handle;
129+
130+
if ((nint)handle == 0)
131+
{
132+
ThrowHelper.ThrowInvalidOperationException();
133+
}
134+
135+
return InternalGetDependent(handle);
136+
}
137+
set
138+
{
139+
IntPtr handle = _handle;
140+
141+
if ((nint)handle == 0)
142+
{
143+
ThrowHelper.ThrowInvalidOperationException();
144+
}
145+
146+
InternalSetDependent(handle, value);
147+
}
148+
}
149+
150+
/// <summary>
151+
/// Gets the values of both <see cref="Target"/> and <see cref="Dependent"/> (if available) as an atomic operation.
152+
/// That is, even if <see cref="Target"/> is concurrently set to <see langword="null"/>, calling this method
153+
/// will either return <see langword="null"/> for both target and dependent, or return both previous values.
154+
/// If <see cref="Target"/> and <see cref="Dependent"/> were used sequentially in this scenario instead, it
155+
/// would be possible to sometimes successfully retrieve the previous target, but then fail to get the dependent.
156+
/// </summary>
157+
/// <returns>The values of <see cref="Target"/> and <see cref="Dependent"/>.</returns>
158+
/// <exception cref="InvalidOperationException">Thrown if <see cref="IsAllocated"/> is <see langword="false"/>.</exception>
159+
/// <remarks>This property is thread-safe.</remarks>
160+
public (object? Target, object? Dependent) TargetAndDependent
161+
{
162+
get
163+
{
164+
IntPtr handle = _handle;
165+
166+
if ((nint)handle == 0)
167+
{
168+
ThrowHelper.ThrowInvalidOperationException();
169+
}
170+
171+
object? target = InternalGetTargetAndDependent(handle, out object? dependent);
172+
173+
return (target, dependent);
174+
}
175+
}
176+
177+
/// <summary>
178+
/// Gets the target object instance for the current handle.
179+
/// </summary>
180+
/// <returns>The target object instance, if present.</returns>
181+
/// <remarks>This method mirrors <see cref="Target"/>, but without the allocation check.</remarks>
182+
internal object? UnsafeGetTarget()
183+
{
184+
return InternalGetTarget(_handle);
185+
}
186+
187+
/// <summary>
188+
/// Atomically retrieves the values of both <see cref="Target"/> and <see cref="Dependent"/>, if available.
189+
/// </summary>
190+
/// <param name="dependent">The dependent instance, if available.</param>
191+
/// <returns>The values of <see cref="Target"/> and <see cref="Dependent"/>.</returns>
192+
/// <remarks>
193+
/// This method mirrors the <see cref="TargetAndDependent"/> property, but without the allocation check.
194+
/// The signature is also kept the same as the one for the internal call, to improve the codegen.
195+
/// Note that <paramref name="dependent"/> is required to be on the stack (or it might not be tracked).
196+
/// </remarks>
197+
internal object? UnsafeGetTargetAndDependent(out object? dependent)
198+
{
199+
return InternalGetTargetAndDependent(_handle, out dependent);
200+
}
201+
202+
/// <summary>
203+
/// Sets the dependent object instance for the current handle to <see langword="null"/>.
204+
/// </summary>
205+
/// <remarks>This method mirrors the <see cref="Target"/> setter, but without allocation and input checks.</remarks>
206+
internal void UnsafeSetTargetToNull()
207+
{
208+
InternalSetTargetToNull(_handle);
209+
}
210+
211+
/// <summary>
212+
/// Sets the dependent object instance for the current handle.
213+
/// </summary>
214+
/// <remarks>This method mirrors <see cref="Dependent"/>, but without the allocation check.</remarks>
215+
internal void UnsafeSetDependent(object? dependent)
216+
{
217+
InternalSetDependent(_handle, dependent);
218+
}
219+
220+
/// <inheritdoc cref="IDisposable.Dispose"/>
221+
/// <remarks>This method is not thread-safe.</remarks>
222+
public void Dispose()
223+
{
224+
// Forces the DependentHandle back to non-allocated state
225+
// (if not already there) and frees the handle if needed.
226+
IntPtr handle = _handle;
227+
228+
if ((nint)handle != 0)
229+
{
230+
_handle = IntPtr.Zero;
231+
232+
InternalFree(handle);
233+
}
234+
}
235+
236+
[MethodImpl(MethodImplOptions.InternalCall)]
237+
private static extern IntPtr InternalInitialize(object? target, object? dependent);
238+
239+
#if DEBUG
240+
[MethodImpl(MethodImplOptions.InternalCall)]
241+
private static extern object? InternalGetTarget(IntPtr dependentHandle);
242+
#else
243+
private static unsafe object? InternalGetTarget(IntPtr dependentHandle)
244+
{
245+
// This optimization is the same that is used in GCHandle in RELEASE mode.
246+
// This is not used in DEBUG builds as the runtime performs additional checks.
247+
// The logic below is the inlined copy of ObjectFromHandle in the unmanaged runtime.
248+
return Unsafe.As<IntPtr, object>(ref *(IntPtr*)(nint)dependentHandle);
249+
}
250+
#endif
251+
252+
[MethodImpl(MethodImplOptions.InternalCall)]
253+
private static extern object? InternalGetDependent(IntPtr dependentHandle);
254+
255+
[MethodImpl(MethodImplOptions.InternalCall)]
256+
private static extern object? InternalGetTargetAndDependent(IntPtr dependentHandle, out object? dependent);
257+
258+
[MethodImpl(MethodImplOptions.InternalCall)]
259+
private static extern void InternalSetDependent(IntPtr dependentHandle, object? dependent);
260+
261+
[MethodImpl(MethodImplOptions.InternalCall)]
262+
private static extern void InternalSetTargetToNull(IntPtr dependentHandle);
263+
264+
[MethodImpl(MethodImplOptions.InternalCall)]
265+
private static extern void InternalFree(IntPtr dependentHandle);
266+
}
267+
}

‎src/coreclr/System.Private.CoreLib/src/System/Runtime/InteropServices/GCHandle.CoreCLR.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ public partial struct GCHandle
2222
internal static extern object? InternalGet(IntPtr handle);
2323
#else
2424
internal static unsafe object? InternalGet(IntPtr handle) =>
25-
Unsafe.As<IntPtr, object>(ref *(IntPtr*)handle);
25+
Unsafe.As<IntPtr, object>(ref *(IntPtr*)(nint)handle);
2626
#endif
2727

2828
[MethodImpl(MethodImplOptions.InternalCall)]

0 commit comments

Comments
 (0)