Prevent clipboard reads from blocking the tray and fix resource cleanup

docs/long-running-hang-audit.md

@@ -0,0 +1,154 @@
+# Long-running tray hang: ownership and blocking-call audit
+
+Audited baseline: `1f47957a9e63a98bd369d8348e23eabca72a548d`, matching
+[Actions run 39242061366](https://github.com/stuartd/ClipboardDiff/actions/runs/39242061366).
+The reported symptom was an unresponsive notification-area icon/menu after several
+days on a work laptop. The process had not been restarted. No stack or resource
+sample from that process was available during this review, so the cause of that
+particular incident remains unconfirmed.
+
+## Findings and changes
+
+| Finding | Impact and correction |
+| --- | --- |
+| Clipboard inspection called `GetClipboardData` on the UI thread. | This API can ask another application to render delayed clipboard data and wait for it. The async capture method did not prevent this: its semaphore await often completed synchronously, and inspection happened before its next await. Inspection now runs on a worker. Opening, inspecting, and closing the clipboard stay on that same worker, and the monitor holds its existing gate until the native call returns. Cancellation rejects the result without abandoning an open clipboard or starting overlapping native reads. Privacy inspection still precedes retrieving text or paths. |
+| Explorer registration reported a change on every refresh. | `UpdatePresentation` runs after clipboard observations and when opening the tray menu. The previous code rewrote identical association values and reported successful pair registration as a change, causing repeated `SHCNE_ASSOCCHANGED` calls from the UI thread. Registration now compares both the value and registry type, writes only changes, and distinguishes pair readiness from an actual change. Deleted/changed values still get repaired, and cross-session reconciliation remains active. This is avoidable shell work, rather than evidence of a C++ memory leak. |
+| Rebuilding the viewer submenu cleared items without disposing them. | `ToolStripItemCollection.Clear` removes items but does not dispose them. Selecting/changing a viewer repeatedly could leave removed items awaiting GC and unmanaged resource cleanup. Each old item is now disposed before its replacement is created. This is an operation-specific disposal defect; merely leaving the app idle does not exercise this path. |
+| About-window links discarded `Process.Start`'s returned `Process`. | The wrapper is now disposed. This closes owned process resources without terminating the browser. This only occurs when clicking About links and is unlikely to explain an idle hang. |
+
+Microsoft documents the rendering callback in
+[Clipboard Operations](https://learn.microsoft.com/windows/win32/dataxchg/clipboard-operations#delayed-rendering).
+Moving the read protects the UI even when a producer blocks. It cannot cancel
+an in-progress Win32 call or prove that delayed rendering caused this incident.
+
+## Native C++ ownership
+
+The DLL is loaded into **Explorer**, not the ClipDiff process. Source review and
+the new Windows lifetime regressions found no unmatched allocation, COM reference,
+event handle, or menu ownership in the exercised production paths. This was not
+a multi-day Windows soak test or inspection of the original hung process.
+
+| Native path | Ownership and release |
+| --- | --- |
+| `IsClipDiffReady` | Each successful `OpenEventW` is followed by a zero-time wait and `CloseHandle`. No persistent per-query handle. |
+| `IsFilePair` | Shell item arrays and individual items are held in `ComPtr`; early failures unwind both. The loop is bounded to two items. The method requests attributes, not display names or file contents. |
+| `ContextMenu::Initialize` | Releases the previous selection before accepting another. Retains at most one data object per handler. |
+| `ContextMenu::InvokeCommand` | A valid invocation swaps selection into a local `ComPtr`. Paused/unready/invalid selection, activation failure, drag-enter rejection, and drop completion all release it. An unknown verb deliberately leaves the still-usable menu selection intact. |
+| Handler destruction | Resets selection and decrements the module reference. All successful `QueryInterface` calls take a matching reference; final `Release` deletes the handler. |
+| Class factory | Creation and failed-interface paths release temporary objects. Factory lifetime and balanced `LockServer` calls are reflected in `DllCanUnloadNow`. |
+| Menu items | `InsertMenuItemW` inserts a text item into the caller's menu. The Shell owns that `HMENU`; the extension must not destroy it. No extension-owned menu bitmap, brush, window, timer, or worker thread exists. |
+
+Added Windows native regression checks for 10,000 factory/handler lifetimes,
+module unload readiness, process-handle counts, and 1,000 selection replacements.
+A reference-counted wrapper around the real Shell data object checks release on
+reinitialization, handler destruction, failed invocation, and successful invocation.
+
+The native calls that can still wait are Shell metadata queries, COM activation,
+and the calls to the out-of-process drop target. A blocked ClipDiff UI can therefore
+also block an Explorer invocation. The ready event indicates that the server was
+registered; it is not a UI-health heartbeat. Retaining a handler while its client
+still owns it, or Explorer keeping a DLL loaded, is not by itself a leak.
+
+## Managed ownership ledger
+
+| Component | Result |
+| --- | --- |
+| Application instance mutex | Held for process lifetime; released/disposed on exit. A second normal launch exits without repairing or foregrounding an existing hung instance. |
+| Hidden message window | Removes its hook and disposes `HwndSource`. Monitor and hotkey callbacks unsubscribe before window disposal. |
+| Clipboard listener | Unregisters on disposal. Retry delays are finite and only run after updates; there is no periodic multi-day timer. |
+| Clipboard reads | `CloseClipboard` and `GlobalUnlock` are in `finally` paths. Clipboard-owned global memory is correctly borrowed, not freed. The UI blocking defect is fixed as described above. |
+| Protected clipboard writes | Failed `GlobalAlloc`/`GlobalLock`/`SetClipboardData` paths free untransferred memory. Successful `SetClipboardData` transfers ownership to Windows; freeing it there would be wrong. Clipboard closure is unconditional. |
+| Read cancellation/semaphore | Superseded cancellation sources are cancelled/disposed. At most one native inspection owns the monitor's gate. Shutdown may finish before an already-running native call returns; its worker still closes the clipboard. |
+| Copied files | Streams are `await using`, including empty, oversized, decoding failure, and cancellation exits. |
+| Shortcut COM objects | A locally created ShellLink wrapper is final-released in `finally`. No `Resolve`, launch, or mutation is used. |
+| Explorer COM server | Revokes class registration, releases owned IUnknown pointers, and balances successful COM initialization. Created targets release temporary IUnknown pointers. |
+| Incoming COM drop data | Every successfully obtained `STGMEDIUM` reaches `ReleaseStgMedium`, including invalid counts/types and path failures. Incoming RCWs are managed by the interop runtime; force-releasing a potentially shared incoming wrapper would be unsafe. |
+| Explorer pipe server/client | Streams, message deadlines, listener shutdown source, and slot semaphore have cleanup paths. Pending receipts are bounded to four; old completed tasks are removed. Handler completion is not awaited while accepting the next message. |
+| Shared registration | Registry keys use `using`; owner/gate leases are released, with deletion on close. Lock acquisition has a 500 ms deadline, not an indefinite wait. Stale-owner reconciliation is tested. |
+| External viewer processes | Exit events are detached; process wrappers and workspaces are disposed by an interlocked, one-shot cleanup. Failed starts and disposal have cleanup paths. Delayed cleanup retains a completed comparison for three seconds intentionally. |
+| External temporary files | Exclusive owner leases protect other sessions. Files are made writable before best-effort deletion; cleanup releases the lease. Windows may prevent deleting a file still held by a viewer. |
+| Tray and drawing | Owned icon, notification icon, menu, and per-paint pens are disposed. The shared system-icon fallback is correctly not disposed. Replaced menu-item disposal is fixed. |
+| WPF windows and documents | One diff window and one About window are reused intentionally; final shutdown closes them. Undo is disabled in diff text boxes. The active document is replaced/cleared; there is no application collection of past documents. |
+| History/comparison tasks | History retains two entries. The last displayed diff and its comparison input can intentionally outlive those history entries. Generation checks reject superseded results; cancellation sources are scoped to each comparison. |
+| Settings/startup/tool discovery | Registry and file APIs close their resources. No periodic registry scan or growing cache of viewer discoveries is present. |
+
+## Remaining risks to distinguish with the hung process
+
+- File probes, file opens, and ShellLink loading run off the UI thread, but a
+  synchronous operation that ignores cancellation can outlive its request.
+  Repeated inaccessible network paths can accumulate blocked workers until the
+  operating system returns. This is distinct from a missing stream disposal;
+  many thread-pool stacks in those calls would support this explanation.
+- The UI still performs local registration/settings work, external-viewer launch
+  and temporary-file creation, and explicit clipboard writes. Storage, antivirus,
+  Shell callbacks, or unavailable installation paths can delay those operations.
+- Very large single-line comparisons can spend substantial time creating and
+  measuring WPF rich-text documents. Core search budgets do not bound WPF layout
+  cost. This needs a Windows UI workload or stack to diagnose.
+- A hidden modal dialog, a stale icon after process exit, and an Explorer hang
+  are different cases. Process presence, wait chains, and stacks help separate them.
+
+These are remaining code paths, not additional confirmed causes. There is no
+uptime or weekend-specific trigger in the resource ownership reviewed here.
+
+## Validation
+
+- 96 core tests and 13 stress-workload tests passed, with no skips or failures.
+- 215 portable application tests passed in a local linked-source harness at
+  `artifacts/audit/PortableAudit.Tests.csproj`. This runs the actual portable
+  production classes and existing/new tests under `net10.0`, without loading WPF
+  or substituting native API stubs. The blocked-renderer test uses a controlled
+  blocking delegate; it is not a real Windows clipboard integration test.
+- The Release managed solution, including the Windows test assembly, compiled
+  with zero warnings/errors.
+- The native DLL and native test executable were cross-compiled with MinGW as
+  an additional C++ check. This is not the supported MSVC release build, and
+  neither native binary was executed on Linux.
+- The hang-inspection PowerShell script parses successfully. Its Windows
+  process-inspection behavior has not been executed on this machine.
+- The full [Windows workflow for code commit f3fb721](https://github.com/stuartd/ClipboardDiff/actions/runs/40388380708)
+  succeeded in 2m 30s. This ran the supported MSVC native build and expanded
+  native lifetime/Shell tests, the managed core and Windows tests (including
+  registry and tray disposal regressions), release-version checks, and the
+  complete release script. Both self-contained and framework-dependent Windows
+  packages passed the script's payload validation and were uploaded as artifacts.
+
+The Windows workflow result is separate from the local Linux checks above.
+To repeat platform validation locally, use a normal Windows PowerShell with
+ClipDiff closed:
+
+```powershell
+./scripts/build-shell-extension.ps1 -Test
+dotnet test ClipDiff.Windows.sln --configuration Release
+dotnet test tests/ClipDiff.Stress.Tests --configuration Release
+```
+
+Then test the tray while a synthetic producer delays rendering, check ordinary
+clipboard capture/privacy behavior, repeat viewer selection, and exercise the
+actual Explorer menu. Those manual desktop checks and reproduction of the
+reported laptop hang remain outstanding.
+
+## Inspect the existing hang before restarting
+
+Run on the affected laptop:
+
+```powershell
+./scripts/inspect-hang.ps1
+```
+
+This only reports process/thread identifiers, CPU/memory/handle/GUI counts and
+wait states. It does not read the clipboard, fetch text or file paths, collect
+window titles or command lines, write a dump, or restart anything. Supply
+`-ClipDiffProcessId <id>` for a dotnet-hosted development launch. `Responding`
+alone is not conclusive for a tray app.
+
+Use Task Manager's Details tab to **Analyze wait chain** for that PID. If the
+Microsoft `dotnet-stack` tool is already available, the script prints its
+`report --process-id` command; the report contains managed method stacks without
+locals or clipboard values. If collection stalls, cancel it. Native symbols may
+be needed to identify a specific underlying Win32 wait.
+
+Do not collect a process/heap dump for this review: it could persist captured
+clipboard values. Start with the read-only inspection; the existing stress tool
+overwrites the clipboard with synthetic data and is for subsequent patched-build
+testing, after the original hang evidence has been captured.

scripts/inspect-hang.ps1

@@ -0,0 +1,96 @@
+[CmdletBinding()]
+param(
+	[int]$ClipDiffProcessId = 0
+)
+
+$ErrorActionPreference = 'Stop'
+
+if ($env:OS -ne 'Windows_NT')
+{
+	throw 'Run this read-only inspection on the Windows desktop where ClipDiff is unresponsive.'
+}
+
+# Do not request clipboard data, window titles, command lines, selected paths,
+# settings contents, or memory dumps. This script does not restart any process.
+if (-not ('ClipDiffHangInspection' -as [type]))
+{
+	Add-Type -TypeDefinition @'
+using System;
+using System.Runtime.InteropServices;
+public static class ClipDiffHangInspection
+{
+	[DllImport("user32.dll", SetLastError = true)]
+	public static extern uint GetGuiResources(IntPtr process, uint flags);
+}
+'@
+}
+
+if ($ClipDiffProcessId -gt 0)
+{
+	$targets = @(Get-Process -Id $ClipDiffProcessId)
+}
+else
+{
+	$targets = @(Get-Process -Name ClipDiff -ErrorAction SilentlyContinue)
+}
+
+if ($targets.Count -eq 0)
+{
+	Write-Host 'No ClipDiff process was found. For a dotnet-hosted launch, supply -ClipDiffProcessId.'
+	return
+}
+
+Write-Host "Inspection UTC: $([DateTime]::UtcNow.ToString('O'))"
+
+foreach ($target in $targets)
+{
+	try
+	{
+		$target.Refresh()
+		[pscustomobject]@{
+			ProcessId = $target.Id
+			SessionId = $target.SessionId
+			StartedUtc = $target.StartTime.ToUniversalTime().ToString('O')
+			Responding = $target.Responding
+			CpuSeconds = $target.TotalProcessorTime.TotalSeconds
+			PrivateMiB = [Math]::Round($target.PrivateMemorySize64 / 1MB, 1)
+			WorkingSetMiB = [Math]::Round($target.WorkingSet64 / 1MB, 1)
+			Handles = $target.HandleCount
+			Threads = $target.Threads.Count
+			GdiObjects = [ClipDiffHangInspection]::GetGuiResources($target.Handle, 0)
+			UserObjects = [ClipDiffHangInspection]::GetGuiResources($target.Handle, 1)
+		} | Format-List
+
+		$threadRows = foreach ($thread in $target.Threads)
+		{
+			try
+			{
+				[pscustomobject]@{
+					ThreadId = $thread.Id
+					State = $thread.ThreadState
+					WaitReason = $(if ($thread.ThreadState -eq 'Wait') { $thread.WaitReason } else { $null })
+					CpuSeconds = $thread.TotalProcessorTime.TotalSeconds
+				}
+			}
+			finally
+			{
+				$thread.Dispose()
+			}
+		}
+
+		$threadRows | Format-Table -AutoSize
+
+		Write-Host "For a wait chain: Task Manager > Details > process $($target.Id) > Analyze wait chain."
+		Write-Host "If dotnet-stack is installed: dotnet-stack report --process-id $($target.Id)"
+		Write-Host 'Managed stacks contain method names, not clipboard values. Cancel collection if it stalls.'
+		Write-Host 'Responding is only a hint for a tray application; a wait chain/stack is stronger evidence.'
+	}
+	catch
+	{
+		Write-Host "Could not inspect process $($target.Id): $($_.Exception.GetType().Name). It may have exited or denied access."
+	}
+	finally
+	{
+		$target.Dispose()
+	}
+}

src/ClipDiff.Windows/Clipboard/ClipboardInspectionReader.cs

@@ -0,0 +1,37 @@
+namespace ClipDiff.Windows.Clipboard;
+
+internal sealed class ClipboardInspectionReader(
+	Func<bool> tryOpen,
+	Action close,
+	Func<uint, DateTimeOffset, ClipboardInspection> inspect)
+{
+	public Task<ClipboardInspection?> ReadAsync(
+		uint sequenceNumber,
+		DateTimeOffset observedAt,
+		CancellationToken cancellationToken)
+	{
+		// GetClipboardData can wait for another process to render a format. Keep
+		// Open/Inspect/Close on one worker thread, away from the UI message loop.
+		// The monitor retains its read gate until this completes: cancellation
+		// must not start another native read while this one owns the clipboard.
+		return Task.Run<ClipboardInspection?>(() =>
+		{
+			if (!tryOpen())
+			{
+				return null;
+			}
+
+			try
+			{
+				cancellationToken.ThrowIfCancellationRequested();
+				var inspection = inspect(sequenceNumber, observedAt);
+				cancellationToken.ThrowIfCancellationRequested();
+				return inspection;
+			}
+			finally
+			{
+				close();
+			}
+		}, cancellationToken);
+	}
+}

src/ClipDiff.Windows/Clipboard/ClipboardMonitor.cs

@@ -16,6 +16,7 @@ internal sealed class ClipboardMonitor : IDisposable
 	private readonly NativeMessageWindow nativeMessageWindow;
 	private readonly NativeClipboard nativeClipboard;
 	private readonly ClipboardPrivacyInspector clipboardPrivacyInspector;
+	private readonly ClipboardInspectionReader clipboardInspectionReader;
 	private readonly ClipboardFileObservationReader clipboardFileObservationReader = new();
 	private readonly SemaphoreSlim readGate = new(1, 1);
 	private CancellationTokenSource? pendingRead;
@@ -34,6 +35,10 @@ public ClipboardMonitor(NativeMessageWindow messageWindow)
 			RegisterFormat("CanIncludeInClipboardHistory"),
 			RegisterFormat("CanUploadToCloudClipboard"));
 		clipboardPrivacyInspector = new ClipboardPrivacyInspector(nativeClipboard, formats);
+		clipboardInspectionReader = new ClipboardInspectionReader(
+			() => nativeClipboard.TryOpen(nint.Zero),
+			nativeClipboard.Close,
+			clipboardPrivacyInspector.Inspect);
 
 		baselineSequence = NativeMethods.GetClipboardSequenceNumber();
 		latestRequestedSequence = baselineSequence;
@@ -155,19 +160,8 @@ private async Task ProcessSequenceAsync(uint expectedSequence, CancellationToken
 						return;
 					}
 
-					ClipboardInspection? inspection = null;
-
-					if (nativeClipboard.TryOpen(nativeMessageWindow.Handle))
-					{
-						try
-						{
-							inspection = clipboardPrivacyInspector.Inspect(expectedSequence, DateTimeOffset.Now);
-						}
-						finally
-						{
-							nativeClipboard.Close();
-						}
-					}
+					var inspection = await clipboardInspectionReader.ReadAsync(
+						expectedSequence, DateTimeOffset.Now, cancellationToken);
 
 					var observation = inspection switch
 					{