Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Switch CoreCLR WeakReference to unified managed implementation #77196

Merged
merged 30 commits into from
Nov 1, 2022
Merged
Show file tree
Hide file tree
Changes from 28 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
4b6a696
Delete managed CoreClr WR
VSadov Oct 14, 2022
079818b
all but com works
VSadov Oct 18, 2022
1dafd25
COM tests are passing
VSadov Oct 18, 2022
30636bc
HandleTagBits const for NativeAot
VSadov Oct 19, 2022
5876f7b
Exclusive Set
VSadov Oct 19, 2022
9d7636c
fix
VSadov Oct 19, 2022
109e7a6
Use sign bit
VSadov Oct 19, 2022
628f8c3
Platforms not supporting COM can mask only one bit.
VSadov Oct 19, 2022
9ad5897
new approach
VSadov Oct 21, 2022
bf57457
fix mono build
VSadov Oct 21, 2022
a917cff
check for FEATURE_COMWRAPPERS too
VSadov Oct 21, 2022
d42dca3
stub NativeAOT support (NYI).
VSadov Oct 21, 2022
b089f32
current
VSadov Oct 21, 2022
333ae9f
moved handle tags on the managed side to one location
VSadov Oct 21, 2022
99f299e
Getter optimizations
VSadov Oct 22, 2022
a119ad6
Optimizations for Setter
VSadov Oct 22, 2022
3f1de5a
accessibility of some members
VSadov Oct 22, 2022
c4624ec
ensure identity of the rehydrated RCW
VSadov Oct 29, 2022
92787ab
make ComWeakRefToObject a QCall
VSadov Oct 29, 2022
22b7507
delete unused pWeakReferenceOfTCanonMT and pWeakReferenceMT
VSadov Oct 29, 2022
87055ed
byte-aligned
VSadov Oct 29, 2022
1a89e73
cleanup unreachable code
VSadov Oct 29, 2022
e709dfb
renamed WeakReferenceObject::m_Handle -> WeakReferenceObject::m_tagge…
VSadov Oct 29, 2022
e51f50c
Apply suggestions from code review
VSadov Oct 31, 2022
06d66e3
some PR feedback
VSadov Oct 31, 2022
9bbbdb5
GetWeakHandle no longer cares about inlining.
VSadov Oct 31, 2022
90dd2bb
turn ObjectToComWeakRef into a QCall
VSadov Oct 31, 2022
ea7c3ee
revert changes under coreclr\gc
VSadov Oct 31, 2022
a5a8a98
added a note to eventually remove HNDTYPE_WEAK_NATIVE_COM
VSadov Nov 1, 2022
230a3e6
Update src/coreclr/gc/gcinterface.h
VSadov Nov 1, 2022
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,7 @@
<Compile Include="$(BclSourcesRoot)\System\Collections\Generic\Comparer.CoreCLR.cs" />
<Compile Include="$(BclSourcesRoot)\System\Collections\Generic\ComparerHelpers.cs" />
<Compile Include="$(BclSourcesRoot)\System\Collections\Generic\EqualityComparer.CoreCLR.cs" />
<Compile Include="$(BclSourcesRoot)\System\ComAwareWeakReference.CoreCLR.cs" />
<Compile Include="$(BclSourcesRoot)\System\Currency.cs" />
<Compile Include="$(BclSourcesRoot)\System\Delegate.CoreCLR.cs" />
<Compile Include="$(BclSourcesRoot)\System\Diagnostics\Debugger.cs" />
Expand Down Expand Up @@ -244,8 +245,6 @@
<Compile Include="$(BclSourcesRoot)\System\TypeLoadException.CoreCLR.cs" />
<Compile Include="$(BclSourcesRoot)\System\TypeNameParser.cs" />
<Compile Include="$(BclSourcesRoot)\System\ValueType.cs" />
<Compile Include="$(BclSourcesRoot)\System\WeakReference.CoreCLR.cs" />
<Compile Include="$(BclSourcesRoot)\System\WeakReference.T.CoreCLR.cs" />
</ItemGroup>
<ItemGroup Condition="'$(FeatureComWrappers)' == 'true'">
<Compile Include="$(BclSourcesRoot)\System\Runtime\InteropServices\ComWrappers.cs" />
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
// 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 System.Runtime.InteropServices;

