Added caching of event registration state to support unsubscribes acr…

…oss garbage collections (#861) * added caching of event registration state to support unsubscribes across GCs * PR feedback and add missing files * treat compile warnings as errors * remove warnings
microsoft · Jun 11, 2021 · fa038af · fa038af
1 parent af996c1
commit fa038af
Show file tree

Hide file tree

Showing 13 changed files with 327 additions and 110 deletions.
diff --git a/src/Tests/TestComponentCSharp/Singleton.cpp b/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();
+    }
+}
diff --git a/src/Tests/TestComponentCSharp/Singleton.h b/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>
+    {
+    };
+}
diff --git a/src/Tests/TestComponentCSharp/TestComponentCSharp.idl b/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
@@ -460,7 +471,7 @@ namespace TestComponentCSharp
             static event Windows.Foundation.EventHandler<Int32> WarningEvent;
         }
     }
-
+    
     [contract(Windows.Foundation.UniversalApiContract, 8)]
     interface IWarning1
     {

diff --git a/src/Tests/TestComponentCSharp/TestComponentCSharp.vcxproj b/src/Tests/TestComponentCSharp/TestComponentCSharp.vcxproj
@@ -70,6 +70,7 @@
     </ClInclude>
     <ClInclude Include="NonAgileClass.h" />
     <ClInclude Include="ComImports.h" />
+    <ClInclude Include="Singleton.h" />
     <ClInclude Include="WarningClass.h" />
     <ClInclude Include="WarningStatic.h" />
   </ItemGroup>
@@ -84,6 +85,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" />
   </ItemGroup>

diff --git a/src/Tests/TestComponentCSharp/TestComponentCSharp.vcxproj.filters b/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" />
   </ItemGroup>
   <ItemGroup>
     <ClInclude Include="pch.h" />
@@ -23,6 +24,7 @@
     <ClInclude Include="ManualProjectionTestClasses.h" />
     <ClInclude Include="WarningStatic.h" />
     <ClInclude Include="WarningClass.h" />
+    <ClInclude Include="Singleton.h" />
   </ItemGroup>
   <ItemGroup>
     <Midl Include="TestComponentCSharp.idl" />

diff --git a/src/Tests/UnitTest/TestComponentCSharp_Tests.cs b/src/Tests/UnitTest/TestComponentCSharp_Tests.cs
@@ -2444,15 +2444,14 @@ public void TestCovariance()
             Assert.True(TestObject.IterableOfObjectIterablesProperty.SequenceEqual(listOfListOfUris));
         }
 
+        // 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();
@@ -2464,6 +2463,23 @@ 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);
         }
 
 #if NET5_0

diff --git a/src/WinRT.Runtime/ComWrappersSupport.cs b/src/WinRT.Runtime/ComWrappersSupport.cs
@@ -58,6 +58,11 @@ public static void MarshalDelegateInvoke<T>(IntPtr thisPtr, Action<T> invoke)
 
         public static IObjectReference GetObjectReferenceForInterface(IntPtr externalComObject)
         {
+            if (externalComObject == IntPtr.Zero)
+            {
+                return null;
+            }
+
             using var unknownRef = ObjectReference<IUnknownVftbl>.FromAbi(externalComObject);
 
             if (IsFreeThreaded())

diff --git a/src/WinRT.Runtime/Interop/IWeakReferenceSource.net5.cs b/src/WinRT.Runtime/Interop/IWeakReferenceSource.net5.cs
@@ -7,14 +7,14 @@ namespace WinRT.Interop
 {
     [WindowsRuntimeType]
     [Guid("00000038-0000-0000-C000-000000000046")]
-    internal interface IWeakReferenceSource
+    public interface IWeakReferenceSource
     {
         IWeakReference GetWeakReference();
     }
 
     [WindowsRuntimeType]
     [Guid("00000037-0000-0000-C000-000000000046")]
-    internal interface IWeakReference
+    public interface IWeakReference
     {
         IObjectReference Resolve(Guid riid);
     }

diff --git a/src/WinRT.Runtime/Interop/IWeakReferenceSource.netstandard2.0.cs b/src/WinRT.Runtime/Interop/IWeakReferenceSource.netstandard2.0.cs
@@ -7,14 +7,14 @@ namespace WinRT.Interop
 {
     [WindowsRuntimeType]
     [Guid("00000038-0000-0000-C000-000000000046")]
-    internal interface IWeakReferenceSource
+    public interface IWeakReferenceSource
     {
         IWeakReference GetWeakReference();
     }
 
     [WindowsRuntimeType]
     [Guid("00000037-0000-0000-C000-000000000046")]
-    internal interface IWeakReference
+    public interface IWeakReference
     {
         IObjectReference Resolve(Guid riid);
     }
@@ -61,28 +61,19 @@ internal struct Vftbl
             public static readonly Vftbl AbiToProjectionVftable;
             public static readonly IntPtr AbiToProjectionVftablePtr;
 
-#if NETSTANDARD2_0
             internal delegate int GetWeakReferenceDelegate(IntPtr thisPtr, IntPtr* weakReference);
             private static readonly Delegate[] DelegateCache = new Delegate[1];
-#endif
             static Vftbl()
             {
                 AbiToProjectionVftable = new Vftbl
                 {
                     IUnknownVftbl = global::WinRT.Interop.IUnknownVftbl.AbiToProjectionVftbl,
-#if NETSTANDARD2_0
                     _GetWeakReference = Marshal.GetFunctionPointerForDelegate(DelegateCache[0] = new GetWeakReferenceDelegate(Do_Abi_GetWeakReference)).ToPointer(),
-#else
-                    _GetWeakReference = (delegate* unmanaged<IntPtr, IntPtr*, int>)&Do_Abi_GetWeakReference
-#endif
                 };
                 AbiToProjectionVftablePtr = Marshal.AllocHGlobal(Marshal.SizeOf<Vftbl>());
                 Marshal.StructureToPtr(AbiToProjectionVftable, AbiToProjectionVftablePtr, false);
             }
 
-#if !NETSTANDARD2_0
-            [UnmanagedCallersOnly]
-#endif
             private static int Do_Abi_GetWeakReference(IntPtr thisPtr, IntPtr* weakReference)
             {
                 *weakReference = default;
@@ -141,28 +132,19 @@ public struct Vftbl
             public static readonly Vftbl AbiToProjectionVftable;
             public static readonly IntPtr AbiToProjectionVftablePtr;
 
-#if NETSTANDARD2_0
             public delegate int ResolveDelegate(IntPtr thisPtr, Guid* riid, IntPtr* objectReference);
             private static readonly Delegate[] DelegateCache = new Delegate[1];
-#endif
             static Vftbl()
             {
                 AbiToProjectionVftable = new Vftbl
                 {
                     IUnknownVftbl = global::WinRT.Interop.IUnknownVftbl.AbiToProjectionVftbl,
-#if NETSTANDARD2_0
                     _Resolve = Marshal.GetFunctionPointerForDelegate(DelegateCache[0] = new ResolveDelegate(Do_Abi_Resolve)).ToPointer(),
-#else
-                    _Resolve = (delegate* unmanaged<IntPtr, Guid*, IntPtr*, int>)&Do_Abi_Resolve
-#endif
                 };
                 AbiToProjectionVftablePtr = Marshal.AllocHGlobal(Marshal.SizeOf<Vftbl>());
                 Marshal.StructureToPtr(AbiToProjectionVftable, AbiToProjectionVftablePtr, false);
             }
 
-#if !NETSTANDARD2_0
-            [UnmanagedCallersOnly]
-#endif
             private static int Do_Abi_Resolve(IntPtr thisPtr, Guid* riid, IntPtr* objectReference)
             {
                 IObjectReference _objectReference = default;

diff --git a/src/WinRT.Runtime/MatchingRefApiCompatBaseline.net5.0.txt b/src/WinRT.Runtime/MatchingRefApiCompatBaseline.net5.0.txt
@@ -3,4 +3,6 @@ TypesMustExist : Type 'System.Numerics.VectorExtensions' does not exist in the r
 TypesMustExist : Type 'WinRT.ComWrappersHelper' does not exist in the reference but it does exist in the implementation.
 MembersMustExist : Member 'public void WinRT.ComWrappersSupport.RegisterObjectForInterface(System.Object, System.IntPtr, System.Runtime.InteropServices.CreateObjectFlags)' does not exist in the reference but it does exist in the implementation.
 MembersMustExist : Member 'protected void WinRT.IObjectReference.AddRef(System.Boolean)' does not exist in the reference but it does exist in the implementation.
-Total Issues: 4
+TypesMustExist : Type 'WinRT.Interop.IWeakReference' does not exist in the reference but it does exist in the implementation.
+TypesMustExist : Type 'WinRT.Interop.IWeakReferenceSource' does not exist in the reference but it does exist in the implementation.
+Total Issues: 6