[codex] Keep SUBCLASSPROC delegates rooted per window

[HavenDV]

Summary

• keep a rooted SUBCLASSPROC delegate per transparent window instead of a single process-wide slot
• remove the rooted delegate when the subclassed window is destroyed
• preserve the existing transparent-window behavior otherwise

Why

WindowUtilities.MakeTransparent() currently stores the subclass callback in a single static field. Once another window is subclassed, the older callback can lose its managed root while the native subclass is still active. On .NET 10 that now terminates the process with a fail-fast when Windows invokes the collected delegate.

Validation

• dotnet build src/libs/H.NotifyIcon/H.NotifyIcon.csproj --configuration Release /p:GeneratePackageOnBuild=false
• dotnet build src/libs/H.NotifyIcon.WinUI/H.NotifyIcon.WinUI.csproj --configuration Release /p:AppxGeneratePriEnabled=false /p:GeneratePackageOnBuild=false

Fixes #229

[claude]

Code Review

This PR correctly addresses a real crash on .NET 10 where invoking a GC-collected delegate via native callback causes FailFast. The core approach — rooting one SUBCLASSPROC per window handle in a ConcurrentDictionary — is sound. Here are my observations:

---

What's good

• Root cause is fixed: The single static field was the problem; the dictionary correctly keeps each delegate alive as long as its window exists.
• Thread-safe: ConcurrentDictionary is the right choice here.
• Static lambda in GetOrAdd: Avoids an unnecessary closure allocation. Good.
• WM_NCDESTROY for cleanup: Correct choice — it's the last message a window receives, so it's the right place to release per-window resources.

---

Suggestions

1. Call RemoveWindowSubclass in the WM_NCDESTROY handler

Per Microsoft's documentation on ComCtl32 subclassing:

"You should call RemoveWindowSubclass inside your subclass procedure while processing WM_NCDESTROY. Failure to do so could result in some memory leaks."

The current code removes the delegate from the dictionary but doesn't explicitly remove the Win32 subclass. Recommended:

case WindowNcDestroy:
{
    _ = PInvoke.RemoveWindowSubclass(hWnd, SubClassDelegates.GetValueOrDefault((nint)hWnd.Value), 0);
    _ = SubClassDelegates.TryRemove((nint)hWnd.Value, out _);
    break;
}

(The window is being destroyed anyway, so this won't cause a functional regression, but it's defensive and avoids internal ComCtl32 state leaks.)

2. Use PInvoke constant for WM_NCDESTROY for consistency

The rest of the file uses CsWin32-generated constants (e.g. PInvoke.WM_ERASEBKGND). For consistency, add WM_NCDESTROY to NativeMethods.json and use PInvoke.WM_NCDESTROY instead of the raw magic number 0x0082. If that's not feasible, at minimum add a comment on the constant:

private const uint WindowNcDestroy = 0x0082; // WM_NCDESTROY

3. GetOrAdd factory can be called more than once under contention

ConcurrentDictionary.GetOrAdd does not guarantee the factory is called at most once — under contention, two threads may both invoke new SUBCLASSPROC(WindowSubClass) and one result is discarded. Since SUBCLASSPROC delegates here all point to the same static method, this is not a correctness problem (the discarded delegate is just an extra allocation). But if this ever changes to capture per-window state, it could become an issue. Worth a brief comment acknowledging this.

---

No tests

Understandable given the GC-timing nature of the bug (hard to reproduce deterministically), but a basic test that calls MakeTransparent on two different window handles, then forces a GC cycle, and verifies the dictionary still holds both delegates, would provide a regression guard. Optional, but would strengthen confidence.

---

Summary

The fix is correct and addresses a legitimate .NET 10 crash. The RemoveWindowSubclass omission is the only item I'd consider a real gap — everything else is style/robustness. Good catch on the bug.