From 330b8d99d0b1e3947e10a6860fee88e1132e91be Mon Sep 17 00:00:00 2001 From: Chris Sienkiewicz Date: Fri, 4 Sep 2026 12:00:52 -0700 Subject: [PATCH 1/2] Make Razor tag helper resolution safe for concurrent inputs Classic rzc processes .cshtml and .razor inputs in parallel through a shared RazorProjectEngine. A phase-wide resolver field lets one document overwrite another document's file-kind strategy, producing component IR for a legacy code target. Keep resolver selection in the per-execution resolution context and pass it through fallback unwrapping. Add a mixed MVC/component concurrency test and document that phase instances cannot own per-document mutable state. Fixes dotnet/roslyn#85132 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: d07d252c-d5c2-43ae-ae7c-e56c4624e953 --- .github/instructions/Razor.instructions.md | 3 ++ .../DefaultTagHelperResolutionPhaseTest.cs | 49 +++++++++++++++++-- .../DefaultTagHelperResolutionPhase.cs | 36 ++++++++------ 3 files changed, 68 insertions(+), 20 deletions(-) diff --git a/.github/instructions/Razor.instructions.md b/.github/instructions/Razor.instructions.md index 39d949aa8ce67..c5302c5680581 100644 --- a/.github/instructions/Razor.instructions.md +++ b/.github/instructions/Razor.instructions.md @@ -36,6 +36,9 @@ their original sub-tree layout fields through the constructor. When adding a new field, thread it through every existing `With*` method. Prefer computing derived data via extension methods (e.g., `GetUnusedDirectives()`) rather than storing computed results as fields. +- **Razor engine concurrency**: Hosts can process multiple documents concurrently through one + `RazorProjectEngine`. Phase and pass instances are shared, so keep per-document state in locals + or an execution context rather than mutable instance fields. - **Razor documents in Roslyn**: Stored as additional documents. Resolve via `solution.GetDocumentIdsWithFilePath(filePath)` then `solution.GetAdditionalDocument(documentId)`. - **Razor documents with virtual URIs**: Remote Razor document classification preserves the full diff --git a/src/Razor/src/Compiler/Microsoft.AspNetCore.Razor.Language/test/DefaultTagHelperResolutionPhaseTest.cs b/src/Razor/src/Compiler/Microsoft.AspNetCore.Razor.Language/test/DefaultTagHelperResolutionPhaseTest.cs index 2e80286a53770..72f16bdbdf25b 100644 --- a/src/Razor/src/Compiler/Microsoft.AspNetCore.Razor.Language/test/DefaultTagHelperResolutionPhaseTest.cs +++ b/src/Razor/src/Compiler/Microsoft.AspNetCore.Razor.Language/test/DefaultTagHelperResolutionPhaseTest.cs @@ -1,19 +1,58 @@ -// Licensed to the .NET Foundation under one or more agreements. +// Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System.Collections.Immutable; +using System.Threading.Tasks; using Xunit; namespace Microsoft.AspNetCore.Razor.Language; public class DefaultTagHelperResolutionPhaseTest { + [Fact] + public void Process_ConcurrentMixedFileKinds_UsesCorrectResolver() + { + // Arrange + var configuration = new RazorConfiguration( + RazorLanguageVersion.Version_5_0, + "MVC-3.0", + Extensions: []); + + var projectEngine = RazorProjectEngine.Create( + configuration, + RazorProjectFileSystem.Empty, + builder => builder.RegisterExtensions()); + + var inputs = new[] + { + (Source: RazorSourceDocument.Create("

View

", "/Views/Index.cshtml"), FileKind: RazorFileKind.Legacy), + (Source: RazorSourceDocument.Create("

Component

