Bug: UseEffect cleanup does not fire when a Component is embedded under DockableContent.Content

[codemonkeychris]

Summary

When a Component (ComponentElement) is used as the Content of a DockableContent inside a Spec 045 docking host, and the pane is later removed from the layout (programmatic close, drag tear-out, or layout-prop change), the component's body does disappear from the visual tree, but its UseEffect cleanup callback is never invoked. This violates the Reactor reliability invariant in spec 045 §8.10 (useEffect cleanup on pane close runs in dependency order) and is a memory-leak / resource-leak hazard for any app pane that holds a subscription, timer, file handle, or other disposable.

A real-world example of what this would silently break: a Document whose Content opens a database connection in UseEffect and closes it in the cleanup. After the user clicks the X on the tab, the visual goes away but the connection stays open until the connection-holder closes/GCs.

Reproducer

The selftest fixture NativeDocking_Reliability_UseEffectCleanup_RunsOnPaneClose in tests/Reactor.AppTests.Host/SelfTest/Fixtures/NativeDockingReliabilityFixture.cs reproduces this end-to-end. To run:

dotnet build tests/Reactor.AppTests.Host/Reactor.AppTests.Host.csproj -p:Platform=x64
./tests/Reactor.AppTests.Host/bin/x64/Debug/net10.0-windows10.0.22621.0/Reactor.AppTests.Host.exe \
    --self-test --filter NativeDocking_Reliability_UseEffectCleanup

Currently the fixture asserts what does work (the body unmounts, the §2.16 drain runs, Pending empties) and leaves the cleanup assertion commented out with a pointer to this issue. When the bug is fixed, uncomment the line marked // NOTE: known-failing assertion and confirm it passes.

The fixture's shape, in miniature:

internal sealed class EffectCounterComponent : Component<EffectCounterProps>
{
    public static int MountedCount;
    public static int CleanupCount;

    public override Element Render()
    {
        UseEffect(() =>
        {
            MountedCount++;
            return () => CleanupCount++;   // <-- never runs
        });
        return TextBlock($"effect-body-{Props.Marker}");
    }
}

// In the fixture:
var pane = new Document
{
    Title = "EffectPane",
    Key = "effect:pane",
    Content = Component<EffectCounterComponent, EffectCounterProps>(new EffectCounterProps("p1")),
    CanClose = true,
};
var managerEl = new DockManager { Layout = new DockTabGroup(new DockableContent[] { pane }) };
host.Mount(_ => managerEl);
await Harness.Render();

// MountedCount == 1, CleanupCount == 0, body in tree (OK)

var model = DockHostModelBridge.Get(managerEl);
model.Close(pane);
host.Mount(_ => managerEl with { });   // force sub-host re-render
await Harness.Render();

// Pending drained (OK)
// body-effect-p1 gone from visual tree (OK)
// CleanupCount == 0   <-- BUG: expected 1

What works

• Mount-time UseEffect body runs exactly once (MountedCount == 1).
• The §2.16 model-mutation drain processes the CloseOp and clears model.Pending.
• The TextBlock representing the component's body does disappear from the WinUI visual tree (Harness.FindText("effect-body-p1") returns null after the close).
• OnDocumentClosed fires.
• The reconciler-side Microsoft-UI-Reactor ComponentUnmount event is in the codebase (src/Reactor/Core/Reconciler.cs line 1051), so the unmount path exists.

What's broken

• EffectCounterComponent.CleanupCount stays at 0 after the pane is removed.
• The Trace list contains ["mount:p1"] only — the matching "cleanup:p1" never lands.

Pumping additional Harness.Render() frames does not flush the cleanup. Three extra frames were attempted; cleanup still does not fire.

Hypothesis on the root cause

The host's DockHostNativeComponent.WrapLeafWithPaneContext wraps every leaf like this:

private static Element WrapLeafWithPaneContext(DockableContent leaf)
{
    var content = leaf.Content ?? (Element)new BorderElement(null);
    var padded = new BorderElement(content) { Background = null, BorderThickness = 0 };
    var info = new DockPaneInfo(leaf.Key, leaf.Title ?? string.Empty, leaf);
    return padded
        .Padding(16)
        .Provide(DockContexts.Pane, (DockPaneInfo?)info)
        .Provide(DockContexts.PaneState, DockPaneState.Docked);
}

So when leaf.Content is a ComponentElement, the actual element-tree shape under the TabViewItem is:

