Added caching of event registration state to support unsubscribes across garbage collections

src/Tests/ObjectLifetimeTests/ObjectLifetimeTests.cs

@@ -355,10 +355,10 @@ public void BasicTest6b()
 
 
         AutoResetEvent loadedSignal = new AutoResetEvent(false);
-        AutoResetEvent unloadedSignal = new AutoResetEvent(false);
-
-
-        // Bug: _parentRef is still alive https://github.com/microsoft/CsWinRT/issues/897
+        AutoResetEvent unloadedSignal = new AutoResetEvent(false);
+
+
+        [TestMethod]
         public void CycleTest1()
         {
             //Debugger.Launch();
@@ -414,15 +414,14 @@ public void CycleTest1()
                 });
 
             _asyncQueue.Run();
-        }
-
-        //
-        // Cycle from StackPanel to custom element to delegate back to StackPanel.
-        //
-
-
-        //[MethodImplAttribute(MethodImplOptions.NoOptimization | MethodImplOptions.NoInlining)]
-        //Bug:_parentRef is still alive https://github.com/microsoft/CsWinRT/issues/897
+        }
+
+        //
+        // Cycle from StackPanel to custom element to delegate back to StackPanel.
+        //
+
+        [TestMethod]
+        [MethodImplAttribute(MethodImplOptions.NoOptimization | MethodImplOptions.NoInlining)]
         public void CycleTest1b()
         {
             _asyncQueue
@@ -476,15 +475,15 @@ public void CycleTest1b()
                 });
 
             _asyncQueue.Run();
-        }
-
-        //
-        // Cycle from StackPanel to built-in element to delegate on framework event back to StackPanel.
-        //
-
-        
-        //[MethodImplAttribute(MethodImplOptions.NoOptimization | MethodImplOptions.NoInlining)]
-        //Bug:_element1Ref is still alive https://github.com/microsoft/CsWinRT/issues/897
+        }
+
+        //
+        // Cycle from StackPanel to built-in element to delegate on framework event back to StackPanel.
+        //
+
+
+        [TestMethod]
+        [MethodImplAttribute(MethodImplOptions.NoOptimization | MethodImplOptions.NoInlining)]
         public void CycleTest1c()
         {
             _asyncQueue

src/Tests/TestComponentCSharp/Singleton.cpp

@@ -0,0 +1,43 @@
+#include "pch.h"
+#include "Singleton.h"
+#include "Singleton.g.cpp"
+
+using namespace winrt;
+using namespace Windows::Foundation;
+
+namespace winrt::TestComponentCSharp::implementation
+{
+    TestComponentCSharp::ISingleton Singleton::Instance()
+    {
+        struct singleton : winrt::implements<singleton, ISingleton>
+        {
+            int _int{};
+            winrt::event<EventHandler<int32_t>> _intChanged {};
+
+            int32_t IntProperty()
+            {
+                return _int;
+            }
+            void IntProperty(int32_t value)
+            {
+                _int = value;
+                _intChanged(nullptr, _int);
+            }
+            winrt::event_token IntPropertyChanged(EventHandler<int32_t> const& handler)
+            {
+                return _intChanged.add(handler);
+            }
+            void IntPropertyChanged(winrt::event_token const& token) noexcept
+            {
+                _intChanged.remove(token);
+            }
+        };
+        static TestComponentCSharp::ISingleton _singleton = winrt::make<singleton>();
+        return _singleton;
+    }
+
+    void Singleton::Instance(TestComponentCSharp::ISingleton const& value)
+    {
+        throw hresult_not_implemented();
+    }
+}

src/Tests/TestComponentCSharp/Singleton.h

@@ -0,0 +1,19 @@
+#pragma once
+#include "Singleton.g.h"
+
+namespace winrt::TestComponentCSharp::implementation
+{
+    struct Singleton
+    {
+        Singleton() = default;
+
+        static TestComponentCSharp::ISingleton Instance();
+        static void Instance(TestComponentCSharp::ISingleton const& value);
+    };
+}
+namespace winrt::TestComponentCSharp::factory_implementation
+{
+    struct Singleton : SingletonT<Singleton, implementation::Singleton>
+    {
+    };
+}

src/Tests/TestComponentCSharp/TestComponentCSharp.idl

@@ -130,6 +130,17 @@ namespace TestComponentCSharp
         static Int32 NumObjects{ get; };
     }
 
+    interface ISingleton
+    {
+        Int32 IntProperty;
+        event Windows.Foundation.EventHandler<Int32> IntPropertyChanged;
+    }
+
+    static runtimeclass Singleton
+    {
+        static ISingleton Instance;
+    }
+
     [default_interface, gc_pressure(Windows.Foundation.Metadata.GCPressureAmount.High)]
     runtimeclass Class :
         Windows.Foundation.IStringable
