Skip to content

Commit bf9b2e7

Browse files
fix(reconciler): dedupe ToggleSwitch.Toggled wiring via EventHandlerState (#113)
* fix(reconciler): dedupe ToggleSwitch.Toggled wiring via EventHandlerState The wire-dedupe flag for ToggleSwitch.Toggled lived in ConditionalWeakTable<FrameworkElement, PoolableWireFlags>, keyed by managed RCW identity. WinRT projection can produce two RCWs over the same underlying DependencyObject; each gets its own CWT entry, both pass the dedupe check, and toggle.Toggled += subscribes the trampoline twice. One IsOn change then fans into multiple user-callback invocations. Symptom: EchoSuppress_ToggleSwitch_NoEchoCall flaked in ~8% of full-suite selftest runs but never in isolation. The first subscription consumed the BeginSuppress token; the second subscription saw counter=0 and invoked the user OnChanged with the value Reactor had just written. Fix: store the trampoline on EventHandlerState (attached via ReactorAttached.StateProperty, keyed by native DO identity) — the same pattern already used for SizeChanged, PointerPressed, Tapped, etc. The comment on GetOrCreateEventState (Reconciler.cs:2590) explicitly notes this is the safe key. Validation: 60/60 full-suite runs clean across two fix iterations (baseline expected ~4.8 flakes at 8%). Follow-up: PoolableWireFlags.ButtonClick / TextBoxTextChanged / TextBoxSelectionChanged share the same CWT-by-RCW vulnerability and should be migrated to EventHandlerState. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs(reconciler): clarify which dedupe scheme covers which control Address PR #113 review: the comment block above PoolableWireFlags described the CWT approach as covering ToggleSwitch, but that's now out of date — ToggleSwitch dedupes via EventHandlerState (DP keyed by native DO identity). Spell out which scheme each control uses, point at issue #114 for the planned migration of the remaining CWT entries, and direct future contributors at the EventHandlerState pattern. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 6abe668 commit bf9b2e7

2 files changed

Lines changed: 22 additions & 12 deletions

File tree

src/Reactor/Core/Reconciler.Mount.cs

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -675,23 +675,22 @@ private WinUI.ToggleSwitch MountToggleSwitch(ToggleSwitchElement ts)
675675
return toggle;
676676
}
677677

678-
/// <summary>
679-
/// Wires ToggleSwitch.Toggled trampoline only when the element has a
680-
/// handler AND the control hasn't been wired yet. The CWT flag survives
681-
/// pool round-trips (ToggleSwitch is a poolable type).
682-
/// </summary>
678+
// Dedupe via EventHandlerState (attached on ReactorAttached.StateProperty,
679+
// keyed by native DependencyObject identity). A plain CWT keyed by managed
680+
// RCW identity is unsafe — two RCWs over the same DO miss the dedupe and
681+
// double-subscribe, fanning one Toggled into multiple user-callback invocations.
683682
internal static void EnsureToggleSwitchWiring(WinUI.ToggleSwitch toggle, ToggleSwitchElement ts)
684683
{
685684
if (ts.OnChanged is null) return;
686-
var flags = GetPoolableWireFlags(toggle);
687-
if (flags.ToggleSwitchToggled) return;
688-
flags.ToggleSwitchToggled = true;
689-
toggle.Toggled += (s, _) =>
685+
var state = GetOrCreateEventState(toggle);
686+
if (state.ToggleSwitchToggledTrampoline is not null) return;
687+
state.ToggleSwitchToggledTrampoline = (s, _) =>
690688
{
691689
var t = (WinUI.ToggleSwitch)s!;
692690
if (ChangeEchoSuppressor.ShouldSuppress(t)) return;
693691
(GetElementTag(t) as ToggleSwitchElement)?.OnChanged?.Invoke(t.IsOn);
694692
};
693+
toggle.Toggled += state.ToggleSwitchToggledTrampoline;
695694
}
696695

697696
private WinUI.RatingControl MountRatingControl(RatingControlElement rc)

src/Reactor/Core/Reconciler.cs

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -360,15 +360,25 @@ internal static void DetachReactorState(FrameworkElement fe)
360360
// ElementPool.CleanElement), but clears Tag. So "Tag is null" on rent does
361361
// NOT mean "unwired" — the control may already have a trampoline attached.
362362
// We need a per-control flag that survives pool cycles to avoid double-
363-
// subscription. A ConditionalWeakTable gives us that with zero cross-ABI
364-
// cost and weak keys so GC isn't blocked.
363+
// subscription.
364+
//
365+
// Two dedupe schemes coexist:
366+
// - Button.Click and TextBox.TextChanged/SelectionChanged use the CWT
367+
// below, keyed by managed FrameworkElement identity. This is fragile
368+
// when WinRT projection produces a second RCW over the same native
369+
// DependencyObject (each RCW gets its own entry, both pass dedupe,
370+
// trampoline subscribed twice). See issue #114 for the migration.
371+
// - ToggleSwitch.Toggled uses EventHandlerState (attached on
372+
// ReactorAttached.StateProperty), which is keyed by native DO identity
373+
// and so is RCW-stable. New poolable wiring should follow this pattern
374+
// — see EnsureToggleSwitchWiring and the EnsureXxxSubscribed helpers
375+
// for the input events.
365376

366377
internal sealed class PoolableWireFlags
367378
{
368379
public bool ButtonClick;
369380
public bool TextBoxTextChanged;
370381
public bool TextBoxSelectionChanged;
371-
public bool ToggleSwitchToggled;
372382
}
373383

374384
private static readonly global::System.Runtime.CompilerServices.ConditionalWeakTable<FrameworkElement, PoolableWireFlags> _poolableWireFlags = new();
@@ -2554,6 +2564,7 @@ internal sealed class EventHandlerState
25542564
public global::Windows.Foundation.TypedEventHandler<UIElement, Microsoft.UI.Xaml.Input.CharacterReceivedRoutedEventArgs>? CharacterReceivedTrampoline;
25552565
public RoutedEventHandler? GotFocusTrampoline;
25562566
public RoutedEventHandler? LostFocusTrampoline;
2567+
public RoutedEventHandler? ToggleSwitchToggledTrampoline;
25572568
public global::Windows.Foundation.TypedEventHandler<UIElement, Microsoft.UI.Xaml.Input.AccessKeyDisplayRequestedEventArgs>? AccessKeyDisplayRequestedTrampoline;
25582569

25592570
/// <summary>

0 commit comments

Comments
 (0)