Fix StackOverflow crash on cyclic local initializers (MA0091/MA0092/MA0093) - #1341
Merged
Merged
Conversation
EventReferenceVisitor.FindFromLocalSymbol resolved a local's initializer and recursed when it bound to another local, with no visited set. Two locals whose initializers reference each other made the walk recurse until the process died. A StackOverflowException cannot be caught by the analyzer host, so this killed VBCSCompiler, Visual Studio, Rider or OmniSharp instead of being reported as AD0001. The source that triggers it does not compile (CS0841), which is exactly the state of a file while it is being typed, and IDE live analysis runs continuously on erroneous compilations. Track the visited locals in a HashSet<ISymbol> and bail on re-entry. While there, use the real CancellationToken instead of CancellationToken.None: with a cycle present, cancellation was the only thing that could have stopped the walk. ProjectBuilder.IsValidCode, set by WithNoCompilation(), was declared but never read: GetSortedDiagnostics hardcoded compileSolution: true, so no analyzer test could use non-compiling source. Pass IsValidCode so the regression tests can cover it. No existing test used WithNoCompilation().
meziantou
enabled auto-merge (squash)
August 26, 2026 18:37
This was referenced Aug 26, 2026
Closed
Bump Meziantou.Analyzer from 3.0.103 to 3.0.182
Analogy-LogViewer/Analogy.LogViewer.OpenTelemetry#98
Closed
Closed
Closed
Closed
Bump Meziantou.Analyzer from 3.0.139 to 3.0.189
Analogy-LogViewer/Analogy.LogViewer.NLog.Targets#556
Closed
This was referenced Aug 31, 2026
Open
Open
Open
Open
Open
Open
Open
Open
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1323
What changed
EventsShouldHaveProperArgumentsAnalyzer.EventReferenceVisitor.FindFromLocalSymbolresolved a local's initializer and, when that initializer bound to anotherILocalSymbol, called itself — with no visited set. Two locals whose initializers reference each other made the walk recurse until the process died:FindFromLocalSymbolnow tracks the visited locals in aHashSet<ISymbol>(withSymbolEqualityComparer.Default) held by the visitor instance and bails on re-entry.The two hardcoded
CancellationToken.Nonearguments are also gone: the visitor now takes the realcontext.CancellationTokenthrough a primary constructor, so the token parameter disappears from the recursive method's signature. With a cycle present, cancellation was the only thing that could have stopped the walk.Why it matters
A
StackOverflowExceptioncannot be caught by Roslyn's analyzer host, so this was not a recoverableAD0001— the whole process died (VBCSCompiler, Visual Studio, Rider, OmniSharp). MA0091, MA0092 and MA0093 are all enabled by default at Warning.The source that triggers it does not compile (CS0841, use before declaration), which is exactly the state of a file while it is being typed — and IDE live analysis runs continuously on erroneous compilations.
Test infrastructure
ProjectBuilder.IsValidCode, set byWithNoCompilation(), was declared but never read:GetSortedDiagnosticshardcodedcompileSolution: true, so no analyzer test could use non-compiling source (Assert.Fail("The code doesn't compile.")). It now passesIsValidCode, which is what makes the regression tests possible. No existing test usedWithNoCompilation(), so nothing else changes behavior.Tests
Two regression tests: the mutual
a = b; b = a;cycle from the issue, and a self-referencingEventHandler a = a;.I confirmed the new test actually reproduces the bug — with the visited-set guard temporarily removed, the run died exactly as reported in the issue (
Test run summary: Zero tests ran, exit code 7,createdump). Guard restored, it passes.EventsShouldHaveProperArgumentsAnalyzerTestspass on all five Roslyn versions (4.8, 4.14, 5.0, 5.6, 5.9)dotnet run --project src/DocumentationGeneratorproduced no markdown changes