diff --git a/Darling/Darling.Tests/ViewerCommandTimeoutTests.cs b/Darling/Darling.Tests/ViewerCommandTimeoutTests.cs index ae9555f6b1..dc031328ae 100644 --- a/Darling/Darling.Tests/ViewerCommandTimeoutTests.cs +++ b/Darling/Darling.Tests/ViewerCommandTimeoutTests.cs @@ -70,6 +70,96 @@ public sealed class ViewerCommandTimeoutTests @"ViewerReadFanOut\s*\.\s*Of\s*\(", RegexOptions.Compiled | RegexOptions.CultureInvariant); + /// + /// A fire-and-forget call — _ = SomethingAsync();. Two of them in one member is a fan-out; ONE + /// is this project's ordinary single-flight-guarded refresh and is not. + /// + /// Deliberately WIDER than the identical-looking expression in + /// ViewerFleetTimerGuardTests and ViewerFleetTimerFanOutPositionTests, which use + /// (\w+): those two ask a question about named calls inside ONE known tick, so a name they + /// cannot spell is a name they do not need. This is a project-wide census, and a census blind to + /// _ = Controller.ReadAsync(); would report a clean sweep over a shape it never looked at — + /// which is the #3019 failure one level down. + /// + /// Unconstrained about the callee, unlike , and that asymmetry is + /// a deliberate trade in BOTH directions. Requiring an Async suffix here is not available: + /// three of this project's fifteen discarded call names are genuinely async without it — + /// OpenPlanTab (private async Task), OnHeatmapDrillDown and + /// PlanViewerController.LoadPlanIntoSubTab — so the suffix rule would reintroduce exactly the + /// silent gap #3019 is about. The cost paid instead is a false-positive path: _ = compiles for + /// any non-void call, so two discarded SYNCHRONOUS helpers in one member read as a two-wide fan-out and + /// would be asked for a width they do not need. Nothing in this project does that today (every + /// _ = X(...) site is a Task-returning call), the direction is a loud failure rather than a + /// silent one, and the shape is pinned in + /// so it reads as a known cost rather + /// than a surprise. The deferred rule can afford the suffix because its shape — + /// var t = Call(); with no await — is otherwise indistinguishable from ordinary local + /// assignment, which is most lines in the project. + /// + private static readonly Regex s_fireAndForget = new( + @"(^|[^A-Za-z0-9_])_\s*=\s*[A-Za-z_][A-Za-z0-9_\.]*\s*\(", + RegexOptions.Compiled | RegexOptions.CultureInvariant); + + /// + /// A task STARTED into a local without being awaited there — var t = SomethingAsync();. Two of + /// these with no await between them is the same fan-out as Task.WhenAll(t1, t2), written + /// with the awaits one per line instead of joined. + /// + /// A bare _ is excluded from the name, so this shape and + /// stay disjoint. var _ = SomeAsync(); would otherwise satisfy + /// both — _ is a legal name here, and the discard scan matches the same characters — and one + /// physical call would be counted into two different fan-out tallies, pairing against an unrelated + /// deferred start on one side and an unrelated discard on the other. It belongs to the DISCARD shape: + /// nothing holds the task, which is what a discard is. No site in this project spells it that way + /// today. Pinned by . Found in review. + /// + private static readonly Regex s_deferredRead = new( + @"(^|[^A-Za-z0-9_])(?:var|Task(?:\s*<[^;={}]*>)?)\s+(?!_\s*=)[A-Za-z_][A-Za-z0-9_]*\s*=\s*[A-Za-z_][A-Za-z0-9_\.]*Async\s*\(", + RegexOptions.Compiled | RegexOptions.CultureInvariant); + + /// An await as a keyword, not as part of a longer identifier. + private static readonly Regex s_await = new( + @"(^|[^A-Za-z0-9_])await[^A-Za-z0-9_]", + RegexOptions.Compiled | RegexOptions.CultureInvariant); + + /// + /// A member declaration with a body — accessibility, optional modifiers, return type, name, parameter + /// list, an optional generic constraint clause, open brace. The unit fan-outs are paired within: see + /// for why the enclosing BLOCK is the wrong unit. + /// + /// A generic method needs BOTH halves, and the constraint clause is only the visible one. + /// Its type parameters sit between the NAME and the parameter list, and its constraints sit between the + /// parameter list and the BODY, so a tail of \)\s*\{ against a bare name drops the member + /// entirely. Three are shaped that way today — ViewerSettingsFile.Load<T> and + /// Save<T>, and SettingsWindow.TryReadSectionAsync<T>. None holds a fan-out + /// marker, so the walk stayed green while seeing less than it claimed; the first one to grow a marker + /// would have tripped the unattributed assertion rather than being reported as an offender, which is a + /// confusing red rather than the useful one. + /// + /// The type-parameter group excludes = and newlines, which is load-bearing. Allowing + /// them let it run from a field's declared type through = new Dictionary<...>(comparer) to + /// the collection initializer's brace, reading CollectorSchedulePresets.Presets — a field — as a + /// method whose body is the initializer. A phantom body is worse than a missing one here: takes the OUTERMOST match, so one could swallow a real member's markers and attribute + /// them to something that can never declare a width. Measured while widening this pattern, not + /// hypothesised. Pinned by . + /// Found in review. + /// + private static readonly Regex s_memberSignature = new( + @"(?:private|public|protected|internal)(?:\s+(?:static|async|override|virtual|sealed|new|partial|unsafe|extern))*" + + @"\s+[A-Za-z_][A-Za-z0-9_<>,\.\[\]\?\s]*?\s(?[A-Za-z_][A-Za-z0-9_]*)" + + @"\s*(?:<[^;{}()=\n]*>)?\s*\([^;{}]*\)\s*(?:where\s[^{};]*)?\{", + RegexOptions.Compiled | RegexOptions.CultureInvariant); + + /// + /// The concurrency primitives this project does not use and whose fan-outs + /// therefore does not model. Pinned as ABSENT rather + /// than handled — see . + /// + private static readonly Regex s_unmodelledConcurrency = new( + @"Task\s*\.\s*WhenAny\s*\(|Task\s*\.\s*Factory\s*\.|Parallel\s*\.\s*(?:For|Invoke)|\.\s*ContinueWith\s*\(", + RegexOptions.Compiled | RegexOptions.CultureInvariant); + [Fact] public void EveryViewerCommand_SetsAnExplicitDeadline() { @@ -360,30 +450,80 @@ public void TheConstructionScan_ReadsCodeNotProse(string source, bool expectedSi } /// - /// Every joined fan-out in this project declares how wide it is, so the reads inside it are bounded by + /// Every fan-out in this project declares how wide it is, so the reads inside it are bounded by /// rather than by a solo read's ceiling (#3004). /// - /// Paired POSITIONALLY, not by enclosing block. The obvious rule — the declaration must sit - /// in the same block as the Task.WhenAll — is wrong on this codebase's real shape: - /// CorrelatedTimelineLanesControl declares its width in the outer try and awaits the join - /// inside a nested one, so a single-level backward walk finds the inner brace and reports the widest - /// fan-out in the project as an offender. Requiring instead that the k-th join in a file be preceded by - /// at least k declarations is immune to nesting, still order-sensitive, and still fails if any one - /// declaration is deleted — which is the property being bought. + /// A fan-out is N reads in flight together, not a syntax, and this project spells that + /// three ways. #3007 censused only the first, and #3019 is the hole that left: iterating + /// Task.WhenAll matches gives a fan-out that never joins ZERO iterations, so that shape could + /// not be reported however wrong it was — while being the very shape whose discovery took #3007's + /// census from one site to seventeen. + /// + /// Joined — await Task.WhenAll(...). One occurrence makes the member a fan-out. + /// Fire-and-forget — two or more _ = SomethingAsync();. One is this project's + /// ordinary single-flight-guarded refresh (27 such sites, none of them fan-outs); two is a fan-out, + /// because a discarded task has nobody to await it and is therefore still running when the next + /// starts. That is also why an await between two discards does NOT separate them, and why this + /// shape — unlike the deferred one below — takes no account of intervening awaits. + /// Deferred — two or more var t = SomethingAsync(); starts with no await + /// between them. Semantically the joined form with the awaits written one per line, and the spelling + /// ViewerServerTab.Blocking, .Charts and .RunningJobs use; each of those three + /// carries a doc comment stating in as many words that the reads run concurrently. + /// /// - /// What this does NOT cover, stated because the gap is real. A fan-out does not need a - /// Task.WhenAll: MainWindow.OnRefreshTimerTick fires five or six store reads unawaited and - /// joins none of them, and the connect path fires three. Nothing lexical distinguishes those from the - /// twenty other unawaited single reads in this project, which are single-flight-guarded and not - /// fan-outs at all, so a scan for that shape would be mostly false positives. Both real sites declare - /// their width by hand and carry a comment saying so; this test is the guard for the joined shape - /// ONLY. + /// Paired by MEMBER BODY — not positionally, and not by enclosing block. Both obvious + /// rules are wrong on this codebase's real shapes, in opposite directions. Same-BLOCK pairing breaks on + /// CorrelatedTimelineLanesControl, which declares its width in an outer try and awaits + /// the join inside a nested one, so a single-level backward walk finds the inner brace and reports the + /// widest fan-out in the project as an offender. Per-FILE positional counting — "the k-th join is + /// preceded by at least k declarations", which is what this pin used to do — breaks the other way: it + /// lets declarations LAUNDER across unrelated members of one file. MainWindow.xaml.cs carries + /// three declarations against one join, so under the old rule the declaration that actually pairs with + /// that join could be deleted and the two unrelated unjoined ones covered for it. A member body is + /// immune to the nesting (the outer and nested try are the same member) and to the laundering (a + /// declaration in one method cannot satisfy a fan-out in another). + /// + /// The markers are censused TREE-WIDE and attributed afterwards, which is the whole reason + /// unattributed can mean anything. Each pattern is matched against the file, not inside each + /// member body, and a marker whose is null is REPORTED rather than skipped. Scoped + /// the other way round — search within each braced body — a marker in a shape the walk cannot see would + /// simply never be looked at, and unattributed would sit at zero VACUOUSLY while covering less. + /// That distinction is not academic here: an expression-bodied member + /// (Task Both() => Task.WhenAll(a, b);) is a legal join with no braced body at all, and this + /// project has 378 expression-bodied members against 1,234 braced ones. None holds a join today, and + /// the point is that the census does not depend on that staying true — the first one to appear is a red + /// build naming its line, not a silent zero. Pinned by + /// , and the same property for an + /// unrecognised BRACED shape (a property accessor) is why the assertion names the signature pattern in + /// its message rather than the offending member. + /// + /// What this does NOT cover, stated because each gap is real. + /// + /// The declared WIDTH is not checked, only that a width is declared. Of(n)'s own summary + /// makes the value the call site's responsibility. + /// Mutually exclusive branches count as one fan-out: the three switch cases of + /// LoadBlockingAsync never run together, and two discards in an if/else would + /// count as a pair. This over-reports rather than under-reports. + /// Within ONE member, several independent fan-outs share the credit of a single declaration — + /// the positional residual, narrowed from per-file to per-member but not eliminated. + /// Reads reached through a helper the member CALLS rather than fires are the helper's fan-out, + /// not the caller's. + /// The deferred run is broken by any await between two starts, including one nested inside + /// a lambda rather than sequencing the starts at member level. That direction UNDER-reports — the same + /// direction as #3019 itself — and is left because the joined and fire-and-forget rules overlap it and + /// no member in this project is written that way; a depth-aware run would be the fix if one appears. + /// Concurrency primitives this project does not use, pinned absent by + /// so that adding one goes red here + /// rather than quietly widening this gap. + /// /// [Fact] public void EveryViewerFanOut_DeclaresItsWidth() { var offenders = new List(); - var joins = 0; + var fanOuts = 0; + var members = 0; + var unattributed = new List(); foreach (var path in ViewerSources()) { @@ -391,35 +531,483 @@ public void EveryViewerFanOut_DeclaresItsWidth() /* Stripped, for the same reason the command scans are: this file's own prose names Task.WhenAll and ViewerReadFanOut.Of repeatedly, and a scan reading raw text would pair a - join against an explanation of a declaration. */ + join against an explanation of a declaration. BraceBalanced below is only correct over + stripped text, which is that method's own stated contract. */ var code = CSharpSourceWalker.StripCommentsAndStrings(text); + var bodies = MemberBodies(code); + members += bodies.Count; + + var joins = s_joinedFanOut.Matches(code).Select(m => m.Index).ToArray(); + var discards = s_fireAndForget.Matches(code).Select(m => m.Index).ToArray(); + var deferred = DeferredReads(code); var declarations = s_fanOutDeclaration.Matches(code).Select(m => m.Index).ToArray(); - var seen = 0; - foreach (Match join in s_joinedFanOut.Matches(code)) + /* A marker inside no recognised member means the member walk missed a declaration shape, and a + fan-out the walk cannot see is exactly the silence this pin exists to break. Reported rather + than skipped. */ + foreach (var index in joins.Concat(discards).Concat(deferred.Select(d => d.Index))) { - joins++; - seen++; + if (Owner(bodies, index) is null) + { + unattributed.Add($"{Path.GetFileName(path)}:{Line(text, index)}"); + } + } + + foreach (var body in bodies) + { + bool Mine(int index) => Owner(bodies, index) is { } owner && owner.Start == body.Start; + + var myJoins = joins.Where(Mine).ToArray(); + var myDiscards = discards.Where(Mine).ToArray(); + var myDeferred = deferred.Where(d => Mine(d.Index)).ToArray(); - if (declarations.Count(index => index < join.Index) < seen) + var markers = new List(); + + if (myJoins.Length > 0) { - var line = text.Take(join.Index).Count(c => c == '\n') + 1; - offenders.Add($"{Path.GetFileName(path)}:{line}"); + markers.Add(myJoins[0]); + } + + if (myDiscards.Length >= 2) + { + markers.Add(myDiscards[0]); + } + + if (ConcurrentRun(code, myDeferred) >= 2) + { + markers.Add(myDeferred[0].Index); + } + + if (markers.Count == 0) + { + continue; + } + + fanOuts++; + var first = markers.Min(); + + if (!declarations.Any(d => Mine(d) && d < first)) + { + offenders.Add($"{Path.GetFileName(path)}:{Line(text, first)} ({body.Name})"); } } } - Assert.True(joins >= 10, $"the fan-out scan matched only {joins} joins — the sweep is not reading the project"); + /* Three floors, because each one fails differently. The member floor catches a member regex that + stopped matching (every fan-out then sits in no member and the sweep asserts over nothing); the + fan-out floor catches the sweep reading an empty or wrong directory; the attribution floor + catches a member walk that reads the project but drops the members the fan-outs are in. 21 + fan-outs across 1,234 member bodies in 191 files when this landed. */ + Assert.True(members >= 900, $"the member walk found only {members} member bodies — it is not reading the project"); + + Assert.True(fanOuts >= 15, $"the fan-out census matched only {fanOuts} fan-out(s) — the sweep is not reading the project"); + + Assert.True( + unattributed.Count == 0, + $"{unattributed.Count} fan-out marker(s) sit inside no recognised member body, so nothing required " + + "them to declare a width. The member signature pattern has stopped matching a declaration shape " + + "this project uses: " + string.Join(", ", unattributed)); Assert.True( offenders.Count == 0, - $"{offenders.Count} joined fan-out(s) run their reads concurrently without declaring a width, so each " + $"{offenders.Count} fan-out(s) run their reads concurrently without declaring a width, so each " + "read is bounded by a ceiling derived from a read measured ALONE — which the ten-wide case sits " + "entirely above. Declare the width with ViewerReadFanOut.Of(n) before the first read: " + string.Join(", ", offenders)); } + /// + /// The concurrency primitives does not model are + /// pinned ABSENT, which is what lets that pin claim "every" rather than "every one of three spellings". + /// + /// This is the ratchet #3019 was really about. A census names the shapes it knows; the failure is + /// not that the list is short but that a shape added later joins it silently and the guard stays green + /// while covering less. Asserting the unmodelled primitives are unused converts that silence into a red + /// build for whoever introduces one, and their options are then to model it above or to declare the + /// width by hand — either way it is a decision someone makes rather than one nobody sees. + /// + [Fact] + public void NoUnmodelledConcurrencyPrimitive_ReachesTheViewer() + { + var offenders = new List(); + + foreach (var path in ViewerSources()) + { + var text = File.ReadAllText(path); + var code = CSharpSourceWalker.StripCommentsAndStrings(text); + + foreach (Match hit in s_unmodelledConcurrency.Matches(code)) + { + offenders.Add($"{Path.GetFileName(path)}:{Line(text, hit.Index)} ({hit.Value.Trim()})"); + } + } + + Assert.True( + offenders.Count == 0, + $"{offenders.Count} site(s) use a concurrency primitive the fan-out census does not model, so a " + + "fan-out spelled that way would never be asked to declare a width. Either teach " + + "EveryViewerFanOut_DeclaresItsWidth the shape, or declare the width at the site and narrow this " + + "pin deliberately: " + string.Join(", ", offenders)); + } + + /// + /// The census classifier, on the shapes that decide whether it is honest. A false negative here is + /// #3019 again; a false positive fails a green build on correct code. + /// + /// The two that matter most are the last two. Sequential start-await-start-await is NOT a + /// fan-out — nothing is in flight together — and a deferred rule that ignored the intervening + /// await would report every such method. Two discards separated by an await ARE a + /// fan-out, because discarding a task means nothing waits for it; the asymmetry between those two + /// cases is the whole reason the deferred shape counts awaits and the discard shape does not. + /// + [Theory] + /* One discard: the project's single-flight refresh, 27 of them, not a fan-out. */ + [InlineData("_ = RefreshServerStatusAsync();\n", false)] + /* Two: MainWindow.LoadServersAsync's connect pair. */ + [InlineData("_ = RefreshServerStatusAsync();\n_ = RefreshStoreSizeAsync();\n", true)] + /* Dotted receiver — the shape the sibling pins' (\w+) form cannot see. */ + [InlineData("_ = Controller.LoadAsync();\n_ = Controller.SaveAsync();\n", true)] + /* A join alone is a fan-out; one occurrence is enough. */ + [InlineData("await Task.WhenAll(a, b);\n", true)] + /* Deferred pair, then awaited — ViewerServerTab.Charts.LoadTempDbAsync. */ + [InlineData("var trendTask = _dataService.GetTempDbTrendAsync(id);\nvar fileIoTask = _dataService.GetTempDbFileIoTrendAsync(id);\nvar trend = await trendTask;\n", true)] + /* One deferred start is not a fan-out. */ + [InlineData("var only = _dataService.GetTempDbTrendAsync(id);\nvar rows = await only;\n", false)] + /* Sequential: each awaited before the next starts, so nothing overlaps. */ + [InlineData("var a = _dataService.GetXAsync(id);\nvar ra = await a;\nvar b = _dataService.GetYAsync(id);\nvar rb = await b;\n", false)] + /* Discards are NOT separated by an await — the discarded task is still running. */ + [InlineData("_ = RefreshServerStatusAsync();\nawait RefreshVisibleAsync();\n_ = PollAlertsAsync();\n", true)] + /* Prose cannot make a fan-out. */ + [InlineData("/* two reads: _ = OneAsync(); _ = TwoAsync(); */\n", false)] + /* Two SYNCHRONOUS discards read as a fan-out — the accepted cost of not constraining the callee, and + the reason that trade is argued on s_fireAndForget rather than left to be rediscovered. Expected + true: this is the classifier's behaviour, not a defect to be fixed by narrowing the shape. Found in + review. */ + [InlineData("_ = TryParseFirst(out var first);\n_ = TryParseSecond(out var second);\n", true)] + public void TheFanOutCensus_RecognisesEachShape_AndOnlyThose(string body, bool expectedFanOut) + { + var code = CSharpSourceWalker.StripCommentsAndStrings("private async Task Fixture()\n{\n" + body + "}\n"); + var bodies = MemberBodies(code); + + Assert.True(bodies.Count == 1, $"the fixture parsed to {bodies.Count} member bodies, not 1"); + + var joins = s_joinedFanOut.Matches(code).Select(m => m.Index).ToArray(); + var discards = s_fireAndForget.Matches(code).Select(m => m.Index).ToArray(); + var deferred = DeferredReads(code); + + var isFanOut = joins.Length > 0 || discards.Length >= 2 || ConcurrentRun(code, deferred) >= 2; + + Assert.Equal(expectedFanOut, isFanOut); + } + + /// + /// The member walk sees a generic method: type parameters after the name, constraints before the body. + /// A member the walk cannot see cannot be required to declare a width, which is #3019's failure + /// reproduced inside the fix for it. + /// + /// The constraint is spelled WITHOUT parentheses on purpose. where T : class, new() + /// contains a ), and the parameter-list group is greedy, so it absorbs the whole clause and the + /// fixture then matches even with constraint support removed — it passes for the wrong reason. The real + /// SettingsWindow.TryReadSectionAsync<T> is where T : class, paren-free, which is + /// the shape that actually needs the clause admitted. The new() spelling is pinned separately + /// below so both live layouts are covered. Found by a mutation that stayed green. + /// + [Theory] + /* Paren-free constraint — SettingsWindow.TryReadSectionAsync. Sensitive to the constraint clause. */ + [InlineData("private static async Task TryReadSectionAsync(Func> read) where T : class\n", "TryReadSectionAsync")] + /* Constraint on its own line, with new() — ViewerSettingsFile.Load and Save. */ + [InlineData("internal static SettingsObjectRead Load(string filePath, JsonSerializerOptions options)\n where T : class, new()\n", "Load")] + /* No constraint, type parameters only. */ + [InlineData("private static Task PassThroughAsync(Task inner)\n", "PassThroughAsync")] + public void TheMemberWalk_SeesAGenericMethod(string signature, string expectedName) + { + var code = CSharpSourceWalker.StripCommentsAndStrings( + signature + + "{\n" + + " var firstTask = _dataService.GetOneAsync();\n" + + " var secondTask = _dataService.GetTwoAsync();\n" + + " return await firstTask ?? await secondTask;\n" + + "}\n"); + + var bodies = MemberBodies(code); + + Assert.True(bodies.Count == 1, $"the generic signature parsed to {bodies.Count} member bodies, not 1"); + Assert.Equal(expectedName, bodies[0].Name); + + /* And the fan-out inside it is attributed to that member rather than landing nowhere. */ + var deferred = DeferredReads(code); + + Assert.True(deferred.Length == 2, $"{deferred.Length} deferred read(s) parsed out of the fixture, not 2"); + Assert.All(deferred, d => Assert.NotNull(Owner(bodies, d.Index))); + Assert.True(ConcurrentRun(code, deferred) >= 2, "the generic method's deferred pair did not read as a fan-out"); + } + + /// + /// An await inside the FIRST call's own argument list does not sequence the pair. The two tasks + /// are still started back to back, so this is a fan-out. + /// + /// Deliberately separate from + /// : measuring the window from the + /// wrong end satisfies one of the two and breaks the other, so a single assertion would call a half-fix + /// proven. That is the lesson the where T : class, new() fixture taught on this same file. + /// + [Fact] + public void TheDeferredRun_IgnoresAnAwaitInsideTheFirstCallsOwnArguments() + { + var code = CSharpSourceWalker.StripCommentsAndStrings( + "var firstTask = _dataService.GetOneAsync(await ResolveIdAsync());\n" + + "var secondTask = _dataService.GetTwoAsync(id);\n"); + + var deferred = DeferredReads(code); + + Assert.True(deferred.Length == 2, $"{deferred.Length} deferred read(s) parsed, not 2"); + + Assert.True( + ConcurrentRun(code, deferred) >= 2, + "an await inside the first call's own arguments broke the run, so a real fan-out reads as " + + "sequential — the window is being measured from the match end instead of the statement end"); + } + + /// + /// An await genuinely BETWEEN the two statements does sequence them: the first task is finished + /// before the second starts, so nothing is in flight together and this is not a fan-out. + /// + [Fact] + public void TheDeferredRun_IsBrokenByAnAwaitBetweenTheTwoStatements() + { + var code = CSharpSourceWalker.StripCommentsAndStrings( + "var firstTask = _dataService.GetOneAsync(id);\n" + + "var first = await firstTask;\n" + + "var secondTask = _dataService.GetTwoAsync(id);\n" + + "var second = await secondTask;\n"); + + var deferred = DeferredReads(code); + + Assert.True(deferred.Length == 2, $"{deferred.Length} deferred read(s) parsed, not 2"); + + Assert.True( + ConcurrentRun(code, deferred) < 2, + "start-await-start-await read as a fan-out; the window has been widened past the intervening " + + "await and the shape is now over-reported"); + } + + /// + /// A deferred read nested inside an earlier statement — a task started in a lambda handed to another + /// started task — does not index the window backwards. No site in this project is written that way; the + /// naive form throws rather than mis-reporting, so it is pinned rather than left to appear. + /// + [Fact] + public void TheDeferredRun_SurvivesAStartNestedInsideAnEarlierStatement() + { + var code = CSharpSourceWalker.StripCommentsAndStrings( + "var outerTask = _dataService.RunAsync(() => { var innerTask = _dataService.GetOneAsync(id); return innerTask; });\n"); + + var deferred = DeferredReads(code); + + Assert.True(deferred.Length == 2, $"{deferred.Length} deferred read(s) parsed, not 2"); + Assert.True(deferred[0].End > deferred[1].Index, "the fixture no longer nests, so it pins nothing"); + + /* The assertion is that this returns at all. */ + Assert.True(ConcurrentRun(code, deferred) >= 1); + } + + /// + /// A join in a member with NO braced body is reported, not dropped. An expression-bodied member is a + /// legal fan-out site and the member walk cannot represent it, so the only thing standing between it + /// and a silent miss is that the census runs tree-wide and attributes afterwards. + /// + /// This asserts the REPORTING path, not coverage: the join is matched, it has no owner, and the + /// sweep therefore lists it as unattributed. That is the honest outcome for a shape the walk cannot + /// pair a declaration against — a loud red naming the line, rather than a vacuous zero. + /// + [Fact] + public void TheCensus_ReportsAJoinInAMemberWithNoBracedBody() + { + var code = CSharpSourceWalker.StripCommentsAndStrings( + "private Task RefreshBothAsync() => Task.WhenAll(LoadOneAsync(), LoadTwoAsync());\n"); + + var joins = s_joinedFanOut.Matches(code).Select(m => m.Index).ToArray(); + var bodies = MemberBodies(code); + + /* Censused: the pattern runs over the file, so the join is seen even though nothing can own it. */ + Assert.True(joins.Length == 1, $"the census matched {joins.Length} join(s) in an expression-bodied member, not 1"); + + Assert.True( + bodies.Count == 0, + $"an expression-bodied member parsed to {bodies.Count} braced body(ies); if the walk starts " + + "representing them, pair the join against it here instead of asserting it is reported"); + + /* And unowned, so EveryViewerFanOut_DeclaresItsWidth adds it to `unattributed` and fails loudly. */ + Assert.Null(Owner(bodies, joins[0])); + } + + /// + /// A FIELD whose initializer is a collection initializer is not a member body. The type-parameter group + /// excludes = and newlines to keep it that way: allowing them ran the pattern from the field's + /// declared type, through = new Dictionary<...>(comparer), to the initializer's brace. + /// + /// A phantom body is worse than a missing one. takes the OUTERMOST match, so + /// one spanning an initializer could swallow a real member's markers and attribute them to something + /// that can never declare a width — a fan-out reported at a line where no edit makes the build pass. + /// This is CollectorSchedulePresets.Presets's real shape. + /// + [Fact] + public void TheMemberWalk_DoesNotReadAFieldInitializerAsAMember() + { + var code = CSharpSourceWalker.StripCommentsAndStrings( + "public static readonly IReadOnlyDictionary> Presets =\n" + + " new Dictionary>(StringComparer.OrdinalIgnoreCase)\n" + + " {\n" + + " [\"Aggressive\"] = new Dictionary(StringComparer.OrdinalIgnoreCase)\n" + + " {\n" + + " [\"wait_stats\"] = 1,\n" + + " },\n" + + " };\n"); + + var bodies = MemberBodies(code); + + Assert.True( + bodies.Count == 0, + "a field's collection initializer parsed as " + + $"{bodies.Count} member body(ies) ({string.Join(", ", bodies.Select(b => b.Name))}); Owner takes the " + + "outermost body, so a phantom one here would claim a real member's fan-out markers"); + } + + /// + /// The discard and deferred shapes never both claim one call. var _ = SomeAsync(); is a DISCARD — + /// nothing holds the task — and counting it twice would let one physical call inflate two separate + /// fan-out tallies. + /// + [Fact] + public void TheDiscardAndDeferredShapes_StayDisjoint() + { + var code = CSharpSourceWalker.StripCommentsAndStrings("var _ = RefreshServerStatusAsync();\n"); + + Assert.True(s_fireAndForget.Matches(code).Count == 1, "the discard shape did not claim `var _ = SomeAsync();`"); + Assert.True(s_deferredRead.Matches(code).Count == 0, "the deferred shape also claimed `var _ = SomeAsync();`, so one call is double-booked"); + + /* The ordinary deferred spelling still reads as deferred and NOT as a discard, so the exclusion + above narrowed the right one. */ + var named = CSharpSourceWalker.StripCommentsAndStrings("var statusTask = RefreshServerStatusAsync();\n"); + + Assert.True(s_deferredRead.Matches(named).Count == 1, "the deferred shape stopped matching a named task start"); + Assert.True(s_fireAndForget.Matches(named).Count == 0, "the discard shape claimed a named task start"); + } + + /// + /// The longest run of deferred reads with no await between consecutive STATEMENTS of the run — + /// the number actually in flight together. Two starts either side of an await are sequential and + /// must not count; see . + /// + /// The window starts at the end of the previous STATEMENT, not at the end of its regex + /// match. ends on the call's opening paren, so a window measured from + /// there spans the rest of that call's own argument list — and an await in those arguments + /// (var t1 = FooAsync(await Bar());) would break the run and silently drop a real fan-out. That + /// is #3019's own failure inside the detector for it, and it applied to all 63 deferred sites in the + /// project rather than to an edge case. therefore carries the statement end. + /// Found in review. + /// + private static int ConcurrentRun(string code, (int Index, int End)[] deferred) + { + var best = 0; + var run = 0; + var previousEnd = -1; + + foreach (var (index, end) in deferred) + { + if (previousEnd < 0) + { + run = 1; + } + else + { + /* previousEnd can sit PAST the next start, when that start is nested inside the previous + statement — a task started inside a lambda handed to another. There is no "between" to + scan and both are in flight, so the run continues rather than indexing backwards, which + would throw. */ + var between = previousEnd <= index ? code[previousEnd..index] : string.Empty; + + run = s_await.IsMatch(between) ? 1 : run + 1; + } + + best = System.Math.Max(best, run); + + /* Monotonic, so a nested statement's earlier end cannot rewind the enclosing one's. */ + previousEnd = System.Math.Max(previousEnd, end); + } + + return best; + } + + /// + /// Every deferred read in , each carrying the end of the STATEMENT it starts + /// rather than the end of its match. Shared by the sweep and its fixtures deliberately: a fixture + /// computing the window differently from the sweep would pin a property the sweep does not have. + /// + private static (int Index, int End)[] DeferredReads(string code) + { + return s_deferredRead.Matches(code) + .Select(match => + { + /* The match ends ON the opening paren, which is what EndOfParenthesisedStatement wants; + it counts parens, so a lambda argument cannot terminate the statement early. */ + var end = EndOfParenthesisedStatement(code, match.Index + match.Length - 1); + + return (match.Index, End: end < 0 ? match.Index + match.Length : end); + }) + .ToArray(); + } + + /// + /// Every member body in , which must be + /// 's output — a brace in prose or in a literal + /// is exactly what would unbalance the walk. + /// + private static List<(int Start, int End, string Name)> MemberBodies(string code) + { + var bodies = new List<(int Start, int End, string Name)>(); + + foreach (Match signature in s_memberSignature.Matches(code)) + { + /* The signature match ends ON the body brace, so search from there rather than from the + parameter list: a parameter default can carry a brace. */ + var open = code.IndexOf('{', signature.Index + signature.Length - 1); + + if (open < 0) + { + continue; + } + + bodies.Add((open, open + CSharpSourceWalker.BraceBalanced(code, open).Length, signature.Groups["name"].Value)); + } + + return bodies; + } + + /// + /// The OUTERMOST member body containing , or null. Outermost so that a read + /// inside a lambda or a local function is attributed to the method that fans it out, which is where the + /// width has to be declared. + /// + private static (int Start, int End, string Name)? Owner(List<(int Start, int End, string Name)> bodies, int index) + { + (int Start, int End, string Name)? owner = null; + + foreach (var body in bodies) + { + if (index > body.Start && index < body.End && (owner is null || body.Start < owner.Value.Start)) + { + owner = body; + } + } + + return owner; + } + + /// The 1-based line of in . + private static int Line(string text, int index) => text.Take(index).Count(c => c == '\n') + 1; + /// /// A declared width must not outlive the reads it describes. A method-scoped using var runs to /// the closing brace, so a store read AFTER the join inherits a contention count that is over by the @@ -435,6 +1023,14 @@ join against an explanation of a declaration. */ /// level: MainWindow's fleet-totals read sits inside a try block, so a same-depth rule /// would miss a real one. Both shapes are live in this project, and each rules out one of the two /// obvious implementations. + /// + /// Joined fan-outs only, unlike + /// . "Outlives its join" needs a join to be + /// measured against, and the unjoined shapes have none — MainWindow.OnRefreshTimerTick + /// deliberately holds its scope to the end of the tick, because the visible-tab load below really does + /// contend with the reads still in flight. So the release discipline is not merely unchecked for those + /// shapes, it is a different question there with a different answer, and widening this scan to reach + /// them would report that deliberate choice as a defect (#3019). /// [Fact] public void NoFanOutScope_OutlivesItsJoin() diff --git a/Darling/PerformanceMonitor.Darling.Viewer/MainWindow.xaml.cs b/Darling/PerformanceMonitor.Darling.Viewer/MainWindow.xaml.cs index 954ee94eb6..fb007c85eb 100644 --- a/Darling/PerformanceMonitor.Darling.Viewer/MainWindow.xaml.cs +++ b/Darling/PerformanceMonitor.Darling.Viewer/MainWindow.xaml.cs @@ -1898,6 +1898,9 @@ the CSV export separator (next export) and the Server/Local/UTC time-display mod _minimizeToTray live). Minimize-to-tray is viewer-local (the reloaded file); the alerts-master + "Tray notification cooldown" the window just wrote to the STORE are re-read from there. */ _minimizeToTray = reloaded.MinimizeToTray; + /* The toast-settings read and the display-mode tab reload below are both fired unawaited, so they + are in flight together whenever the mode changed. */ + using var readFanOut = ViewerReadFanOut.Of(2); _ = RefreshAlertToastSettingsFromStoreAsync(); /* Re-apply the fleet refresh interval to the live shell timers so a change takes effect immediately (the connection timeout is a connect-time setting and applies on the next viewer launch). */ diff --git a/Darling/PerformanceMonitor.Darling.Viewer/ViewerServerTab.Blocking.cs b/Darling/PerformanceMonitor.Darling.Viewer/ViewerServerTab.Blocking.cs index 4f8290e0c4..9a23e84de4 100644 --- a/Darling/PerformanceMonitor.Darling.Viewer/ViewerServerTab.Blocking.cs +++ b/Darling/PerformanceMonitor.Darling.Viewer/ViewerServerTab.Blocking.cs @@ -134,6 +134,9 @@ private async Task LoadBlockingAsync() switch (BlockingSubTabs.SelectedIndex) { case BlockingTrendsSubTabIndex: + { + using var readFanOut = ViewerReadFanOut.Of(3); + var lockWaitTask = _dataService.GetLockWaitTrendAsync(_server.ServerId, startUtc, endUtc); var blockingTask = _dataService.GetBlockingTrendAsync(_server.ServerId, startUtc, endUtc, databaseNames: SelectedDatabaseFilter); var deadlockTask = _dataService.GetDeadlockTrendAsync(_server.ServerId, startUtc, endUtc); @@ -144,7 +147,11 @@ private async Task LoadBlockingAsync() RenderBlockingTrendChart(blocking); RenderDeadlockTrendChart(deadlocks); break; + } case BlockingStatsSubTabIndex: + { + using var readFanOut = ViewerReadFanOut.Of(3); + /* Blocking SEVERITY: the duration aggregate reconciles with the count trend (same XE→DMV source selection); the deadlock COUNT is the cheap sibling of the Trends tab's deadlock trend, summed here for the summary strip. The deadlock SEVERITY aggregate (victim_count + @@ -162,7 +169,11 @@ private async Task LoadBlockingAsync() RenderDeadlockTotalWaitChart(deadlockSeverity); UpdateBlockingStatsSummary(durationStats, deadlockCounts, deadlockSeverity); break; + } case BlockingCurrentWaitsSubTabIndex: + { + using var readFanOut = ViewerReadFanOut.Of(2); + var durationTask = _dataService.GetWaitingTaskTrendAsync(_server.ServerId, startUtc, endUtc); var blockedTask = _dataService.GetBlockedSessionTrendAsync(_server.ServerId, startUtc, endUtc, databaseNames: SelectedDatabaseFilter); var duration = await durationTask; @@ -170,6 +181,7 @@ private async Task LoadBlockingAsync() RenderCurrentWaitsDurationChart(duration); RenderCurrentWaitsBlockedChart(blocked); break; + } case BlockedProcessReportsSubTabIndex: await LoadBlockedProcessReportsAsync(startUtc, endUtc); break; diff --git a/Darling/PerformanceMonitor.Darling.Viewer/ViewerServerTab.Charts.cs b/Darling/PerformanceMonitor.Darling.Viewer/ViewerServerTab.Charts.cs index d0ed049adf..b7bf6f3987 100644 --- a/Darling/PerformanceMonitor.Darling.Viewer/ViewerServerTab.Charts.cs +++ b/Darling/PerformanceMonitor.Darling.Viewer/ViewerServerTab.Charts.cs @@ -119,6 +119,7 @@ private async Task LoadTempDbAsync() var (startUtc, endUtc) = GetWindowUtc(); /* Both reads run concurrently — NpgsqlDataSource pools a connection for each. */ + using var readFanOut = ViewerReadFanOut.Of(2); var trendTask = _dataService.GetTempDbTrendAsync(_server.ServerId, startUtc); var fileIoTask = _dataService.GetTempDbFileIoTrendAsync(_server.ServerId, startUtc); var trend = await trendTask; diff --git a/Darling/PerformanceMonitor.Darling.Viewer/ViewerServerTab.RunningJobs.cs b/Darling/PerformanceMonitor.Darling.Viewer/ViewerServerTab.RunningJobs.cs index 0097d32833..300ecd76d7 100644 --- a/Darling/PerformanceMonitor.Darling.Viewer/ViewerServerTab.RunningJobs.cs +++ b/Darling/PerformanceMonitor.Darling.Viewer/ViewerServerTab.RunningJobs.cs @@ -30,6 +30,7 @@ public partial class ViewerServerTab /// private async Task LoadRunningJobsAsync() { + using var readFanOut = ViewerReadFanOut.Of(2); var jobsTask = _dataService.GetRunningJobsAsync(_server.ServerId); var statusTask = _dataService.GetLatestRunningJobsCollectorStatusAsync(_server.ServerId); var jobs = await jobsTask;