[DO NOT MERGE] conditional revert of EventSource caching (#876)

* Revert "Added caching of event registration state to support unsubscribes across garbage collections (#861)" This reverts commit fa038af. * fix merge bug and make weak ref interfaces public * Add back null check to avoid future issues if older runtime is used. Co-authored-by: Manodasan Wignarajah <[email protected]>
microsoft · Jun 17, 2021 · 84a3654 · 84a3654
1 parent 21f231b
commit 84a3654
Show file tree

Hide file tree

Showing 10 changed files with 110 additions and 339 deletions.
diff --git a/src/Tests/TestComponentCSharp/Singleton.cpp b/src/Tests/TestComponentCSharp/Singleton.cpp
diff --git a/src/Tests/TestComponentCSharp/Singleton.h b/src/Tests/TestComponentCSharp/Singleton.h
diff --git a/src/Tests/TestComponentCSharp/TestComponentCSharp.idl b/src/Tests/TestComponentCSharp/TestComponentCSharp.idl
@@ -130,17 +130,6 @@ 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
@@ -464,7 +453,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
@@ -71,7 +71,6 @@
     </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" />
@@ -87,7 +86,6 @@
     <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" />

diff --git a/src/Tests/TestComponentCSharp/TestComponentCSharp.vcxproj.filters b/src/Tests/TestComponentCSharp/TestComponentCSharp.vcxproj.filters
@@ -15,7 +15,6 @@
     <ClCompile Include="ManualProjectionTestClasses.cpp" />
     <ClCompile Include="WarningClass.cpp" />
     <ClCompile Include="WarningStatic.cpp" />
-    <ClCompile Include="Singleton.cpp" />
     <ClCompile Include="Windows.Class.cpp" />
   </ItemGroup>
   <ItemGroup>
@@ -25,7 +24,6 @@
     <ClInclude Include="ManualProjectionTestClasses.h" />
     <ClInclude Include="WarningStatic.h" />
     <ClInclude Include="WarningClass.h" />
-    <ClInclude Include="Singleton.h" />
     <ClInclude Include="Windows.Class.h" />
   </ItemGroup>
   <ItemGroup>

diff --git a/src/Tests/UnitTest/TestComponentCSharp_Tests.cs b/src/Tests/UnitTest/TestComponentCSharp_Tests.cs
@@ -2438,14 +2438,15 @@ public void TestCovariance()
             Assert.True(TestObject.IterableOfObjectIterablesProperty.SequenceEqual(listOfListOfUris));
         }
 
-        // Ensure that event subscription state is properly cached to enable later unsubscribes
         [Fact]
-        public void TestEventSourceCaching()
+        public void TestStaticEventWithGC()
         {
             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();
@@ -2457,23 +2458,6 @@ public void TestEventSourceCaching()
             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
@@ -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);

diff --git a/src/cswinrt/code_writers.h b/src/cswinrt/code_writers.h
@@ -2517,7 +2517,7 @@ db_path.stem().string());
 
     void write_event_source_generic_args(writer& w, cswinrt::type_semantics eventTypeSemantics);
 
-    void write_event_source_ctor(writer& w, Event const& evt, int index)
+    void write_event_source_ctor(writer& w, Event const& evt)
     {
         if (for_typedef(w, get_type_semantics(evt.EventType()), [&](TypeDef const& eventType)
             {
@@ -2526,12 +2526,10 @@ db_path.stem().string());
                     auto [add, remove] = get_event_methods(evt);
                     w.write(R"( new EventSource__EventHandler%(_obj,
 %,
-%,
 %))",
 bind<write_type_params>(eventType),
 get_invoke_info(w, add).first,
-get_invoke_info(w, remove).first,
-index);
+get_invoke_info(w, remove).first);
                     return true;
                 }
                 return false;
@@ -2542,15 +2540,13 @@ index);
 
         auto [add, remove] = get_event_methods(evt);
         w.write(R"(
-new %%(_obj,
-%,
-%,
-%))",
+    new %%(_obj,
+    %,
+    %))",
             bind<write_event_source_type_name>(get_type_semantics(evt.EventType())),
             bind<write_event_source_generic_args>(get_type_semantics(evt.EventType())),
             get_invoke_info(w, add).first,
-            get_invoke_info(w, remove).first,
-            index);
+            get_invoke_info(w, remove).first);
     }
 
     void write_event_sources(writer& w, TypeDef const& type)
@@ -3458,37 +3454,26 @@ global::System.Collections.Concurrent.ConcurrentDictionary<RuntimeTypeHandle, ob
             w.write("}\n");
         }
 
-        int index = 0;
         for (auto&& evt : type.EventList())
         {
             auto semantics = get_type_semantics(evt.EventType());
-            auto event_source = w.write_temp(settings.netstandard_compat ? "_%" : "Get_%()", evt.Name());
+            auto event_source = settings.netstandard_compat ? 
+                w.write_temp("_%", evt.Name())
+                : w.write_temp(R"(_%.GetValue((IWinRTObject)this, (key) =>
+{
+%
+return %;
+}))",
+                    evt.Name(),
+                    bind(init_call_variables),
+                    bind<write_event_source_ctor>(evt));
             w.write(R"(
-%%event % %%
+%event % %%
 {
 add => %.Subscribe(value);
 remove => %.Unsubscribe(value);
 }
 )",
