Skip to content

Plan viewer: guard clipboard writes, and hand back the captured query on a truncated single-statement copy - #4593

Merged
erikdarlingdata merged 2 commits into
devfrom
viewer/4582-clipboard-and-truncated-copy
Sep 28, 2026
Merged

erikdarlingdata merged 2 commits into
devfrom
viewer/4582-clipboard-and-truncated-copy

Conversation

@erikdarlingdata

@erikdarlingdata erikdarlingdata commented Sep 28, 2026 •

Copy link
Copy Markdown
Owner

Fixes #4582. Part of #4511.

Why

erikdarlingdata/PerformanceStudio@7cd9218 hardened every viewer clipboard write against a busy
Windows clipboard: another process holding it briefly (a clipboard manager, Office, RDP) makes
SetTextAsync/SetText throw, and an unguarded call crashes the app. PM already had a guarded
clipboard READ (ClipboardText.TryRead, #2833) but no guarded write, and the plan viewer's Copy
Query Text / Copy Operator Name / Copy Predicate / Copy Seek Predicate entries, plus the blocking
chain, deadlock graph, and DataGrid export copy helpers, all called Clipboard.SetText /
Clipboard.SetDataObject unguarded.

Separately, erikdarlingdata/PerformanceStudio@35249e2 (corrected by @fdd3d22) fixed Copy Query Text
to fall back to the query a plan was captured from when the plan's own copy hit SQL Server's
4,000-character showplan cap — but only for single-statement plans, counted the way the Statements
grid counts them (flattened into stored procedure/UDF bodies), not Batches.Sum (which stops at
the outer batch and would mis-treat EXEC dbo.SomeProc as single-statement).

What changes

  • PerformanceMonitor.Ui/ClipboardText.cs: new TrySetText(string), mirroring TryRead's
    bounded retry (8 attempts, 25 ms apart) over a swappable WriteOnce seam so a pin can force the
    clipboard-open failure without a real busy clipboard.
  • Every Clipboard.SetText/SetDataObject call in PerformanceMonitor.Ui now routes through
    ClipboardText.TrySetText: PlanViewerControl.xaml.cs (Copy Query Text),
    PlanViewerControl.Interaction.cs (Copy Operator Name / Object Name / Predicate / Seek
    Predicate), BlockingChainControl.xaml.cs and DeadlockGraphControl.xaml.cs (Copy SQL Text),
    and DataGridExport.cs (Copy Cell / Copy Row / Copy All Rows — used by ~29 grids across Lite and
    the Darling Viewer).
  • PerformanceMonitor.PlanAnalysis/PlanDisplayText.CopyQueryText(statement, statementCount, capturedQueryText): the pure fallback rule. Returns the captured text only when the statement
    is truncated (IsTextTruncated), the plan is single-statement by the caller's already-flattened
    count, and captured text is available; otherwise the plan's own text.
  • PlanViewerControl: added CapturedQueryText (set by LoadPlan's existing queryText
    parameter — no host wiring needed, both hosts already pass it where they have it) and
    _allStatementsCount (the same flattened count PopulateStatementsGrid already builds), and
    wired CopyStatementText_Click through the new helper.

Lite: gets this too (shared control)

Yes — PlanViewerControl, DataGridExport, and both context-menu controls live in
PerformanceMonitor.Ui and are shared by Lite and the Darling Viewer. Both hosts' plan-opening call
sites (Lite/Controls/ServerTab.Plans.cs, Darling/PerformanceMonitor.Darling.Viewer/ViewerServerTab.Plans.cs)
already pass a queryText argument to LoadPlan at most call sites (Query Store stored plans,
active-query snapshots), so CapturedQueryText is populated there for free. Call sites that open a
plan with no stored query text (e.g. a plan opened from a saved .sqlplan file, or a cache-fetched
plan with no query text available) pass null, and the fallback simply doesn't fire for those — the
plan's own (possibly truncated) text is returned, same as today, with Rule 39 still flagging the
truncation.

Not ported / follow-up

  • Lite/Helpers/ContextMenuHelper.cs, Lite/Controls/ServerTab.CopyExport.cs,
    Lite/Controls/RecommendationsTab.xaml.cs, Lite/Windows/SettingsWindow.xaml.cs,
    Lite/Windows/AlertDetailWindow.xaml.cs, Lite/Windows/EntraDeviceCodeWindow.xaml.cs,
    Darling/PerformanceMonitor.Darling.Viewer/MainWindow.xaml.cs,
    ViewerServerTab.CopyExport.cs, ViewerServerTab.ChartContextMenu.cs, SettingsWindow.xaml.cs,
    AlertDetailWindow.xaml.cs (18 unguarded Clipboard.SetText/SetDataObject call sites total)
    have the same unguarded shape but sit outside the shared PerformanceMonitor.Ui plan-viewer
    surface this issue scoped to. They're filed as Lite and the Darling Viewer: 18 unguarded clipboard writes can crash the app when the clipboard is busy #4594.
  • deprecated/Dashboard/** gets no work per standing policy.
  • PS's CopyParameterizedStatementText_Click fallback exclusion (@fdd3d22's other half) has no PM
    equivalent yet — PM has no parameterized-copy menu entry to guard.

Test plan

New pure pins in Darling/Darling.Tests/Viewer4582Tests.cs (macOS-runnable, no WPF):

  • single-statement truncated + captured text → returns captured text
  • single-statement not truncated → returns the plan's own text even with captured text available
  • truncated + no captured text → returns the plan's own (truncated) text
  • multi-statement by grid count (one batch statement, statementCount: 2) truncated → returns the
    plan's own text, NOT the captured outer batch (the @fdd3d22 bug pinned directly)

RED: on origin/dev (461d503), the same test file fails to compile —
PlanDisplayText has no CopyQueryText member (CS0117, all 4 tests). Confirmed via a detached
worktree at that commit.

Mutation: flipping statementCount == 1 to statementCount >= 1 in CopyQueryText turns the
multi-statement pin (case 4 above) [FAIL]; reverted, and a clean rebuild confirmed 4/4 GREEN
again.

Run (macOS, in-process, Microsoft.WindowsDesktop.App stripped from the runtimeconfig):

Total: 235, Errors: 0, Failed: 0, Skipped: 2, Not Run: 0

(Viewer4582Tests plus the full required list: ShowPlanParserCondAndMultiplePlanTests,
ActualPlanRequestTests, ActualPlanDispatchTests, ActualPlanResultParseTests,
QueryModificationDetectorTests, ActualPlanCaptureLoopTests, ActualPlanGatingTests,
ReproScriptBuilderHardeningTests, DarlingAnalysisPipelineTests,
DarlingMcpPlanToolsSurfaceAndSqlTests, DarlingMcpPlanToolsLivePostgresTests,
McpPlanAnalysisEnvelopeTests, SerialLoopStoreSizeSourceTests, TsqlConventionGuardTests,
DocCommentHygieneTests.)

Builds, Release, -p:EnableWindowsTargeting=true, 0 warnings each: Darling.Tests,
PerformanceMonitor.PlanAnalysis, PerformanceMonitor.Ui, Lite/PerformanceMonitorLite.csproj,
Lite.Tests, Darling/PerformanceMonitor.Darling.Viewer.

Not run — Windows-only, CI decides them: Darling.Tests and Lite.Tests themselves build on
macOS but their WPF-dependent Viewer* classes can't discover/run here for PresentationFramework
reasons; none of them reference the files this PR touches, so they're unaffected. Lite.Tests has
no new/changed test in this PR (the guarded-write pin lives in Darling.Tests, which does run
in-process on macOS).

Screenshot plan (for a human on Windows, since the WPF wiring itself has no macOS-runnable
pin): open a plan with a single statement whose text is at or beyond the showplan cap (any plan
whose StatementText is ≥3,990 characters) via a Query Store "View Plan" path that passes stored
query text (so a full, untruncated query is available); right-click the row in the Statements
panel and choose "Copy Query Text"; paste — the pasted text should be the full stored query, not a
string ending mid-token. Then hold the Windows clipboard open in another app (e.g. an active
Office/Word paste-preview) and repeat any Copy action in the plan viewer, the blocking chain
control, or a DataGrid — none should crash; a transient failure should simply not copy.

CHANGELOG

SECTION: Fixed
ENTRY: - The plan viewer no longer crashes when another program is holding the clipboard, and "Copy Query Text" now hands back the full query on a truncated single-statement plan ([#4593]) - Copying anything from the plan viewer, blocking chain, deadlock graph, or a results grid used to crash the app if the Windows clipboard was briefly locked by another program; copies now fail quietly instead. Separately, "Copy Query Text" on a single-statement plan whose text hit SQL Server's 4,000-character cap now returns the full query the plan was captured from, instead of a copy cut off mid-word — for the Query Store and Active Queries plan sources that pass the original query text.
REF: [#4593]: #4593

… on a truncated single-statement copy

The Windows clipboard is a shared resource; another process holding it briefly
(a clipboard manager, Office, RDP) makes an unguarded write throw and crash
the app, the same failure #2833 already fixed for reads. ClipboardText gains
a guarded TrySetText, mirroring TryRead's bounded retry, and every
Clipboard.SetText/SetDataObject call in the shared plan viewer, blocking
chain, deadlock graph, and DataGrid export helpers now routes through it.

Copy Query Text on a truncated single-statement plan now hands back the full
query the plan was captured from instead of the plan's own 4,000-character
showplan-capped copy, matching PerformanceStudio's fix. The guard counts
statements the way the Statements grid counts them (descending into stored
procedure and UDF bodies), not by outer batch, so a plan captured around
EXEC dbo.SomeProc is correctly treated as multi-statement and never hands
back the outer EXEC for a truncated body statement.

Fixes #4582
…and-truncated-copy

# Conflicts:
#	PerformanceMonitor.PlanAnalysis/PlanDisplayText.cs
@erikdarlingdata
erikdarlingdata marked this pull request as ready for review September 28, 2026 16:05
@erikdarlingdata
erikdarlingdata merged commit 30e93e8 into dev Sep 28, 2026
15 of 16 checks passed
@erikdarlingdata
erikdarlingdata deleted the viewer/4582-clipboard-and-truncated-copy branch September 28, 2026 16:05
erikdarlingdata added a commit that referenced this pull request Sep 28, 2026
…dText.TrySetText

Merging dev brought in a properties-panel copy path with its own bare-try
guard around Clipboard.SetText, added after #4593/#4600 branched. It didn't
route through the shared ClipboardText helper, so ClipboardWriteCensusTests
failed post-merge. Delegate to ClipboardText.TrySetText instead of
duplicating the guard.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant