diff --git a/src/coreclr/System.Private.CoreLib/System.Private.CoreLib.csproj b/src/coreclr/System.Private.CoreLib/System.Private.CoreLib.csproj index 97671f56e5dc0..b42232633a777 100644 --- a/src/coreclr/System.Private.CoreLib/System.Private.CoreLib.csproj +++ b/src/coreclr/System.Private.CoreLib/System.Private.CoreLib.csproj @@ -206,13 +206,13 @@ - + diff --git a/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/DependentHandle.cs b/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/DependentHandle.cs deleted file mode 100644 index d0aff34f2bc7b..0000000000000 --- a/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/DependentHandle.cs +++ /dev/null @@ -1,84 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -namespace System.Runtime.CompilerServices -{ - // ========================================================================================= - // This struct collects all operations on native DependentHandles. The DependentHandle - // merely wraps an IntPtr so this struct serves mainly as a "managed typedef." - // - // DependentHandles exist in one of two states: - // - // IsAllocated == false - // No actual handle is allocated underneath. Illegal to call GetPrimary - // or GetPrimaryAndSecondary(). Ok to call Free(). - // - // Initializing a DependentHandle using the nullary ctor creates a DependentHandle - // that's in the !IsAllocated state. - // (! Right now, we get this guarantee for free because (IntPtr)0 == NULL unmanaged handle. - // ! If that assertion ever becomes false, we'll have to add an _isAllocated field - // ! to compensate.) - // - // - // IsAllocated == true - // There's a handle allocated underneath. You must call Free() on this eventually - // or you cause a native handle table leak. - // - // This struct intentionally does no self-synchronization. It's up to the caller to - // to use DependentHandles in a thread-safe way. - // ========================================================================================= - internal struct DependentHandle - { - private IntPtr _handle; - - public DependentHandle(object primary, object? secondary) => - // no need to check for null result: nInitialize expected to throw OOM. - _handle = nInitialize(primary, secondary); - - public bool IsAllocated => _handle != IntPtr.Zero; - - // Getting the secondary object is more expensive than getting the first so - // we provide a separate primary-only accessor for those times we only want the - // primary. - public object? GetPrimary() => nGetPrimary(_handle); - - public object? GetPrimaryAndSecondary(out object? secondary) => - nGetPrimaryAndSecondary(_handle, out secondary); - - public void SetPrimary(object? primary) => - nSetPrimary(_handle, primary); - - public void SetSecondary(object? secondary) => - nSetSecondary(_handle, secondary); - - // Forces dependentHandle back to non-allocated state (if not already there) - // and frees the handle if needed. - public void Free() - { - if (_handle != IntPtr.Zero) - { - IntPtr handle = _handle; - _handle = IntPtr.Zero; - nFree(handle); - } - } - - [MethodImpl(MethodImplOptions.InternalCall)] - private static extern IntPtr nInitialize(object primary, object? secondary); - - [MethodImpl(MethodImplOptions.InternalCall)] - private static extern object? nGetPrimary(IntPtr dependentHandle); - - [MethodImpl(MethodImplOptions.InternalCall)] - private static extern object? nGetPrimaryAndSecondary(IntPtr dependentHandle, out object? secondary); - - [MethodImpl(MethodImplOptions.InternalCall)] - private static extern void nSetPrimary(IntPtr dependentHandle, object? primary); - - [MethodImpl(MethodImplOptions.InternalCall)] - private static extern void nSetSecondary(IntPtr dependentHandle, object? secondary); - - [MethodImpl(MethodImplOptions.InternalCall)] - private static extern void nFree(IntPtr dependentHandle); - } -} diff --git a/src/coreclr/System.Private.CoreLib/src/System/Runtime/DependentHandle.cs b/src/coreclr/System.Private.CoreLib/src/System/Runtime/DependentHandle.cs new file mode 100644 index 0000000000000..f080ba28dda62 --- /dev/null +++ b/src/coreclr/System.Private.CoreLib/src/System/Runtime/DependentHandle.cs @@ -0,0 +1,267 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Runtime.CompilerServices; +#if !DEBUG +using Internal.Runtime.CompilerServices; +#endif + +namespace System.Runtime +{ + /// + /// Represents a dependent GC handle, which will conditionally keep a dependent object instance alive as long as + /// a target object instance is alive as well, without representing a strong reference to the target instance. + /// + /// + /// A value with a given object instance as target will not cause the target + /// to be kept alive if there are no other strong references to it, but it will do so for the dependent + /// object instance as long as the target is alive. + /// + /// Using this type is conceptually equivalent to having a weak reference to a given target object instance A, + /// with that object having a field or property (or some other strong reference) to a dependent object instance B. + /// + /// + /// The type is not thread-safe, and consumers are responsible for ensuring that + /// is not called concurrently with other APIs. Not doing so results in undefined behavior. + /// + /// + /// The , , and + /// properties are instead thread-safe, and safe to use if is not concurrently invoked as well. + /// + /// + public struct DependentHandle : IDisposable + { + // ========================================================================================= + // This struct collects all operations on native DependentHandles. The DependentHandle + // merely wraps an IntPtr so this struct serves mainly as a "managed typedef." + // + // DependentHandles exist in one of two states: + // + // IsAllocated == false + // No actual handle is allocated underneath. Illegal to get Target, Dependent + // or GetTargetAndDependent(). Ok to call Dispose(). + // + // Initializing a DependentHandle using the nullary ctor creates a DependentHandle + // that's in the !IsAllocated state. + // (! Right now, we get this guarantee for free because (IntPtr)0 == NULL unmanaged handle. + // ! If that assertion ever becomes false, we'll have to add an _isAllocated field + // ! to compensate.) + // + // + // IsAllocated == true + // There's a handle allocated underneath. You must call Dispose() on this eventually + // or you cause a native handle table leak. + // + // This struct intentionally does no self-synchronization. It's up to the caller to + // to use DependentHandles in a thread-safe way. + // ========================================================================================= + + private IntPtr _handle; + + /// + /// Initializes a new instance of the struct with the specified arguments. + /// + /// The target object instance to track. + /// The dependent object instance to associate with . + public DependentHandle(object? target, object? dependent) + { + // no need to check for null result: InternalInitialize expected to throw OOM. + _handle = InternalInitialize(target, dependent); + } + + /// + /// Gets a value indicating whether this instance was constructed with + /// and has not yet been disposed. + /// + /// This property is thread-safe. + public bool IsAllocated => (nint)_handle != 0; + + /// + /// Gets or sets the target object instance for the current handle. The target can only be set to a value + /// once the instance has been created. Doing so will cause to start + /// returning as well, and to become eligible for collection even if the previous target is still alive. + /// + /// + /// Thrown if is or if the input value is not . + /// This property is thread-safe. + public object? Target + { + get + { + IntPtr handle = _handle; + + if ((nint)handle == 0) + { + ThrowHelper.ThrowInvalidOperationException(); + } + + return InternalGetTarget(handle); + } + set + { + IntPtr handle = _handle; + + if ((nint)handle == 0 || value is not null) + { + ThrowHelper.ThrowInvalidOperationException(); + } + + InternalSetTargetToNull(handle); + } + } + + /// + /// Gets or sets the dependent object instance for the current handle. + /// + /// + /// If it is needed to retrieve both and , it is necessary + /// to ensure that the returned instance from will be kept alive until + /// is retrieved as well, or it might be collected and result in unexpected behavior. This can be done by storing the + /// target in a local and calling on it after is accessed. + /// + /// Thrown if is . + /// This property is thread-safe. + public object? Dependent + { + get + { + IntPtr handle = _handle; + + if ((nint)handle == 0) + { + ThrowHelper.ThrowInvalidOperationException(); + } + + return InternalGetDependent(handle); + } + set + { + IntPtr handle = _handle; + + if ((nint)handle == 0) + { + ThrowHelper.ThrowInvalidOperationException(); + } + + InternalSetDependent(handle, value); + } + } + + /// + /// Gets the values of both and (if available) as an atomic operation. + /// That is, even if is concurrently set to , calling this method + /// will either return for both target and dependent, or return both previous values. + /// If and were used sequentially in this scenario instead, it + /// would be possible to sometimes successfully retrieve the previous target, but then fail to get the dependent. + /// + /// The values of and . + /// Thrown if is . + /// This property is thread-safe. + public (object? Target, object? Dependent) TargetAndDependent + { + get + { + IntPtr handle = _handle; + + if ((nint)handle == 0) + { + ThrowHelper.ThrowInvalidOperationException(); + } + + object? target = InternalGetTargetAndDependent(handle, out object? dependent); + + return (target, dependent); + } + } + + /// + /// Gets the target object instance for the current handle. + /// + /// The target object instance, if present. + /// This method mirrors , but without the allocation check. + internal object? UnsafeGetTarget() + { + return InternalGetTarget(_handle); + } + + /// + /// Atomically retrieves the values of both and , if available. + /// + /// The dependent instance, if available. + /// The values of and . + /// + /// This method mirrors the 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 is required to be on the stack (or it might not be tracked). + /// + internal object? UnsafeGetTargetAndDependent(out object? dependent) + { + return InternalGetTargetAndDependent(_handle, out dependent); + } + + /// + /// Sets the dependent object instance for the current handle to . + /// + /// This method mirrors the setter, but without allocation and input checks. + internal void UnsafeSetTargetToNull() + { + InternalSetTargetToNull(_handle); + } + + /// + /// Sets the dependent object instance for the current handle. + /// + /// This method mirrors , but without the allocation check. + internal void UnsafeSetDependent(object? dependent) + { + InternalSetDependent(_handle, dependent); + } + + /// + /// This method is not thread-safe. + public void Dispose() + { + // Forces the DependentHandle back to non-allocated state + // (if not already there) and frees the handle if needed. + IntPtr handle = _handle; + + if ((nint)handle != 0) + { + _handle = IntPtr.Zero; + + InternalFree(handle); + } + } + + [MethodImpl(MethodImplOptions.InternalCall)] + private static extern IntPtr InternalInitialize(object? target, object? dependent); + +#if DEBUG + [MethodImpl(MethodImplOptions.InternalCall)] + private static extern object? InternalGetTarget(IntPtr dependentHandle); +#else + private static unsafe object? InternalGetTarget(IntPtr dependentHandle) + { + // This optimization is the same that is used in GCHandle in RELEASE mode. + // This is not used in DEBUG builds as the runtime performs additional checks. + // The logic below is the inlined copy of ObjectFromHandle in the unmanaged runtime. + return Unsafe.As(ref *(IntPtr*)(nint)dependentHandle); + } +#endif + + [MethodImpl(MethodImplOptions.InternalCall)] + private static extern object? InternalGetDependent(IntPtr dependentHandle); + + [MethodImpl(MethodImplOptions.InternalCall)] + private static extern object? InternalGetTargetAndDependent(IntPtr dependentHandle, out object? dependent); + + [MethodImpl(MethodImplOptions.InternalCall)] + private static extern void InternalSetDependent(IntPtr dependentHandle, object? dependent); + + [MethodImpl(MethodImplOptions.InternalCall)] + private static extern void InternalSetTargetToNull(IntPtr dependentHandle); + + [MethodImpl(MethodImplOptions.InternalCall)] + private static extern void InternalFree(IntPtr dependentHandle); + } +} diff --git a/src/coreclr/System.Private.CoreLib/src/System/Runtime/InteropServices/GCHandle.CoreCLR.cs b/src/coreclr/System.Private.CoreLib/src/System/Runtime/InteropServices/GCHandle.CoreCLR.cs index 8cfaff78e938b..fa342d4db706a 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Runtime/InteropServices/GCHandle.CoreCLR.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Runtime/InteropServices/GCHandle.CoreCLR.cs @@ -22,7 +22,7 @@ public partial struct GCHandle internal static extern object? InternalGet(IntPtr handle); #else internal static unsafe object? InternalGet(IntPtr handle) => - Unsafe.As(ref *(IntPtr*)handle); + Unsafe.As(ref *(IntPtr*)(nint)handle); #endif [MethodImpl(MethodImplOptions.InternalCall)] diff --git a/src/coreclr/vm/comdependenthandle.cpp b/src/coreclr/vm/comdependenthandle.cpp index e261f16064a6c..b222190810233 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 7c80d0bebe4a4..06786e6257746 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 5cc1d5827fb6f..f77dc75c80b5c 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) diff --git a/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/ConditionalWeakTable.cs b/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/ConditionalWeakTable.cs index b3c72604977b6..88b8187f3dfd2 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/ConditionalWeakTable.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/ConditionalWeakTable.cs @@ -515,6 +515,7 @@ internal bool TryGetValueWorker(TKey key, [MaybeNullWhen(false)] out TValue valu /// Returns -1 if not found (if key expires during FindEntry, this can be treated as "not found."). /// Must hold _lock, or be prepared to retry the search while holding _lock. /// + /// This method requires to be on the stack to be properly tracked. internal int FindEntry(TKey key, out object? value) { Debug.Assert(key != null); // Key already validated as non-null. @@ -523,14 +524,15 @@ internal int FindEntry(TKey key, out object? value) int bucket = hashCode & (_buckets.Length - 1); for (int entriesIndex = Volatile.Read(ref _buckets[bucket]); entriesIndex != -1; entriesIndex = _entries[entriesIndex].Next) { - if (_entries[entriesIndex].HashCode == hashCode && _entries[entriesIndex].depHnd.GetPrimaryAndSecondary(out value) == key) + if (_entries[entriesIndex].HashCode == hashCode && _entries[entriesIndex].depHnd.UnsafeGetTargetAndDependent(out value) == key) { - GC.KeepAlive(this); // ensure we don't get finalized while accessing DependentHandles. + GC.KeepAlive(this); // Ensure we don't get finalized while accessing DependentHandle + return entriesIndex; } } - GC.KeepAlive(this); // ensure we don't get finalized while accessing DependentHandles. + GC.KeepAlive(this); // Ensure we don't get finalized while accessing DependentHandle value = null; return -1; } @@ -540,8 +542,9 @@ internal bool TryGetEntry(int index, [NotNullWhen(true)] out TKey? key, [MaybeNu { if (index < _entries.Length) { - object? oKey = _entries[index].depHnd.GetPrimaryAndSecondary(out object? oValue); - GC.KeepAlive(this); // ensure we don't get finalized while accessing DependentHandles. + object? oKey = _entries[index].depHnd.UnsafeGetTargetAndDependent(out object? oValue); + + GC.KeepAlive(this); // Ensure we don't get finalized while accessing DependentHandle if (oKey != null) { @@ -592,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.SetPrimary(null); + entry.depHnd.UnsafeSetTargetToNull(); } internal void UpdateValue(int entryIndex, TValue newValue) @@ -602,7 +605,7 @@ internal void UpdateValue(int entryIndex, TValue newValue) VerifyIntegrity(); _invalid = true; - _entries[entryIndex].depHnd.SetSecondary(newValue); + _entries[entryIndex].depHnd.UnsafeSetDependent(newValue); _invalid = false; } @@ -634,7 +637,7 @@ internal Container Resize() break; } - if (entry.depHnd.IsAllocated && entry.depHnd.GetPrimary() is null) + if (entry.depHnd.IsAllocated && entry.depHnd.UnsafeGetTarget() is null) { // the entry has expired hasExpiredEntries = true; @@ -699,7 +702,7 @@ internal Container Resize(int newSize) DependentHandle depHnd = oldEntry.depHnd; if (hashCode != -1 && depHnd.IsAllocated) { - if (depHnd.GetPrimary() != null) + if (depHnd.UnsafeGetTarget() is not null) { ref Entry newEntry = ref newEntries[newEntriesIndex]; @@ -795,7 +798,7 @@ private void VerifyIntegrity() // another container, removed entries are not, therefore this container must free them. if (_oldKeepAlive is null || entries[entriesIndex].HashCode == -1) { - entries[entriesIndex].depHnd.Free(); + entries[entriesIndex].depHnd.Dispose(); } } } diff --git a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/GCHandle.cs b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/GCHandle.cs index 78aa52682e417..579633db55caf 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/GCHandle.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/GCHandle.cs @@ -142,7 +142,7 @@ public IntPtr AddrOfPinnedObject() } /// Determine whether this handle has been allocated or not. - public bool IsAllocated => _handle != IntPtr.Zero; + public bool IsAllocated => (nint)_handle != 0; /// /// Used to create a GCHandle from an int. This is intended to @@ -165,9 +165,9 @@ public static GCHandle FromIntPtr(IntPtr value) public override bool Equals([NotNullWhen(true)] object? o) => o is GCHandle && _handle == ((GCHandle)o)._handle; - public static bool operator ==(GCHandle a, GCHandle b) => a._handle == b._handle; + public static bool operator ==(GCHandle a, GCHandle b) => (nint)a._handle == (nint)b._handle; - public static bool operator !=(GCHandle a, GCHandle b) => a._handle != b._handle; + public static bool operator !=(GCHandle a, GCHandle b) => (nint)a._handle != (nint)b._handle; [MethodImpl(MethodImplOptions.AggressiveInlining)] private static IntPtr GetHandleValue(IntPtr handle) => new IntPtr((nint)handle & ~(nint)1); // Remove Pin flag @@ -179,7 +179,7 @@ public static GCHandle FromIntPtr(IntPtr value) private static void ThrowIfInvalid(IntPtr handle) { // Check if the handle was never initialized or was freed. - if (handle == IntPtr.Zero) + if ((nint)handle == 0) { ThrowHelper.ThrowInvalidOperationException_HandleIsNotInitialized(); } diff --git a/src/libraries/System.Runtime/ref/System.Runtime.cs b/src/libraries/System.Runtime/ref/System.Runtime.cs index 4f9afb90503f3..47e45170b40c9 100644 --- a/src/libraries/System.Runtime/ref/System.Runtime.cs +++ b/src/libraries/System.Runtime/ref/System.Runtime.cs @@ -9593,6 +9593,17 @@ public sealed partial class AssemblyTargetedPatchBandAttribute : System.Attribut public AssemblyTargetedPatchBandAttribute(string targetedPatchBand) { } public string TargetedPatchBand { get { throw null; } } } + public partial struct DependentHandle : System.IDisposable + { + private object _dummy; + private int _dummyPrimitive; + 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; } set { } } + public (object? Target, object? Dependent) TargetAndDependent { get { throw null; } } + public void Dispose() { } + } public enum GCLargeObjectHeapCompactionMode { Default = 1, diff --git a/src/libraries/System.Runtime/tests/System.Runtime.Tests.csproj b/src/libraries/System.Runtime/tests/System.Runtime.Tests.csproj index db3b3dcd1b5da..780fcbfc16145 100644 --- a/src/libraries/System.Runtime/tests/System.Runtime.Tests.csproj +++ b/src/libraries/System.Runtime/tests/System.Runtime.Tests.csproj @@ -215,6 +215,7 @@ + diff --git a/src/libraries/System.Runtime/tests/System/Runtime/DependentHandleTests.cs b/src/libraries/System.Runtime/tests/System/Runtime/DependentHandleTests.cs new file mode 100644 index 0000000000000..2ee4eca51d48f --- /dev/null +++ b/src/libraries/System.Runtime/tests/System/Runtime/DependentHandleTests.cs @@ -0,0 +1,334 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Runtime.CompilerServices; +using Xunit; + +namespace System.Runtime.Tests +{ + // NOTE: DependentHandle is already heavily tested indirectly through ConditionalWeakTable<,>. + // This class contains some specific tests for APIs that are only relevant when used directly. + public class DependentHandleTests + { + [Fact] + public void GetNullTarget() + { + object target = new(); + DependentHandle handle = new(null, null); + + Assert.True(handle.IsAllocated); + Assert.Null(handle.Target); + Assert.Null(handle.Dependent); + + handle.Dispose(); + } + + [Fact] + public void GetNotNullTarget() + { + object target = new(); + DependentHandle handle = new(target, null); + + // A handle with a set target and no dependent is valid + Assert.True(handle.IsAllocated); + Assert.Same(target, handle.Target); + Assert.Null(handle.Dependent); + + 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() + { + object target = new(), dependent = new(); + DependentHandle handle = new(target, null); + + // The target can be retrieved correctly + Assert.True(handle.IsAllocated); + Assert.Same(target, handle.Target); + Assert.Null(handle.Dependent); + + handle.Dependent = dependent; + + // The dependent can also be retrieved correctly + Assert.Same(target, handle.Target); + Assert.Same(dependent, handle.Dependent); + + handle.Dispose(); + } + + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsPreciseGcSupported))] + public void TargetKeepsDependentAlive() + { + [MethodImpl(MethodImplOptions.NoInlining)] + static DependentHandle Initialize(out object target, out WeakReference weakDependent) + { + target = new object(); + + object dependent = new(); + + weakDependent = new WeakReference(dependent); + + return new DependentHandle(target, dependent); + } + + DependentHandle handle = Initialize(out object target, out WeakReference dependent); + + GC.Collect(); + + // The dependent has to still be alive as the target has a strong reference + Assert.Same(target, handle.Target); + Assert.True(dependent.IsAlive); + Assert.Same(dependent.Target, handle.Dependent); + + GC.KeepAlive(target); + + handle.Dispose(); + } + + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsPreciseGcSupported))] + public void DependentDoesNotKeepTargetAlive() + { + [MethodImpl(MethodImplOptions.NoInlining)] + static DependentHandle Initialize(out WeakReference weakTarget, out object dependent) + { + dependent = new object(); + + object target = new(); + + weakTarget = new WeakReference(target); + + return new DependentHandle(target, dependent); + } + + DependentHandle handle = Initialize(out WeakReference target, out object dependent); + + GC.Collect(); + + // The target has to be collected, as there were no strong references to it + Assert.Null(handle.Target); + Assert.False(target.IsAlive); + + GC.KeepAlive(target); + + handle.Dispose(); + } + + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsPreciseGcSupported))] + public void DependentIsCollectedOnTargetNotReachable() + { + [MethodImpl(MethodImplOptions.NoInlining)] + static DependentHandle Initialize(out WeakReference weakTarget, out WeakReference weakDependent) + { + object target = new(), dependent = new(); + + weakTarget = new WeakReference(target); + weakDependent = new WeakReference(dependent); + + return new DependentHandle(target, dependent); + } + + DependentHandle handle = Initialize(out WeakReference target, out WeakReference dependent); + + GC.Collect(); + + // Both target and dependent have to be collected, as there were no strong references to either + Assert.Null(handle.Target); + Assert.Null(handle.Dependent); + Assert.False(target.IsAlive); + Assert.False(dependent.IsAlive); + + handle.Dispose(); + } + + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsPreciseGcSupported))] + public void DependentIsCollectedOnTargetNotReachable_EvenWithReferenceCycles() + { + [MethodImpl(MethodImplOptions.NoInlining)] + static DependentHandle Initialize(out WeakReference weakTarget, out WeakReference weakDependent) + { + object target = new(); + ObjectWithReference dependent = new() { Reference = target }; + + weakTarget = new WeakReference(target); + weakDependent = new WeakReference(dependent); + + return new DependentHandle(target, dependent); + } + + DependentHandle handle = Initialize(out WeakReference target, out WeakReference dependent); + + GC.Collect(); + + // Both target and dependent have to be collected, as there were no strong references to either. + // The fact that the dependent has a strong reference back to the target should not affect this. + Assert.Null(handle.Target); + Assert.Null(handle.Dependent); + Assert.False(target.IsAlive); + Assert.False(dependent.IsAlive); + + handle.Dispose(); + } + + private sealed class ObjectWithReference + { + public object Reference; + } + + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsPreciseGcSupported))] + public void DependentIsCollectedAfterTargetIsSetToNull() + { + [MethodImpl(MethodImplOptions.NoInlining)] + static DependentHandle Initialize(out object target, out WeakReference weakDependent) + { + target = new(); + + object dependent = new(); + + weakDependent = new WeakReference(dependent); + + return new DependentHandle(target, dependent); + } + + DependentHandle handle = Initialize(out object target, out WeakReference dependent); + + handle.Target = null; + + GC.Collect(); + + // After calling StopTracking, the dependent is collected even if + // target is still alive and the handle itself has not been disposed + Assert.True(handle.IsAllocated); + Assert.Null(handle.Target); + Assert.Null(handle.Dependent); + Assert.False(dependent.IsAlive); + + GC.KeepAlive(target); + + handle.Dispose(); + } + + [Fact] + public void GetTarget_ThrowsInvalidOperationException() + { + Assert.Throws(() => default(DependentHandle).Target); + } + + [Fact] + public void GetDependent_ThrowsInvalidOperationException() + { + Assert.Throws(() => default(DependentHandle).Dependent); + } + + [Fact] + public void SetTarget_NotAllocated_ThrowsInvalidOperationException() + { + Assert.Throws(() => + { + DependentHandle handle = default; + handle.Target = new(); + }); + } + + [Fact] + public void SetTarget_NotNullObject_ThrowsInvalidOperationException() + { + Assert.Throws(() => + { + DependentHandle handle = default; + + try + { + handle.Target = new(); + } + finally + { + handle.Dispose(); + } + }); + } + + [Fact] + public void SetDependent_ThrowsInvalidOperationException() + { + Assert.Throws(() => + { + DependentHandle handle = default; + handle.Dependent = new(); + }); + } + + [Fact] + public void Dispose_RepeatedCallsAreFine() + { + object target = new(), dependent = new(); + DependentHandle handle = new(target, dependent); + + Assert.True(handle.IsAllocated); + + handle.Dispose(); + + Assert.False(handle.IsAllocated); + + handle.Dispose(); + + Assert.False(handle.IsAllocated); + + handle.Dispose(); + handle.Dispose(); + handle.Dispose(); + + Assert.False(handle.IsAllocated); + } + + [Fact] + public void Dispose_ValidOnDefault() + { + DependentHandle handle = default; + Assert.False(handle.IsAllocated); + handle.Dispose(); + } + } +} diff --git a/src/mono/System.Private.CoreLib/System.Private.CoreLib.csproj b/src/mono/System.Private.CoreLib/System.Private.CoreLib.csproj index 803d83464ea12..21d6492b7189f 100644 --- a/src/mono/System.Private.CoreLib/System.Private.CoreLib.csproj +++ b/src/mono/System.Private.CoreLib/System.Private.CoreLib.csproj @@ -241,8 +241,8 @@ + - diff --git a/src/mono/System.Private.CoreLib/src/System/GC.Mono.cs b/src/mono/System.Private.CoreLib/src/System/GC.Mono.cs index ee874822c563c..a92ebab49920d 100644 --- a/src/mono/System.Private.CoreLib/src/System/GC.Mono.cs +++ b/src/mono/System.Private.CoreLib/src/System/GC.Mono.cs @@ -1,6 +1,7 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System.Runtime; using System.Runtime.CompilerServices; using Internal.Runtime.CompilerServices; using System.Diagnostics.Tracing; diff --git a/src/mono/System.Private.CoreLib/src/System/Runtime/CompilerServices/DependentHandle.cs b/src/mono/System.Private.CoreLib/src/System/Runtime/CompilerServices/DependentHandle.cs deleted file mode 100644 index 1b976a5906afa..0000000000000 --- a/src/mono/System.Private.CoreLib/src/System/Runtime/CompilerServices/DependentHandle.cs +++ /dev/null @@ -1,73 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -namespace System.Runtime.CompilerServices -{ - internal struct Ephemeron - { - public object? key; - public object? value; - } - - // - // Instead of dependent handles, mono uses arrays of Ephemeron objects. - // - internal struct DependentHandle - { - private Ephemeron[] data; - - public DependentHandle(object? primary, object? secondary) - { - data = new Ephemeron[1]; - data[0].key = primary; - data[0].value = secondary; - GC.register_ephemeron_array(data); - } - - public bool IsAllocated => data != null; - - // Getting the secondary object is more expensive than getting the first so - // we provide a separate primary-only accessor for those times we only want the - // primary. - public object? GetPrimary() - { - if (!IsAllocated) - throw new NotSupportedException(); - if (data[0].key == GC.EPHEMERON_TOMBSTONE) - return null; - return data[0].key; - } - - public object? GetPrimaryAndSecondary(out object? secondary) - { - if (!IsAllocated) - throw new NotSupportedException(); - if (data[0].key == GC.EPHEMERON_TOMBSTONE) - { - secondary = null; - return null; - } - secondary = data[0].value; - return data[0].key; - } - - public void SetPrimary(object? primary) - { - if (!IsAllocated) - throw new NotSupportedException(); - data[0].key = primary; - } - - public void SetSecondary(object? secondary) - { - if (!IsAllocated) - throw new NotSupportedException(); - data[0].value = secondary; - } - - public void Free() - { - data = null!; - } - } -} diff --git a/src/mono/System.Private.CoreLib/src/System/Runtime/DependentHandle.cs b/src/mono/System.Private.CoreLib/src/System/Runtime/DependentHandle.cs new file mode 100644 index 0000000000000..8cfd9b013e52c --- /dev/null +++ b/src/mono/System.Private.CoreLib/src/System/Runtime/DependentHandle.cs @@ -0,0 +1,155 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +namespace System.Runtime +{ + internal struct Ephemeron + { + public object? Key; + public object? Value; + } + + // + // Instead of dependent handles, mono uses arrays of Ephemeron objects. + // + public struct DependentHandle : IDisposable + { + private Ephemeron[]? _data; + + public DependentHandle(object? target, object? dependent) + { + _data = new Ephemeron[1]; + _data[0].Key = target; + _data[0].Value = dependent; + GC.register_ephemeron_array(_data); + } + + public bool IsAllocated => _data is not null; + + public object? Target + { + get => UnsafeGetTarget(); + set + { + Ephemeron[]? data = _data; + + if (data is null || value is not null) + { + ThrowHelper.ThrowInvalidOperationException(); + + return; + } + + data[0].Key = null; + } + } + + public object? Dependent + { + get => UnsafeGetDependent(); + set => UnsafeSetDependent(value); + } + + public (object? Target, object? Dependent) TargetAndDependent + { + get + { + object? target = UnsafeGetTargetAndDependent(out object? dependent); + + return (target, dependent); + } + } + + internal object? UnsafeGetTarget() + { + Ephemeron[]? data = _data; + + if (data is null) + { + ThrowHelper.ThrowInvalidOperationException(); + + return default; + } + + object? key = data[0].Key; + + return key != GC.EPHEMERON_TOMBSTONE ? key : null; + } + + internal object? UnsafeGetDependent() + { + Ephemeron[]? data = _data; + + if (data is null) + { + ThrowHelper.ThrowInvalidOperationException(); + + return default; + } + + Ephemeron e = data[0]; + + return e.Key != GC.EPHEMERON_TOMBSTONE && e.Key is not null ? e.Value : null; + } + + internal object? UnsafeGetTargetAndDependent(out object? dependent) + { + Ephemeron[]? data = _data; + + if (data is null) + { + ThrowHelper.ThrowInvalidOperationException(); + + dependent = null; + + return null; + } + + Ephemeron e = data[0]; + + if (e.Key != GC.EPHEMERON_TOMBSTONE && e.Key is not null) + { + dependent = e.Value; + + return e.Key; + } + + dependent = null; + + return null; + } + + internal void UnsafeSetTargetToNull() + { + Ephemeron[]? data = _data; + + if (data is null) + { + ThrowHelper.ThrowInvalidOperationException(); + + return; + } + + data[0].Key = null; + } + + internal void UnsafeSetDependent(object? dependent) + { + Ephemeron[]? data = _data; + + if (data is null) + { + ThrowHelper.ThrowInvalidOperationException(); + + return; + } + + data[0].Value = dependent; + } + + public void Dispose() + { + _data = null; + } + } +}