Fix FailFast crash in LabelEditNativeWindow due to garbage collected WinEvent hook delegate

[LeafShi1]

Fixes #14657

Root Cause

Window destroyed → hooks not unregistered → object garbage collected → hook callback invokes freed memory → crash (FailFast).

The original code skipped cleanup when handle was gone, leaving orphaned hooks running.

Proposed changes

This prevents callbacks from invoking garbage-collected delegates.

• Added UnhookWinEventHooks() method to centralize hook cleanup
• Call unhook when handle is destroyed in OnHandleChange()
• Added _isReleasing flag to prevent reentry during cleanup
• Set _winEventProcCallback = null after unhooking to allow GC

Customer Impact

• Applications using screen readers (e.g., Narrator) no longer crash when editing PropertyGrid or TreeView with accessibility enabled.

Regression?

• No

Risk

• Minimal: only defensive guards added, no behavior changes.

Screenshots

Before

protected override void OnHandleChange()
{
    base.OnHandleChange();
    
    if (!IsHandleCreated)
    {
        return;  // ← Hooks remain registered even after window destruction
    }
    
    InstallWinEventHooks();
}

When native window is destroyed, WinEvent hooks are not unregistered. This orphaned hook persists after the managed object is garbage collected, causing late callbacks to invoke freed delegates → FailFast crash.

After

protected override void OnHandleChange()
{
    base.OnHandleChange();
    
    if (!IsHandleCreated)
    {
        UnhookWinEventHooks();  // ← Properly clean up hooks
        return;
    }
    
    InstallWinEventHooks();
}

private void UnhookWinEventHooks()
{
    if (_winEventHooksInstalled)
    {
        PInvoke.UnhookWinEvent(_valueChangeHook);
        PInvoke.UnhookWinEvent(_textSelectionChangedHook);
        _winEventHooksInstalled = false;
        _winEventProcCallback = null;  // Allow GC
    }
}

Hooks are properly unregistered when the native window handle is destroyed, preventing orphaned callbacks and GC-related crashes.

Test methodology

• Manually

Test environment(s)

• .net 11.0.0-preview.6.26310.110

Microsoft Reviewers: Open in CodeFlow

[Copilot]

Pull request overview

Fixes a GC-induced FailFast crash in LabelEditNativeWindow accessibility scenarios by ensuring WinEvent hooks are reliably unregistered when the underlying native handle is destroyed, preventing callbacks from invoking a collected delegate.

Changes:

• Centralized WinEvent hook cleanup into UnhookWinEventHooks() and invoked it when the handle is destroyed (OnHandleChange when !IsHandleCreated).
• Added a _isReleasing reentrancy guard to prevent hook install/uninstall cycles during teardown.
• Added callback-side guards and cleared the stored delegate reference after unhooking to avoid stale callbacks and enable GC.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[LeafShi1]

The fix has passed testing, and no regressions were found.

[ricardobossan]

I removed the _isReleasing = true; line from both ..._DuringRelease_DoesNotReinstallHooks (135) and ..._PreventsReentrantHookInstallation (167), and both still pass. Since those tests exist to verify the _isReleasing guard, being able to delete the guard's own setup without turning them red means they aren't really exercising it; a regression in that guard wouldn't be caught. And the scenario the PR fixes; the hooks still registered when the handle goes away; has no test that would catch a regression. Would a test like this, which drives the unhook path the PR adds, be worth adding instead?

Suggested test (added to the PR and passing):

[WinFormsFact]
public void LabelEditNativeWindow_OnHandleChange_HandleDestroyedWhileHooked_UnhooksHooks()
{
    using TreeView treeView = new() { Size = new Size(300, 200) };
    treeView.CreateControl();

    TreeViewLabelEditNativeWindow labelEdit = new(treeView);

    // Reproduce the crash state: hooks still installed after the handle is gone.
    labelEdit.TestAccessor.Dynamic._winEventHooksInstalled = true;
    labelEdit.TestAccessor.Dynamic._isReleasing = false;

    labelEdit.TestAccessor.Dynamic.OnHandleChange();

    // The hooks must be removed so the collected delegate can't fire.
    Assert.False((bool)labelEdit.TestAccessor.Dynamic._winEventHooksInstalled);
    Assert.Null(labelEdit.TestAccessor.Dynamic._winEventProcCallback);
}

[hiSandog]

The cleanup path looks targeted to the lifetime issue described here. One useful regression case might repeatedly destroy and recreate the label-edit handle while forcing GC between transitions, then verify that hooks are reinstalled exactly once and that a late callback is safely ignored. That would cover both halves of the lifecycle rather than only final disposal.