Skip to content

Commit

Permalink
[DO NOT MERGE] conditional revert of EventSource caching (#876)
Browse files Browse the repository at this point in the history
* 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 <mawign@microsoft.com>
  • Loading branch information
Scottj1s and manodasanW authored Jun 17, 2021
1 parent 21f231b commit 84a3654
Show file tree
Hide file tree
Showing 10 changed files with 110 additions and 339 deletions.
43 changes: 0 additions & 43 deletions src/Tests/TestComponentCSharp/Singleton.cpp

This file was deleted.

19 changes: 0 additions & 19 deletions src/Tests/TestComponentCSharp/Singleton.h

This file was deleted.

13 changes: 1 addition & 12 deletions src/Tests/TestComponentCSharp/TestComponentCSharp.idl
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -464,7 +453,7 @@ namespace TestComponentCSharp
static event Windows.Foundation.EventHandler<Int32> WarningEvent;
}
}

[contract(Windows.Foundation.UniversalApiContract, 8)]
interface IWarning1
{
Expand Down
2 changes: 0 additions & 2 deletions src/Tests/TestComponentCSharp/TestComponentCSharp.vcxproj
Original file line number Diff line number Diff line change
Expand Up @@ -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" />
Expand All @@ -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" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand All @@ -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>
Expand Down
26 changes: 5 additions & 21 deletions src/Tests/UnitTest/TestComponentCSharp_Tests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion src/WinRT.Runtime/ComWrappersSupport.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
117 changes: 49 additions & 68 deletions src/cswinrt/code_writers.h
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand All @@ -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;
Expand All @@ -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)
Expand Down Expand Up @@ -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)
Expand All @@ -3501,7 +3486,6 @@ return %;
evt.Name(),
event_source,
event_source);
index++;
}
}

Expand Down Expand Up @@ -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)
{
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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),
Expand Down
3 changes: 0 additions & 3 deletions src/cswinrt/cswinrt.vcxproj
Original file line number Diff line number Diff line change
Expand Up @@ -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" />
Expand Down
Loading

0 comments on commit 84a3654

Please sign in to comment.