Crashes and loss of SyntaxNode tracking when enumerating descendant nodes of the original syntax root

[jnm2]

Version Used: 3.4.0 nupkg

Summary

This only happens in some files. So far, on files between 134 and 1300 lines and all files I don't have permission to share. Trimming the files down causes the repro to be lost I'll try to work on this. Maybe the information I have so far will help, though.

➡ The syntaxNode.DescendantNodes() enumerable starts returning a different series of SyntaxNode instances when enumerated after TrackNodes. This causes GetCurrentNode calls to return null because these node instances have never been seen before.

➡ This is rare, but one way to make it repro almost every time is to then await document.GetSemanticModelAsync and enumerate again after that. Most of the returned SyntaxNode instances to be never seen before.

➡ Occasionally (six out of a hundred runs so far?), TrackNodes even crashes internally inside Microsoft.CodeAnalysis.GreenNode..ctor:

System.ArgumentException: '
Parameter name: annotations'

With this stack trace:

   at Microsoft.CodeAnalysis.GreenNode..ctor(UInt16 kind, DiagnosticInfo[] diagnostics, SyntaxAnnotation[] annotations)
   at Microsoft.CodeAnalysis.CSharp.Syntax.InternalSyntax.IdentifierNameSyntax.SetAnnotations(SyntaxAnnotation[] annotations)
   at Microsoft.CodeAnalysis.GreenNodeExtensions.WithAdditionalAnnotationsGreen[TNode](TNode node, IEnumerable`1 annotations)
   at Microsoft.CodeAnalysis.SyntaxNode.WithAdditionalAnnotationsInternal(IEnumerable`1 annotations)
   at Microsoft.CodeAnalysis.AnnotationExtensions.WithAdditionalAnnotations[TNode](TNode node, SyntaxAnnotation[] annotations)
   at Microsoft.CodeAnalysis.SyntaxNodeExtensions.<>c__37`1.<TrackNodes>b__37_0(SyntaxNode n, SyntaxNode r)
   at Microsoft.CodeAnalysis.CSharp.Syntax.SyntaxReplacer.Replacer`1.Visit(SyntaxNode node)
   at Microsoft.CodeAnalysis.CSharp.CSharpSyntaxRewriter.VisitDeclarationExpression(DeclarationExpressionSyntax node)
   at Microsoft.CodeAnalysis.CSharp.CSharpSyntaxRewriter.Visit(SyntaxNode node)
   at Microsoft.CodeAnalysis.CSharp.Syntax.SyntaxReplacer.Replacer`1.Visit(SyntaxNode node)
   at Microsoft.CodeAnalysis.CSharp.CSharpSyntaxRewriter.VisitAssignmentExpression(AssignmentExpressionSyntax node)
   at Microsoft.CodeAnalysis.CSharp.CSharpSyntaxRewriter.Visit(SyntaxNode node)
   at Microsoft.CodeAnalysis.CSharp.Syntax.SyntaxReplacer.Replacer`1.Visit(SyntaxNode node)
   at Microsoft.CodeAnalysis.CSharp.CSharpSyntaxRewriter.VisitExpressionStatement(ExpressionStatementSyntax node)
   at Microsoft.CodeAnalysis.CSharp.CSharpSyntaxRewriter.Visit(SyntaxNode node)
   at Microsoft.CodeAnalysis.CSharp.Syntax.SyntaxReplacer.Replacer`1.Visit(SyntaxNode node)
   at Microsoft.CodeAnalysis.CSharp.CSharpSyntaxRewriter.VisitListElement[TNode](TNode node)
   at Microsoft.CodeAnalysis.CSharp.CSharpSyntaxRewriter.VisitList[TNode](SyntaxList`1 list)
   at Microsoft.CodeAnalysis.CSharp.CSharpSyntaxRewriter.VisitBlock(BlockSyntax node)
   at Microsoft.CodeAnalysis.CSharp.CSharpSyntaxRewriter.Visit(SyntaxNode node)
   at Microsoft.CodeAnalysis.CSharp.Syntax.SyntaxReplacer.Replacer`1.Visit(SyntaxNode node)
   at Microsoft.CodeAnalysis.CSharp.CSharpSyntaxRewriter.VisitAccessorDeclaration(AccessorDeclarationSyntax node)
   at Microsoft.CodeAnalysis.CSharp.CSharpSyntaxRewriter.Visit(SyntaxNode node)
   at Microsoft.CodeAnalysis.CSharp.Syntax.SyntaxReplacer.Replacer`1.Visit(SyntaxNode node)
   at Microsoft.CodeAnalysis.CSharp.CSharpSyntaxRewriter.VisitListElement[TNode](TNode node)
   at Microsoft.CodeAnalysis.CSharp.CSharpSyntaxRewriter.VisitList[TNode](SyntaxList`1 list)
   at Microsoft.CodeAnalysis.CSharp.CSharpSyntaxRewriter.VisitAccessorList(AccessorListSyntax node)
   at Microsoft.CodeAnalysis.CSharp.CSharpSyntaxRewriter.Visit(SyntaxNode node)
   at Microsoft.CodeAnalysis.CSharp.Syntax.SyntaxReplacer.Replacer`1.Visit(SyntaxNode node)
   at Microsoft.CodeAnalysis.CSharp.CSharpSyntaxRewriter.VisitPropertyDeclaration(PropertyDeclarationSyntax node)
   at Microsoft.CodeAnalysis.CSharp.CSharpSyntaxRewriter.Visit(SyntaxNode node)
   at Microsoft.CodeAnalysis.CSharp.Syntax.SyntaxReplacer.Replacer`1.Visit(SyntaxNode node)
   at Microsoft.CodeAnalysis.CSharp.CSharpSyntaxRewriter.VisitListElement[TNode](TNode node)
   at Microsoft.CodeAnalysis.CSharp.CSharpSyntaxRewriter.VisitList[TNode](SyntaxList`1 list)
   at Microsoft.CodeAnalysis.CSharp.CSharpSyntaxRewriter.VisitClassDeclaration(ClassDeclarationSyntax node)
   at Microsoft.CodeAnalysis.CSharp.CSharpSyntaxRewriter.Visit(SyntaxNode node)
   at Microsoft.CodeAnalysis.CSharp.Syntax.SyntaxReplacer.Replacer`1.Visit(SyntaxNode node)
   at Microsoft.CodeAnalysis.CSharp.CSharpSyntaxRewriter.VisitListElement[TNode](TNode node)
   at Microsoft.CodeAnalysis.CSharp.CSharpSyntaxRewriter.VisitList[TNode](SyntaxList`1 list)
   at Microsoft.CodeAnalysis.CSharp.CSharpSyntaxRewriter.VisitNamespaceDeclaration(NamespaceDeclarationSyntax node)
   at Microsoft.CodeAnalysis.CSharp.CSharpSyntaxRewriter.Visit(SyntaxNode node)
   at Microsoft.CodeAnalysis.CSharp.Syntax.SyntaxReplacer.Replacer`1.Visit(SyntaxNode node)
   at Microsoft.CodeAnalysis.CSharp.CSharpSyntaxRewriter.VisitListElement[TNode](TNode node)
   at Microsoft.CodeAnalysis.CSharp.CSharpSyntaxRewriter.VisitList[TNode](SyntaxList`1 list)
   at Microsoft.CodeAnalysis.CSharp.CSharpSyntaxRewriter.VisitCompilationUnit(CompilationUnitSyntax node)
   at Microsoft.CodeAnalysis.CSharp.CSharpSyntaxRewriter.Visit(SyntaxNode node)
   at Microsoft.CodeAnalysis.CSharp.Syntax.SyntaxReplacer.Replacer`1.Visit(SyntaxNode node)
   at Microsoft.CodeAnalysis.CSharp.CSharpSyntaxNode.ReplaceCore[TNode](IEnumerable`1 nodes, Func`3 computeReplacementNode, IEnumerable`1 tokens, Func`3 computeReplacementToken, IEnumerable`1 trivia, Func`3 computeReplacementTrivia)
   at Microsoft.CodeAnalysis.SyntaxNodeExtensions.ReplaceNodes[TRoot,TNode](TRoot root, IEnumerable`1 nodes, Func`3 computeReplacementNode)

Repro

Console app:

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFramework>net48</TargetFramework>
  </PropertyGroup>

  <ItemGroup>
    <PackageReference Include="Microsoft.CodeAnalysis.CSharp.Features" Version="4.6.0" />
  </ItemGroup>

</Project>

using Microsoft.CodeAnalysis;
using System;
using System.IO;
using System.Linq;
using System.Runtime.CompilerServices;
using System.Threading.Tasks;

public static class Program
{
    public static async Task Main()
    {
        const string filePath = @"C:\Users\Joseph\Source\Repos\roslyn\src\Features\Core\Portable\AddImport\AbstractAddImportFeatureService.cs";

        while (true)
        {
            using (var workspace = new AdhocWorkspace())
            {
                var document = workspace
                    .AddProject("TestProject", LanguageNames.CSharp)
                    .AddDocument("TestDocument.cs", File.ReadAllText(filePath));

                var syntaxRoot = await document.GetSyntaxRootAsync().ConfigureAwait(false);

                var descendantsEnumerable = syntaxRoot.DescendantNodes().Where(node => true)
                    .Select((node, index) =>
                    {
                        Console.WriteLine($"Enumerating index [{index}], object reference {RuntimeHelpers.GetHashCode(node)}");
                        return node;
                    });

                Console.WriteLine("Calling TrackNodes");
                var trackingSyntaxRoot = syntaxRoot.TrackNodes(descendantsEnumerable);
                Console.WriteLine("Returned from TrackNodes");

                Console.WriteLine("Tracking lost on " + descendantsEnumerable.Count(node => trackingSyntaxRoot.GetCurrentNode(node) is null) + " nodes");

                Console.WriteLine("Calling GetSemanticModelAsync");
                await document.GetSemanticModelAsync();
                Console.WriteLine("Returned from GetSemanticModelAsync");

                Console.WriteLine("Tracking lost on " + descendantsEnumerable.Count(node => trackingSyntaxRoot.GetCurrentNode(node) is null) + " nodes");
            }
        }
    }
}

Repro output

On the 134-line file, one run printed:

Tracking lost on 323 nodes
[...]
Calling GetSemanticModelAsync
Returned from GetSemanticModelAsync
[...]
Tracking lost on 375 nodes

On the next run instead of 323 and 375, it was 0 and 375.

Here's some visual evidence that the enumerator is returning chunks of object references that are different between the earlier and later enumerations of the same syntaxRoot.Descendants() enumerable instance.

This is comparing the object references printed during the enumeration caused when the enumerable was passed to TraceNodes with the object references printed in the .Count( call immediately afterwards, before even getting to await document.GetSemanticModelAsync:

The diff on the left shows the changes in the object references returned from the enumerator for the f first .Count call, before GetSemanticModelAsync (323 object instances changed).

The diff on the right shows the changes in the object references returned from the enumerator for the f second .Count call, after GetSemanticModelAsync (375 object instances changed).

Use case

I discovered this while trying to write a test in https://github.com/jnm2/SuppressionCleanupTool that would force me to stop my accidental multiple enumeration of an enumerable based on syntaxRoot.DescendantNodes.

I was unable to pare down repro syntax for my test, so I tried looking at what the logic could be behind GetCurrentNode returning null even though I was using multiple enumeration. Two and a half hours later, here I was.

[jnm2]

Repro update: one way to repro the GreenNode crash every time is to use https://github.com/dotnet/roslyn/blob/master/src/Compilers/CSharp/Portable/Compiler/MethodCompiler.cs as the document text.

[jnm2]

Repro update: I got a repro of the non-crashing scenario! 🎉

I used https://github.com/dotnet/roslyn/blob/master/src/Features/Core/Portable/AddImport/AbstractAddImportFeatureService.cs.

Full output:
repro_output.txt

Tracking was lost on 0 out of 2459 nodes before await document.GetSemanticModelAsync() and was lost on 1616 nodes after it.

(For all I know, awaiting GetSemanticModelAsync may be merely functioning as a delay. )

[bernd5]

Code like node.TrackNodes(node.DescendantNodesAndSelf()); seems to cause the ArgumentException because nodes which are not referenced somewhere else are maybe collected in TrackNodes.
This causes that those nodes are removed from the ConditionalWeakTable - and the call to GetId(n) returns null.