-                bind([&](writer& w)
-                {
-                    if(settings.netstandard_compat)
-                        return;
-                    w.write(R"(private EventSource<%> Get_%()
-{
-return _%.GetValue((IWinRTObject)this, (key) =>
-{
-%
-return %;
-});
-}
-)",
-                        bind<write_type_name>(get_type_semantics(evt.EventType()), typedef_name_type::Projected, false),
-                        evt.Name(),
-                        evt.Name(),
-                        bind(init_call_variables),
-                        bind<write_event_source_ctor>(evt, index));
-                }),
                 settings.netstandard_compat ? "public " : "",
                 bind<write_type_name>(get_type_semantics(evt.EventType()), typedef_name_type::Projected, false),
                 bind([&](writer& w)
@@ -3501,7 +3486,6 @@ return %;
                 evt.Name(),
                 event_source,
                 event_source);
-            index++;
         }
     }
 
@@ -4967,14 +4951,11 @@ public static Guid PIID = Vftbl.PIID;
             type_name,
             type.TypeName(),
             type.TypeName(),
-            [&](writer& w) 
+            bind_each([&](writer& w, Event const& evt)
             {
-                int index = 0;
-                for (auto&& evt : type.EventList())
-                {
-                    w.write("_% = %;\n", evt.Name(), bind<write_event_source_ctor>(evt, index++));
-                }
+                w.write("_% = %;\n", evt.Name(), bind<write_event_source_ctor>(evt));
             },
+                type.EventList()),
             [&](writer& w) {
                 for (auto required_interface : required_interfaces)
                 {
@@ -6436,7 +6417,7 @@ bind<write_type_name>(type, typedef_name_type::CCW, true)
         {
             return;
         }
-        for_typedef(w, eventTypeSemantics, [&](TypeDef const&)
+        for_typedef(w, eventTypeSemantics, [&](TypeDef const& eventType)
             {
                 std::vector<std::string> genericArgs;
                 auto arg_count = std::get<generic_type_instance>(eventTypeSemantics).generic_args.size();
@@ -6470,36 +6451,36 @@ bind<write_type_name>(type, typedef_name_type::CCW, true)
                 auto eventTypeCode = w.write_temp("%", bind<write_type_name>(eventType, typedef_name_type::Projected, false));
                 auto invokeMethodSig = get_event_invoke_method(eventType);
                 w.write(R"(
-    internal unsafe class %% : EventSource<%>
-    {
-        private % handler;
+internal unsafe class %% : EventSource<%>
+{
+private % handler;
 
-        internal %(IObjectReference obj,
-            delegate* unmanaged[Stdcall]<System.IntPtr, System.IntPtr, out WinRT.EventRegistrationToken, int> addHandler,
-            delegate* unmanaged[Stdcall]<System.IntPtr, WinRT.EventRegistrationToken, int> removeHandler, int index) : base(obj, addHandler, removeHandler, index)
-        {
-        }
+internal %(IObjectReference obj,
+delegate* unmanaged[Stdcall]<System.IntPtr, System.IntPtr, out WinRT.EventRegistrationToken, int> addHandler,
+delegate* unmanaged[Stdcall]<System.IntPtr, WinRT.EventRegistrationToken, int> removeHandler) : base(obj, addHandler, removeHandler)
+{
+}
 
-        override protected System.Delegate EventInvoke
-        {
-            get
-            {
-                if (handler == null)
-                {
-                    handler = (%) =>
-                    {
-                        var localDel = _state.del;
-                        if (localDel == null)
-                        {
-                            return %;
-                        }
-                        %localDel.Invoke(%);
-                    };
-                }
-                return handler;
-            }
-        }
-    }
+override protected System.Delegate EventInvoke
+{
+get
+{
+if (handler == null)
+{
+handler = (%) =>
+{
+var localDel = _event;
+if (localDel == null)
+{
+return %;
+}
+%localDel.Invoke(%);
+};
+}
+return handler;
+}
+}
+}
 )",
                     bind<write_event_source_type_name>(eventTypeSemantics),
                     bind<write_event_source_generic_args>(eventTypeSemantics),

diff --git a/src/cswinrt/cswinrt.vcxproj b/src/cswinrt/cswinrt.vcxproj
@@ -46,9 +46,6 @@
       <SubSystem>Console</SubSystem>
       <AdditionalDependencies>kernel32.lib;user32.lib;%(AdditionalDependencies);windowsapp.lib;advapi32.lib;shlwapi.lib</AdditionalDependencies>
     </Link>
-    <ClCompile>
-      <TreatWarningAsError>true</TreatWarningAsError>
-    </ClCompile>
   </ItemDefinitionGroup>
   <ItemGroup>
     <ClInclude Include="cmd_reader.h" />