@@ -471,7 +482,7 @@ namespace TestComponentCSharp
             static event Windows.Foundation.EventHandler<Int32> WarningEvent;
         }
     }
-
+    
     [contract(Windows.Foundation.UniversalApiContract, 8)]
     interface IWarning1
     {

src/Tests/TestComponentCSharp/TestComponentCSharp.vcxproj

@@ -72,6 +72,7 @@
     </ClInclude>
     <ClInclude Include="NonAgileClass.h" />
     <ClInclude Include="ComImports.h" />
+    <ClInclude Include="Singleton.h" />
     <ClInclude Include="WarningClass.h" />
     <ClInclude Include="WarningStatic.h" />
     <ClInclude Include="Windows.Class.h" />
@@ -88,6 +89,7 @@
     <ClCompile Include="NonAgileClass.cpp" />
     <ClCompile Include="ComImports.cpp" />
     <ClCompile Include="ManualProjectionTestClasses.cpp" />
+    <ClCompile Include="Singleton.cpp" />
     <ClCompile Include="WarningClass.cpp" />
     <ClCompile Include="WarningStatic.cpp" />
     <ClCompile Include="Windows.Class.cpp" />

src/Tests/TestComponentCSharp/TestComponentCSharp.vcxproj.filters

@@ -15,6 +15,7 @@
     <ClCompile Include="ManualProjectionTestClasses.cpp" />
     <ClCompile Include="WarningClass.cpp" />
     <ClCompile Include="WarningStatic.cpp" />
+    <ClCompile Include="Singleton.cpp" />
     <ClCompile Include="Windows.Class.cpp" />
     <ClCompile Include="ABCDEFGHIJKLMNOPQRSTUVQXYZabcdefghijklmnopqrstuvqxyzABCDEFGHIJKLMNOPQRSTUVQXYZabcdefghijklmnopqrstuvqxyzABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz.cpp" />
   </ItemGroup>
@@ -25,6 +26,7 @@
     <ClInclude Include="ManualProjectionTestClasses.h" />
     <ClInclude Include="WarningStatic.h" />
     <ClInclude Include="WarningClass.h" />
+    <ClInclude Include="Singleton.h" />
     <ClInclude Include="Windows.Class.h" />
     <ClInclude Include="ABCDEFGHIJKLMNOPQRSTUVQXYZabcdefghijklmnopqrstuvqxyzABCDEFGHIJKLMNOPQRSTUVQXYZabcdefghijklmnopqrstuvqxyzABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz.h" />
   </ItemGroup>

src/Tests/UnitTest/TestComponentCSharp_Tests.cs

@@ -2461,15 +2461,44 @@ public void TestCovariance()
             Assert.True(TestObject.IterableOfObjectIterablesProperty.SequenceEqual(listOfListOfUris));
         }
 
+        (System.WeakReference<Class>, System.WeakReference<EventHandlerClass>) TestEventDelegateCleanup()
+        {
+            // Both WinRT object and handler class alive.
+            var eventCalled = false;
+            var eventHandlerClass = new EventHandlerClass(() => eventCalled = true);
+            var classInstance = new Class();
+            classInstance.IntPropertyChanged += eventHandlerClass.IntPropertyChanged;
+            GC.Collect(2, GCCollectionMode.Forced, true);
+            GC.WaitForPendingFinalizers();
+            classInstance.IntProperty = 3;
+            Assert.True(eventCalled);
+
+            // No strong reference to handler class, but delegate is still registered on
+            // the WinRT object keeping it alive.
+            eventCalled = false;
+            var weakEventHandlerClass = new System.WeakReference<EventHandlerClass>(eventHandlerClass);
+            eventHandlerClass = null;
+            GC.Collect(2, GCCollectionMode.Forced, true);
+            GC.WaitForPendingFinalizers();
+            classInstance.IntProperty = 3;
+            Assert.True(eventCalled);
+            Assert.True(weakEventHandlerClass.TryGetTarget(out var _));
+
+            // No strong reference to WinRT object.  It should no longer be alive
+            // and should also cause for the event handler class to be no longer alive.
+            var weakClassInstance = new System.WeakReference<Class>(classInstance);
+            classInstance = null;
+            return (weakClassInstance, weakEventHandlerClass);
+        }
+
+        // Ensure that event subscription state is properly cached to enable later unsubscribes
         [Fact]