Provide(PaneState) -> Provide(Pane) -> Padding -> BorderElement -> ComponentElement -> (component renders TextBlock)

Reconciler.UnmountRecursive (src/Reactor/Core/Reconciler.cs line 1023) looks the control up in _componentNodes and calls RunCleanups() on the component's context (line 1053):

if (_componentNodes.TryGetValue(control, out var node))
{
    Diagnostics.ReactorEventSource.Log.ComponentUnmount(...);
    node.Component?.Context.RunCleanups();
    node.Context?.RunCleanups();
    _componentNodes.Remove(control);
    ...
}

Suspected gap: the _componentNodes map keys are the WinUI controls the reconciler placed into the parent's child collection. When a ComponentElement is nested several layers deep behind context Providers and Padding wrappers, the cleanup-firing lookup may be keyed on the wrong control (e.g. the outer Border that the wrapper materialized, not the inner control the component actually produced) — so when that outer Border is unmounted, the registry lookup for the component's own node misses.

A second possibility: the unmount path recurses into children of registered type-handlers (TabView, FlexPanel, etc.) but does not recurse through plain Border-with-Padding chains, so the component node attached deeper down never gets a RunCleanups visit.

Suggested investigation path

1. Add a Debug.WriteLine (or temporary ReactorEventSource event) at Reconciler.UnmountRecursive's line 1049 _componentNodes.TryGetValue site — log the control type + whether the lookup hit. Run the fixture. If the lookup misses for the EffectCounterComponent's control, the registry-key mismatch hypothesis is confirmed.
2. Walk the _componentNodes dictionary on the way down: instead of (or in addition to) looking up the current control, scan descendants for any registered component node when unmounting an arbitrary subtree. This is what most React-style reconcilers do.
3. As an alternative — if option 2 is too expensive — store a back-pointer from each ComponentNode to its containing control so that unmounting the wrapper Border can locate the inner component nodes and tear them down.

Why this matters now (Spec 045)

• Spec 045 §8.10 explicitly calls out "useEffect cleanup on pane close runs in dependency order" as a reliability invariant.
• The §2.16 model-mutation drain (commit 1cd0dc61 on feat/045-docking-windows-p2) enables programmatic model.Close(pane), which is the most likely production trigger for this code path: apps that close panes via UI buttons will silently leak whatever the pane's components had registered.
• The §2.25 reliability fixture (commit 8c9d8051) documented the gap with a [~] (partial) status in docs/specs/tasks/045-docking-windows-implementation.md.
• The fixture is intentionally green right now (the known-failing assertion is commented). When this issue is fixed, that assertion should be uncommented and lit up.

Verification path once fixed

1. Uncomment the // NOTE: known-failing assertion line in NativeDockingReliabilityFixture.UseEffectCleanup_RunsOnPaneClose:

   H.Check("Reliability_Effect_CleanupRanOnClose", EffectCounterComponent.CleanupCount == 1);

2. Re-add the trace-ordering assertion that was also dropped:

   H.Check("Reliability_Effect_TraceOrderingMountThenCleanup",
       EffectCounterComponent.Trace.Count == 2 &&
       EffectCounterComponent.Trace[0] == "mount:p1" &&
       EffectCounterComponent.Trace[1] == "cleanup:p1");

3. Flip the spec checklist in docs/specs/tasks/045-docking-windows-implementation.md §2.25 from [~] (partial) to [x].
4. Run --self-test --filter NativeDocking_Reliability and confirm all assertions pass.
5. Also run --self-test --filter NativeDocking (the full docking suite) to confirm no regressions — the unmount path is shared by drag tear-out, splitter re-layout, and scene re-render, all of which have existing identity-preservation tests (M19, M20).

Out of scope for this issue

• The non-docking case of nested components in general (e.g. a Component inside a VStack inside a Border inside another component). If the fix turns out to be a generic reconciler change, that's the right design, but the test surface for this issue is specifically the docking-host integration.
• Floating-window cleanup (P3 territory per spec §6.4).

Related references

• Spec 045 design: docs/specs/045-docking-windows-design.md §5.3.10, §8.10
• Spec 045 implementation: docs/specs/tasks/045-docking-windows-implementation.md §2.25
• Branch: feat/045-docking-windows-p2
• Recent commits surfacing this:
  • 1cd0dc61 feat(045): model-mutation drain §2.16
  • 8c9d8051 test(045): reliability + security selftests §2.24 §2.25; LayoutOverride wrapper fix