-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
Changes from 23 commits
4b6a696
079818b
1dafd25
30636bc
5876f7b
9d7636c
109e7a6
628f8c3
9ad5897
bf57457
a917cff
d42dca3
b089f32
333ae9f
99f299e
a119ad6
3f1de5a
c4624ec
92787ab
22b7507
87055ed
1a89e73
e709dfb
e51f50c
06d66e3
9bbbdb5
90dd2bb
ea7c3ee
a5a8a98
230a3e6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,26 @@ | ||
// 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.InternalCall)] | ||
internal static extern IntPtr ObjectToComWeakRef(object target, out long wrapperId); | ||
} | ||
} | ||
#endif |
This file was deleted.
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -453,20 +453,6 @@ typedef enum | |
* | ||
*/ | ||
HNDTYPE_SIZEDREF = 8, | ||
|
||
/* | ||
* NATIVE WEAK HANDLES | ||
* | ||
* Native weak reference handles hold two different types of weak handles to any | ||
* RCW with an underlying COM object that implements IWeakReferenceSource. The | ||
* object reference itself is a short weak handle to the RCW. In addition an | ||
* IWeakReference* to the underlying COM object is stored, allowing the handle | ||
* to create a new RCW if the existing RCW is collected. This ensures that any | ||
* code holding onto a native weak reference can always access an RCW to the | ||
* underlying COM object as long as it has not been released by all of its strong | ||
* references. | ||
*/ | ||
HNDTYPE_WEAK_NATIVE_COM = 9 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. GC/VM interface change. cc @dotnet/gc There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this change looks ok, assuming this enum is not shared between clrgc and coreclr? We need to ensure that latest clrgc continues to work with .net 7 runtime. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Everything in gcinterface.h is shared between clrgc and coreclr.
The changes under src/coreclr/gc need to be reverted if you would like to maintain this invariant. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Is this a temporary requirement? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We have added a new method to IGCHeap as part of the frozen literals work, that does not prevent new clrgc + old coreclr from working. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ok. Considering how much GC and VM are coupled, it feels like compat requirement like this is not sustainable for long. Hopefully this is just for the short term. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't see why that's not sustainable. we just don't delete things from the interface. yes, it'd be a bit nice to be able to delete things but considering the benefit it seems like the right tradeoff. |
||
} HandleType; | ||
|
||
typedef enum | ||
|
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.