", "/Components/App.razor"), FileKind: RazorFileKind.Component), + }; + + // Act and assert + Parallel.For(0, 10_000, iteration => + { + var (source, fileKind) = inputs[iteration % inputs.Length]; + var codeDocument = projectEngine.Process( + source, + fileKind, + ImmutableArray.Empty, + tagHelpers: null); + + var generatedCode = codeDocument.GetCSharpDocument().GeneratedCode.ToString(); + Assert.Contains( + fileKind == RazorFileKind.Component ? "BuildRenderTree" : "ExecuteAsync", + generatedCode); + }); + } + [Fact] public void MergeSourceSpans_SameLine_ReturnsCorrectSpan() { // Arrange var filePath = "test.razor"; var first = new SourceSpan(filePath, absoluteIndex: 10, lineIndex: 2, characterIndex: 5, length: 3, lineCount: 0, endCharacterIndex: 8); - var last = new SourceSpan(filePath, absoluteIndex: 15, lineIndex: 2, characterIndex: 10, length: 4, lineCount: 0, endCharacterIndex: 14); + var last = new SourceSpan(filePath, absoluteIndex: 15, lineIndex: 2, characterIndex: 10, length: 4, lineCount: 0, endCharacterIndex: 14); // Act var result = DefaultTagHelperResolutionPhase.MergeSourceSpans(first, last); @@ -36,7 +75,7 @@ public void MergeSourceSpans_MultiLine_ReturnsCorrectSpan() // first spans lines 1-2 (lineCount = 1 means it crosses into the next line) var first = new SourceSpan(filePath, absoluteIndex: 0, lineIndex: 1, characterIndex: 0, length: 10, lineCount: 1, endCharacterIndex: 5); // last is on line 3 (lineIndex = 3) - var last = new SourceSpan(filePath, absoluteIndex: 20, lineIndex: 3, characterIndex: 2, length: 5, lineCount: 0, endCharacterIndex: 7); + var last = new SourceSpan(filePath, absoluteIndex: 20, lineIndex: 3, characterIndex: 2, length: 5, lineCount: 0, endCharacterIndex: 7); // Act var result = DefaultTagHelperResolutionPhase.MergeSourceSpans(first, last); @@ -58,7 +97,7 @@ public void MergeSourceSpans_AdjacentSpans_ReturnsCorrectSpan() var filePath = "test.razor"; var first = new SourceSpan(filePath, absoluteIndex: 5, lineIndex: 0, characterIndex: 5, length: 3, lineCount: 0, endCharacterIndex: 8); // last starts right where first ends - var last = new SourceSpan(filePath, absoluteIndex: 8, lineIndex: 0, characterIndex: 8, length: 4, lineCount: 0, endCharacterIndex: 12); + var last = new SourceSpan(filePath, absoluteIndex: 8, lineIndex: 0, characterIndex: 8, length: 4, lineCount: 0, endCharacterIndex: 12); // Act var result = DefaultTagHelperResolutionPhase.MergeSourceSpans(first, last); @@ -94,7 +133,7 @@ public void MergeSourceSpans_NullFilePath_PreservesNullFilePath() { // Arrange — file path is null (e.g. for in-memory content) var first = new SourceSpan(filePath: null, absoluteIndex: 0, lineIndex: 0, characterIndex: 0, length: 3, lineCount: 0, endCharacterIndex: 3); - var last = new SourceSpan(filePath: null, absoluteIndex: 5, lineIndex: 0, characterIndex: 5, length: 2, lineCount: 0, endCharacterIndex: 7); + var last = new SourceSpan(filePath: null, absoluteIndex: 5, lineIndex: 0, characterIndex: 5, length: 2, lineCount: 0, endCharacterIndex: 7); // Act var result = DefaultTagHelperResolutionPhase.MergeSourceSpans(first, last); diff --git a/src/Razor/src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Language/DefaultTagHelperResolutionPhase.cs b/src/Razor/src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Language/DefaultTagHelperResolutionPhase.cs index 5932ffe6a7f06..574c93e7ad6c5 100644 --- a/src/Razor/src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Language/DefaultTagHelperResolutionPhase.cs +++ b/src/Razor/src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Language/DefaultTagHelperResolutionPhase.cs @@ -24,8 +24,6 @@ namespace Microsoft.AspNetCore.Razor.Language; /// internal partial class DefaultTagHelperResolutionPhase : RazorEnginePhaseBase { - private TagHelperResolver _resolver; - /// /// Entry point: resolves all unresolved nodes /// in the IR tree. For each, matches against tag helper bindings and either converts to a @@ -50,7 +48,7 @@ protected override RazorCodeDocument ExecuteCore(RazorCodeDocument codeDocument, // Choose resolver based on file kind and language version. Component features // (MarkupElementIntermediateNode, RZ10012 diagnostics) require Version_3_0+ because // the ComponentDocumentClassifierPass is only registered at that version. - _resolver = (codeDocument.FileKind.IsComponent() || codeDocument.FileKind.IsComponentImport()) + TagHelperResolver resolver = (codeDocument.FileKind.IsComponent() || codeDocument.FileKind.IsComponentImport()) && parserOptions.LanguageVersion >= RazorLanguageVersion.Version_3_0 ? new ComponentTagHelperResolver() : new LegacyTagHelperResolver(); @@ -58,7 +56,7 @@ protected override RazorCodeDocument ExecuteCore(RazorCodeDocument codeDocument, if (tagHelperContext == null || tagHelperContext.TagHelpers is []) { // No tag helpers discovered - unwrap all UnresolvedElement nodes to their fallback. - UnwrapAllElements(documentNode, documentNode); + UnwrapAllElements(documentNode, resolver, documentNode); // Still need to set referenced tag helpers for downstream phases. return codeDocument.WithReferencedTagHelpers([]); @@ -69,7 +67,7 @@ protected override RazorCodeDocument ExecuteCore(RazorCodeDocument codeDocument, using var usedHelpers = new TagHelperCollection.Builder(); var sourceDocument = codeDocument.Source; - var context = new ResolutionContext(sourceDocument, documentNode); + var context = new ResolutionContext(sourceDocument, documentNode, resolver); ResolveElements(documentNode, binder, prefix, usedHelpers, in context); // Add tag helper descriptor validation diagnostics (e.g. RZ3003). @@ -95,11 +93,16 @@ private readonly struct ResolutionContext { public readonly RazorSourceDocument SourceDocument; public readonly DocumentIntermediateNode DocumentNode; + public readonly TagHelperResolver Resolver; - public ResolutionContext(RazorSourceDocument sourceDocument, DocumentIntermediateNode documentNode) + public ResolutionContext( + RazorSourceDocument sourceDocument, + DocumentIntermediateNode documentNode, + TagHelperResolver resolver) { SourceDocument = sourceDocument; DocumentNode = documentNode; + Resolver = resolver; } } @@ -160,7 +163,7 @@ private TagHelperIntermediateNode ResolveElement( { TryAddMalformedEndTagDiagnostic(elementNode, tagName, binder, attributes, parent, tagHelperParent, prefix); - _resolver.ConvertToPlainElement(parent, index, elementNode); + context.Resolver.ConvertToPlainElement(parent, index, elementNode); return null; } @@ -264,7 +267,7 @@ private TagHelperIntermediateNode ResolveElement( // Add resolver-specific diagnostics (e.g. RZ10012 for component-like elements, // case mismatch between start/end tags). - _resolver.AddMatchedElementDiagnostics(tagHelperNode, elementNode, binding, in context); + context.Resolver.AddMatchedElementDiagnostics(tagHelperNode, elementNode, binding, in context); // Check if resolved tag name is a void element (handles prefixed elements like th:input). var isResolvedVoidElement = elementNode.IsVoidElement || Legacy.ParserHelpers.VoidElements.Contains(resolvedTagName); @@ -281,7 +284,7 @@ private TagHelperIntermediateNode ResolveElement( // Build body and attributes. var bodyNode = new TagHelperBodyIntermediateNode(); - _resolver.BuildTagHelper(tagHelperNode, bodyNode, elementNode, binding, context.SourceDocument, in context); + context.Resolver.BuildTagHelper(tagHelperNode, bodyNode, elementNode, binding, context.SourceDocument, in context); return (tagHelperNode, bodyNode); } @@ -445,12 +448,12 @@ private void ConvertToPlainElementAndResolve( bool emitDiagnostics = true) { var childCountBefore = parent.Children.Count; - _resolver.ConvertToPlainElement(parent, index, elementNode); + context.Resolver.ConvertToPlainElement(parent, index, elementNode); var resultCount = parent.Children.Count - childCountBefore + 1; // +1 because the original was removed if (emitDiagnostics && resultCount > 0) { - _resolver.AddUnmatchedElementDiagnostic(parent.Children[index], elementNode, context.DocumentNode); + context.Resolver.AddUnmatchedElementDiagnostic(parent.Children[index], elementNode, context.DocumentNode); } for (var j = index + resultCount - 1; j >= index; j--) @@ -785,7 +788,10 @@ private static void FlattenToDirectCSharpTokens(IntermediateNode source, Interme /// resolved by tag helper matching. Converts each to a plain element using the resolver. /// Recursively processes the tree to handle nested elements. /// - private void UnwrapAllElements(IntermediateNode node, DocumentIntermediateNode documentNode = null) + private static void UnwrapAllElements( + IntermediateNode node, + TagHelperResolver resolver, + DocumentIntermediateNode documentNode = null) { if (node is DocumentIntermediateNode doc) { @@ -795,17 +801,17 @@ private void UnwrapAllElements(IntermediateNode node, DocumentIntermediateNode d for (var i = node.Children.Count - 1; i >= 0; i--) { var child = node.Children[i]; - UnwrapAllElements(child, documentNode); + UnwrapAllElements(child, resolver, documentNode); if (child is UnresolvedElementIntermediateNode elementNode) { var countBefore = node.Children.Count; - _resolver.ConvertToPlainElement(node, i, elementNode); + resolver.ConvertToPlainElement(node, i, elementNode); var resultCount = node.Children.Count - countBefore + 1; if (resultCount > 0) { - _resolver.AddUnmatchedElementDiagnostic(node.Children[i], elementNode, documentNode); + resolver.AddUnmatchedElementDiagnostic(node.Children[i], elementNode, documentNode); } } } From 12e43985ce9e3ab3b426c06d24db257bbcda396c Mon Sep 17 00:00:00 2001 From: Chris Sienkiewicz Date: Fri, 4 Sep 2026 12:13:16 -0700 Subject: [PATCH 2/2] Bound Razor resolver race test concurrency Match classic rzc's four-way parallelism and run 2,000 mixed document iterations. This preserves a wide overlap window for the timing-sensitive race while keeping the test cheaper on constrained agents. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: d07d252c-d5c2-43ae-ae7c-e56c4624e953 --- .../test/DefaultTagHelperResolutionPhaseTest.cs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/Razor/src/Compiler/Microsoft.AspNetCore.Razor.Language/test/DefaultTagHelperResolutionPhaseTest.cs b/src/Razor/src/Compiler/Microsoft.AspNetCore.Razor.Language/test/DefaultTagHelperResolutionPhaseTest.cs index 72f16bdbdf25b..6783ec3da1272 100644 --- a/src/Razor/src/Compiler/Microsoft.AspNetCore.Razor.Language/test/DefaultTagHelperResolutionPhaseTest.cs +++ b/src/Razor/src/Compiler/Microsoft.AspNetCore.Razor.Language/test/DefaultTagHelperResolutionPhaseTest.cs @@ -29,8 +29,9 @@ public void Process_ConcurrentMixedFileKinds_UsesCorrectResolver() (Source: RazorSourceDocument.Create("

Component

", "/Components/App.razor"), FileKind: RazorFileKind.Component), }; - // Act and assert - Parallel.For(0, 10_000, iteration => + // Act and assert: match classic rzc's concurrency while repeating enough to exercise the timing-sensitive overlap. + var parallelOptions = new ParallelOptions { MaxDegreeOfParallelism = 4 }; + Parallel.For(0, 2_000, parallelOptions, iteration => { var (source, fileKind) = inputs[iteration % inputs.Length]; var codeDocument = projectEngine.Process(