-        public void TestStaticEventWithGC()
+        public void TestEventSourceCaching()
         {
             bool eventCalled = false;
-            void Class_StaticIntPropertyChanged(object sender, int e)
-            {
-                eventCalled = (e == 3);
-            }
+            void Class_StaticIntPropertyChanged(object sender, int e) => eventCalled = (e == 3);
 
+            // Test static codegen-based EventSource caching
             Class.StaticIntPropertyChanged += Class_StaticIntPropertyChanged;
             GC.Collect(2, GCCollectionMode.Forced, true);
             GC.WaitForPendingFinalizers();
@@ -2481,6 +2510,47 @@ void Class_StaticIntPropertyChanged(object sender, int e)
             GC.WaitForPendingFinalizers();
             Class.StaticIntProperty = 3;
             Assert.True(eventCalled);
+
+            // Test dynamic WeakRef-based EventSource caching
+            eventCalled = false;
+            static void Subscribe(EventHandler<int> handler) => Singleton.Instance.IntPropertyChanged += handler;
+            static void Unsubscribe(EventHandler<int> handler) => Singleton.Instance.IntPropertyChanged -= handler;
+            static void Assign(int value) => Singleton.Instance.IntProperty = value;
+            Subscribe(Class_StaticIntPropertyChanged);
+            GC.Collect(2, GCCollectionMode.Forced, true);
+            GC.WaitForPendingFinalizers();
+            Unsubscribe(Class_StaticIntPropertyChanged);
+            Assign(3);
+            Assert.False(eventCalled);
+            Subscribe(Class_StaticIntPropertyChanged);
+            GC.Collect(2, GCCollectionMode.Forced, true);
+            GC.WaitForPendingFinalizers();
+            Assign(3);
+            Assert.True(eventCalled);
+
+            // Test that event delegates don't leak when not unsubscribed.
+            // Test runs into a different function as the finalizer wasn't
+            // getting triggered otherwise with a weak reference.
+            (System.WeakReference<Class> weakClassInstance, System.WeakReference<EventHandlerClass> weakEventHandlerClass) =
+                TestEventDelegateCleanup();
+            GC.Collect(2, GCCollectionMode.Forced, true);
+            GC.WaitForPendingFinalizers();
+            GC.Collect(2, GCCollectionMode.Forced, true);
+            GC.WaitForPendingFinalizers();
+            Assert.False(weakClassInstance.TryGetTarget(out _));
+            Assert.False(weakEventHandlerClass.TryGetTarget(out _));
+        }
+
+        class EventHandlerClass
+        {
+            private readonly Action eventCalled;
+
+            public EventHandlerClass(Action eventCalled)
+            {
+                this.eventCalled = eventCalled;
+            }
+
+            public void IntPropertyChanged(object sender, int e) => eventCalled();
         }
 
 #if NET

src/WinRT.Runtime/ComWrappersSupport.cs

@@ -60,7 +60,7 @@ public static IObjectReference GetObjectReferenceForInterface(IntPtr externalCom
         {
             if (externalComObject == IntPtr.Zero)
             {
-                return null;
+                return null;
             }
 
             using var unknownRef = ObjectReference<IUnknownVftbl>.FromAbi(externalComObject);

src/WinRT.Runtime/Interop/IWeakReferenceSource.net5.cs

@@ -172,7 +172,14 @@ private static int Do_Abi_Resolve(IntPtr thisPtr, Guid* riid, IntPtr* objectRefe
             var ThisPtr = _obj.ThisPtr;
 
             ExceptionHelpers.ThrowExceptionForHR(_obj.Vftbl.Resolve(ThisPtr, ref riid, out IntPtr objRef));
-            return ComWrappersSupport.GetObjectReferenceForInterface(objRef);
+            try
+            {
+                return ComWrappersSupport.GetObjectReferenceForInterface(objRef);
+            }
+            finally
+            {
+                MarshalInspectable<object>.DisposeAbi(objRef);
+            }
         }
     }
 }

src/WinRT.Runtime/Interop/IWeakReferenceSource.netstandard2.0.cs

@@ -182,7 +182,14 @@ public IWeakReference(ObjectReference<Vftbl> obj)
         public IObjectReference Resolve(Guid riid)
         {
             ExceptionHelpers.ThrowExceptionForHR(_obj.Vftbl.Resolve(ThisPtr, ref riid, out IntPtr objRef));
-            return ComWrappersSupport.GetObjectReferenceForInterface(objRef);
+            try
+            {
+                return ComWrappersSupport.GetObjectReferenceForInterface(objRef);
+            }
+            finally
+            {
+                MarshalInspectable<object>.DisposeAbi(objRef);
+            }
         }
     }
 }