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

Fix InvalidCastExceptions from RCW reuse #833

Merged
merged 7 commits into from
May 4, 2021
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
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
24 changes: 16 additions & 8 deletions src/Tests/UnitTest/TestComponentCSharp_Tests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2221,24 +2221,32 @@ static Object MakeObject()

static void TestImports()
{
var (initializeWithWindow, windowNative) = MakeImports();

GC.Collect();
GC.WaitForPendingFinalizers();
var (initializeWithWindow, windowNative) = MakeImports();

GCCollect();

var hwnd = new IntPtr(0x12345678);
initializeWithWindow.Initialize(hwnd);
Assert.Equal(windowNative.WindowHandle, hwnd);
}

static void GCCollect()
manodasanW marked this conversation as resolved.
Show resolved Hide resolved
{
// Require multiple GC collects due to
// the final release is not done immediately.
for(int idx = 0; idx < 3; idx++)
{
GC.Collect();
GC.WaitForPendingFinalizers();
}
}

TestObject();
GC.Collect();
GC.WaitForPendingFinalizers();
GCCollect();
Assert.Equal(0, ComImports.NumObjects);

TestImports();
GC.Collect();
GC.WaitForPendingFinalizers();
GCCollect();
Assert.Equal(0, ComImports.NumObjects);
}

Expand Down
17 changes: 10 additions & 7 deletions src/WinRT.Runtime/ComWrappersSupport.net5.cs
Original file line number Diff line number Diff line change
Expand Up @@ -99,12 +99,12 @@ public static T CreateRcwForComObject<T>(IntPtr ptr)
winrtObj.Resurrect();
}

return rcw switch
{
ABI.System.Nullable<string> ns => (T)(object) ns.Value,
ABI.System.Nullable<Type> nt => (T)(object) nt.Value,
_ => (T) rcw
};
return rcw switch
{
ABI.System.Nullable<string> ns => (T)(object)ns.Value,
ABI.System.Nullable<Type> nt => (T)(object)nt.Value,
_ => (T)rcw
};
}

public static bool TryUnwrapObject(object o, out IObjectReference objRef)
Expand Down Expand Up @@ -143,6 +143,7 @@ public static object TryRegisterObjectForInterface(object obj, IntPtr thisPtr)
if (target is IWinRTObject winrtObj)
{
winrtObj.Resurrect();
winrtObj.NativeObject.CleanupRCW = true;
}
return rcw;
}
Expand Down Expand Up @@ -205,7 +206,8 @@ public unsafe static void Init(
IntPtr inner,
out IObjectReference objRef)
{
objRef = ComWrappersSupport.GetObjectReferenceForInterface(isAggregation ? inner : newInstance);
objRef = ComWrappersSupport.GetObjectReferenceForInterface(isAggregation ? inner : newInstance);
objRef.CleanupRCW = true;

IntPtr referenceTracker;
{
Expand Down Expand Up @@ -464,6 +466,7 @@ protected override object CreateObject(IntPtr externalComObject, CreateObjectFla
// on destruction as the CLR would do it.
winrtObj.NativeObject.ReleaseFromTrackerSource();
winrtObj.NativeObject.PreventReleaseFromTrackerSourceOnDispose = true;
winrtObj.NativeObject.CleanupRCW = true;
}

return obj;
Expand Down
28 changes: 26 additions & 2 deletions src/WinRT.Runtime/ObjectReference.cs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ public abstract class IObjectReference : IDisposable
protected bool disposed;
private readonly IntPtr _thisPtr;
private object _disposedLock = new object();
private IntPtr _referenceTrackerPtr;
private IntPtr _referenceTrackerPtr;
private int RCWCleanupCounter = 2;
manodasanW marked this conversation as resolved.
Show resolved Hide resolved

public IntPtr ThisPtr
{
Expand Down Expand Up @@ -74,6 +75,8 @@ internal unsafe IReferenceTrackerVftbl ReferenceTracker
}
}

internal bool CleanupRCW { get; set; }
manodasanW marked this conversation as resolved.
Show resolved Hide resolved

protected unsafe IUnknownVftbl VftblIUnknown
{
get
Expand Down Expand Up @@ -195,7 +198,14 @@ protected void ThrowIfDisposed()
}

public void Dispose()
{
{
// If this is the object reference associated with the RCW,
// defer dispose to after the RCW has been finalized for .NET 5.
if (CleanupRCW)
{
return;
}

Dispose(true);
GC.SuppressFinalize(this);
}
Expand All @@ -208,6 +218,19 @@ protected virtual void Dispose(bool disposing)
{
return;
}

// If the object reference is associated with the RCW, we need to
// defer the final release on the ThisPtr until after the RCW has been
// finalized and it has been removed from the ComWrappers cache.
// In .NET 6, there will be a new API for us to use, but until then
// in .NET 5, we defer the finalization of this object until it
// has reached Gen 2 by reregistering for finalization.
if(CleanupRCW && --RCWCleanupCounter >= 0)
{
GC.ReRegisterForFinalize(this);
return;
}

#if DEBUG
if (BreakOnDispose && System.Diagnostics.Debugger.IsAttached)
{
Expand All @@ -234,6 +257,7 @@ internal bool Resurrect()
return false;
}
disposed = false;
RCWCleanupCounter = 2;
ResurrectTrackerSource();
AddRef();
GC.ReRegisterForFinalize(this);
Expand Down