#if FEATURE_COMINTEROP || FEATURE_COMWRAPPERS
namespace System
{
internal sealed partial class ComAwareWeakReference
{
[LibraryImport(RuntimeHelpers.QCall, EntryPoint = "ComWeakRefToObject")]
private static partial void ComWeakRefToObject(IntPtr pComWeakRef, long wrapperId, ObjectHandleOnStack retRcw);

internal static object? ComWeakRefToObject(IntPtr pComWeakRef, long wrapperId)
{
object? retRcw = null;
ComWeakRefToObject(pComWeakRef, wrapperId, ObjectHandleOnStack.Create(ref retRcw));
return retRcw;
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
internal static unsafe bool PossiblyComObject(object target)
{
// see: syncblk.h
const int IS_HASHCODE_BIT_NUMBER = 26;
const int BIT_SBLK_IS_HASHCODE = 1 << IS_HASHCODE_BIT_NUMBER;
const int BIT_SBLK_IS_HASH_OR_SYNCBLKINDEX = 0x08000000;

fixed (byte* pRawData = &target.GetRawData())
{
// The header is 4 bytes before MT field on all architectures
int header = *(int*)(pRawData - sizeof(IntPtr) - sizeof(int));
// common case: target does not have a syncblock, so there is no interop info
return (header & (BIT_SBLK_IS_HASH_OR_SYNCBLKINDEX | BIT_SBLK_IS_HASHCODE)) == BIT_SBLK_IS_HASH_OR_SYNCBLKINDEX;
}
}

[MethodImpl(MethodImplOptions.InternalCall)]
internal static extern bool HasInteropInfo(object target);

[LibraryImport(RuntimeHelpers.QCall, EntryPoint = "ObjectToComWeakRef")]
private static partial IntPtr ObjectToComWeakRef(ObjectHandleOnStack retRcw, out long wrapperId);

internal static nint ObjectToComWeakRef(object target, out long wrapperId)
{
if (HasInteropInfo(target))
{
return ObjectToComWeakRef(ObjectHandleOnStack.Create(ref target), out wrapperId);
}

wrapperId = 0;
return IntPtr.Zero;
}
}
}
#endif
Original file line number Diff line number Diff line change
Expand Up @@ -305,7 +305,7 @@ public static void KeepAlive(object? obj)
//
public static int GetGeneration(WeakReference wo)
{
int result = GetGenerationWR(wo.m_handle);
int result = GetGenerationWR(wo.WeakHandle);
KeepAlive(wo);
return result;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ namespace System.Runtime.InteropServices
public partial struct GCHandle
{
[MethodImpl(MethodImplOptions.InternalCall)]
private static extern IntPtr InternalAlloc(object? value, GCHandleType type);
internal static extern IntPtr InternalAlloc(object? value, GCHandleType type);

[MethodImpl(MethodImplOptions.InternalCall)]
internal static extern void InternalFree(IntPtr handle);
Expand Down

This file was deleted.

This file was deleted.

4 changes: 0 additions & 4 deletions src/coreclr/debug/daccess/daccess.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7858,10 +7858,6 @@ void CALLBACK DacHandleWalker::EnumCallbackSOS(PTR_UNCHECKED_OBJECTREF handle, u
data.Type = param->Type;
if (param->Type == HNDTYPE_DEPENDENT)
data.Secondary = GetDependentHandleSecondary(handle.GetAddr()).GetAddr();
#ifdef FEATURE_COMINTEROP
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this mean that SOS no longer has insight into weak references?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Regular weak references did not change from the introspection point of view.

The thing that changed is that there is no need to special case native COM weak handles, since they no longer exist. The implementation uses regular managed objects now and they will be reachable in SOS walks like any other managed object.

else if (param->Type == HNDTYPE_WEAK_NATIVE_COM)
data.Secondary = HndGetHandleExtraInfo(handle.GetAddr());
#endif // FEATURE_COMINTEROP
else
data.Secondary = 0;
data.AppDomain = param->AppDomain;
Expand Down
9 changes: 0 additions & 9 deletions src/coreclr/debug/daccess/dacdbiimpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7601,10 +7601,6 @@ UINT32 DacRefWalker::GetHandleWalkerMask()
if ((mHandleMask & CorHandleWeakRefCount) || (mHandleMask & CorHandleStrongRefCount))
result |= (1 << HNDTYPE_REFCOUNTED);
#endif // FEATURE_COMINTEROP || FEATURE_COMWRAPPERS || FEATURE_OBJCMARSHAL
#if defined(FEATURE_COMINTEROP) || defined(FEATURE_COMWRAPPERS)
if (mHandleMask & CorHandleWeakNativeCom)
result |= (1 << HNDTYPE_WEAK_NATIVE_COM);
#endif // FEATURE_COMINTEROP || FEATURE_COMWRAPPERS

if (mHandleMask & CorHandleStrongDependent)
result |= (1 << HNDTYPE_DEPENDENT);
Expand Down Expand Up @@ -7778,11 +7774,6 @@ void CALLBACK DacHandleWalker::EnumCallbackDac(PTR_UNCHECKED_OBJECTREF handle, u
data.i64ExtraData = refCnt;
break;
#endif // FEATURE_COMINTEROP || FEATURE_COMWRAPPERS || FEATURE_OBJCMARSHAL
#if defined(FEATURE_COMINTEROP) || defined(FEATURE_COMWRAPPERS)
case HNDTYPE_WEAK_NATIVE_COM:
data.dwType = (DWORD)CorHandleWeakNativeCom;
break;
#endif // FEATURE_COMINTEROP || FEATURE_COMWRAPPERS

case HNDTYPE_DEPENDENT:
data.dwType = (DWORD)CorHandleStrongDependent;
Expand Down
6 changes: 0 additions & 6 deletions src/coreclr/debug/daccess/request.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3210,9 +3210,6 @@ HRESULT ClrDataAccess::GetHandleEnum(ISOSHandleEnum **ppHandleEnum)
#if defined(FEATURE_COMINTEROP) || defined(FEATURE_COMWRAPPERS) || defined(FEATURE_OBJCMARSHAL)
HNDTYPE_REFCOUNTED,
#endif // FEATURE_COMINTEROP || FEATURE_COMWRAPPERS || FEATURE_OBJCMARSHAL
#if defined(FEATURE_COMINTEROP) || defined(FEATURE_COMWRAPPERS)
HNDTYPE_WEAK_NATIVE_COM
#endif // FEATURE_COMINTEROP
};

return GetHandleEnumForTypes(types, ARRAY_SIZE(types), ppHandleEnum);
Expand Down Expand Up @@ -3251,9 +3248,6 @@ HRESULT ClrDataAccess::GetHandleEnumForGC(unsigned int gen, ISOSHandleEnum **ppH
#if defined(FEATURE_COMINTEROP) || defined(FEATURE_COMWRAPPERS) || defined(FEATURE_OBJCMARSHAL)
HNDTYPE_REFCOUNTED,
#endif // FEATURE_COMINTEROP || FEATURE_COMWRAPPERS || FEATURE_OBJCMARSHAL
#if defined(FEATURE_COMINTEROP) || defined(FEATURE_COMWRAPPERS)
HNDTYPE_WEAK_NATIVE_COM
#endif // FEATURE_COMINTEROP || FEATURE_COMWRAPPERS
};

DacHandleWalker *walker = new DacHandleWalker();
Expand Down
3 changes: 3 additions & 0 deletions src/coreclr/inc/dacvars.h
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,9 @@ DEFINE_DACVAR(UNKNOWN_POINTER_TYPE, dac__g_pThreadClass, ::g_pThreadClass)
DEFINE_DACVAR(UNKNOWN_POINTER_TYPE, dac__g_pPredefinedArrayTypes, ::g_pPredefinedArrayTypes)
DEFINE_DACVAR(UNKNOWN_POINTER_TYPE, dac__g_TypedReferenceMT, ::g_TypedReferenceMT)

DEFINE_DACVAR(UNKNOWN_POINTER_TYPE, dac__g_pWeakReferenceClass, ::g_pWeakReferenceClass)
DEFINE_DACVAR(UNKNOWN_POINTER_TYPE, dac__g_pWeakReferenceOfTClass, ::g_pWeakReferenceOfTClass)

#ifdef FEATURE_COMINTEROP
DEFINE_DACVAR(UNKNOWN_POINTER_TYPE, dac__g_pBaseCOMObject, ::g_pBaseCOMObject)
#endif
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/nativeaot/Runtime/ObjectLayout.h
Original file line number Diff line number Diff line change
Expand Up @@ -136,5 +136,5 @@ static uintptr_t const MAX_STRING_LENGTH = 0x3FFFFFDF;
class WeakReference : public Object
{
public:
uintptr_t m_HandleAndKind;
uintptr_t m_taggedHandle;
};
11 changes: 8 additions & 3 deletions src/coreclr/nativeaot/Runtime/gcrhenv.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1181,10 +1181,15 @@ bool GCToEEInterface::EagerFinalized(Object* obj)
// Managed code should not be running.
ASSERT(GCHeapUtilities::GetGCHeap()->IsGCInProgressHelper());

// the lowermost 1 bit is reserved for storing additional info about the handle
const uintptr_t HandleTagBits = 1;

WeakReference* weakRefObj = (WeakReference*)obj;
OBJECTHANDLE handle = (OBJECTHANDLE)(weakRefObj->m_HandleAndKind & ~(uintptr_t)1);
HandleType handleType = (weakRefObj->m_HandleAndKind & 1) ? HandleType::HNDTYPE_WEAK_LONG : HandleType::HNDTYPE_WEAK_SHORT;
weakRefObj->m_HandleAndKind &= (uintptr_t)1;
OBJECTHANDLE handle = (OBJECTHANDLE)(weakRefObj->m_taggedHandle & ~HandleTagBits);
_ASSERTE((weakRefObj->m_taggedHandle & 2) == 0);
HandleType handleType = (weakRefObj->m_taggedHandle & 1) ? HandleType::HNDTYPE_WEAK_LONG : HandleType::HNDTYPE_WEAK_SHORT;
// keep the bit that indicates whether this reference was tracking resurrection, clear the rest.
weakRefObj->m_taggedHandle &= HandleTagBits;
GCHandleUtilities::GetGCHandleManager()->DestroyHandleOfType(handle, handleType);
return true;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,7 @@
<Compile Include="System\Collections\Generic\Comparer.NativeAot.cs" />
<Compile Include="System\Collections\Generic\EqualityComparer.NativeAot.cs" />
<Compile Include="System\Collections\Generic\EqualOnlyComparer.cs" />
<Compile Include="System\ComAwareWeakReference.NativeAot.cs" />
<Compile Include="System\InvokeUtils.cs" />
<Compile Include="System\IO\FileLoadException.NativeAot.cs" />
<Compile Include="System\RuntimeMethodHandle.cs" />
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System;

#if FEATURE_COMINTEROP || FEATURE_COMWRAPPERS
namespace System
{
internal sealed partial class ComAwareWeakReference
{
internal static object? ComWeakRefToObject(IntPtr pComWeakRef, long wrapperId)
{
// NativeAOT support for COM WeakReference is NYI
throw new NotImplementedException();
}

internal static bool PossiblyComObject(object target)
{
// NativeAOT support for COM WeakReference is NYI
return false;
}

internal static IntPtr ObjectToComWeakRef(object target, out long wrapperId)
{
// NativeAOT support for COM WeakReference is NYI
wrapperId = 0;
return 0;
}
}
}
#endif
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,9 @@ public static int GetGeneration(WeakReference wo)
{
// note - this throws an NRE if given a null weak reference. This isn't
// documented, but it's the behavior of Desktop and CoreCLR.
object? obj = RuntimeImports.RhHandleGet(wo.Handle);
object? obj = RuntimeImports.RhHandleGet(wo.WeakHandle);
KeepAlive(wo);

if (obj == null)
{
throw new ArgumentNullException(nameof(wo));
Expand Down
3 changes: 3 additions & 0 deletions src/coreclr/vm/appdomain.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1384,6 +1384,9 @@ void SystemDomain::LoadBaseSystemClasses()

g_pThreadClass = CoreLibBinder::GetClass(CLASS__THREAD);

g_pWeakReferenceClass = CoreLibBinder::GetClass(CLASS__WEAKREFERENCE);
g_pWeakReferenceOfTClass = CoreLibBinder::GetClass(CLASS__WEAKREFERENCEGENERIC);

#ifdef FEATURE_COMINTEROP
if (g_pConfig->IsBuiltInCOMSupported())
{
Expand Down
6 changes: 0 additions & 6 deletions src/coreclr/vm/appdomain.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -1064,12 +1064,6 @@ class BaseDomain
WRAPPER_NO_CONTRACT;
return ::CreateRefcountedHandle(m_handleStore, object);
}

OBJECTHANDLE CreateNativeComWeakHandle(OBJECTREF object, NativeComWeakHandleInfo* pComWeakHandleInfo)
{
WRAPPER_NO_CONTRACT;
return ::CreateNativeComWeakHandle(m_handleStore, object, pComWeakHandleInfo);
}
#endif // FEATURE_COMINTEROP || FEATURE_COMWRAPPERS

OBJECTHANDLE CreateVariableHandle(OBJECTREF object, UINT type)
Expand Down
7 changes: 4 additions & 3 deletions src/coreclr/vm/corelib.h
Original file line number Diff line number Diff line change
Expand Up @@ -939,11 +939,12 @@ DEFINE_METHOD(GC, KEEP_ALIVE, KeepAlive,
DEFINE_METHOD(GC, COLLECT, Collect, SM_RetVoid)
DEFINE_METHOD(GC, WAIT_FOR_PENDING_FINALIZERS, WaitForPendingFinalizers, SM_RetVoid)

DEFINE_CLASS_U(System, WeakReference, WeakReferenceObject)
DEFINE_FIELD_U(m_handle, WeakReferenceObject, m_Handle)
DEFINE_CLASS_U(System, WeakReference, WeakReferenceObject)
DEFINE_FIELD_U(_taggedHandle, WeakReferenceObject, m_taggedHandle)
DEFINE_CLASS(WEAKREFERENCE, System, WeakReference)
DEFINE_CLASS(WEAKREFERENCEGENERIC, System, WeakReference`1)

DEFINE_CLASS_U(Threading, WaitHandle, WaitHandleBase)
DEFINE_CLASS_U(Threading, WaitHandle, WaitHandleBase)
DEFINE_FIELD_U(_waitHandle, WaitHandleBase, m_safeHandle)

DEFINE_CLASS(DEBUGGER, Diagnostics, Debugger)
Expand Down
Loading