From 8c08a397a053d05c3ceb7955c513eedb32a368fd Mon Sep 17 00:00:00 2001 From: Tomas Matousek Date: Thu, 14 Nov 2019 19:00:51 -0800 Subject: [PATCH 1/5] EnC: Fix document out-of-sync checks when module is not loaded when debugging session starts Previous change https://github.com/dotnet/roslyn/pull/39295 allowed us to rely on PDB source file checksums when distinguishing between design-time-only documents that should be ignored when applying changes and documents whose changes must be applied to the debuggee. The change did not handle the case when the PDB isn't loaded yet when this check is being performed. This bug resulted in changes not being applied correctly and documents getting out-of-sync in scenarios where the modules were not loaded to the debuggee fast enough. Fixes https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1013242 --- .../EditAndContinueWorkspaceServiceTests.cs | 78 +++++++++++- .../EditAndContinue/CommittedSolution.cs | 115 ++++++++++++------ .../Portable/EditAndContinue/EditSession.cs | 3 + .../IDebuggeeModuleMetadataProvider.cs | 1 + 4 files changed, 156 insertions(+), 41 deletions(-) diff --git a/src/EditorFeatures/Test/EditAndContinue/EditAndContinueWorkspaceServiceTests.cs b/src/EditorFeatures/Test/EditAndContinue/EditAndContinueWorkspaceServiceTests.cs index c275764462302..3ff10770913e8 100644 --- a/src/EditorFeatures/Test/EditAndContinue/EditAndContinueWorkspaceServiceTests.cs +++ b/src/EditorFeatures/Test/EditAndContinue/EditAndContinueWorkspaceServiceTests.cs @@ -138,6 +138,16 @@ TService IDocumentServiceProvider.GetService() } private (DebuggeeModuleInfo, Guid) EmitAndLoadLibraryToDebuggee(string source, ProjectId projectId, string assemblyName = "", string sourceFilePath = "test1.cs") + { + var (debuggeeModuleInfo, moduleId) = EmitLibrary(source, projectId, assemblyName, sourceFilePath); + LoadLibraryToDebuggee(debuggeeModuleInfo); + return (debuggeeModuleInfo, moduleId); + } + + private void LoadLibraryToDebuggee(DebuggeeModuleInfo debuggeeModuleInfo) + => _mockDebugeeModuleMetadataProvider.TryGetBaselineModuleInfo = mvid => debuggeeModuleInfo; + + private (DebuggeeModuleInfo, Guid) EmitLibrary(string source, ProjectId projectId, string assemblyName = "", string sourceFilePath = "test1.cs") { var sourceText = SourceText.From(new MemoryStream(Encoding.UTF8.GetBytes(source)), encoding: Encoding.UTF8, checksumAlgorithm: SourceHashAlgorithm.Sha256); var tree = SyntaxFactory.ParseSyntaxTree(sourceText, TestOptions.RegularPreview, sourceFilePath); @@ -148,9 +158,6 @@ TService IDocumentServiceProvider.GetService() var moduleId = moduleMetadata.GetModuleVersionId(); var debuggeeModuleInfo = new DebuggeeModuleInfo(moduleMetadata, symReader); - // "load" it to the debuggee: - _mockDebugeeModuleMetadataProvider.TryGetBaselineModuleInfo = mvid => debuggeeModuleInfo; - // associate the binaries with the project _mockCompilationOutputsService.Outputs.Add(projectId, new MockCompilationOutputs(moduleId)); @@ -163,7 +170,6 @@ private SourceText CreateSourceTextFromFile(string path) return SourceText.From(stream, Encoding.UTF8, SourceHashAlgorithm.Sha256); } - [Fact] public void ActiveStatementTracking() { @@ -1021,6 +1027,70 @@ public async Task BreakMode_RudeEdits_DocumentWithoutSequencePoints() service.EndDebuggingSession(); } + [Fact] + public async Task BreakMode_RudeEdits_DelayLoadedModule() + { + var source1 = "class C { public void M() { } }"; + var dir = Temp.CreateDirectory(); + var sourceFile = dir.CreateFile("a.cs").WriteAllText(source1); + + using var workspace = new TestWorkspace(); + + // the workspace starts with a version of the source that's not updated with the output of single file generator (or design-time build): + var document1 = workspace.CurrentSolution. + AddProject("test", "test", LanguageNames.CSharp). + AddMetadataReferences(TargetFrameworkUtil.GetReferences(TargetFramework.Mscorlib40)). + AddDocument("test.cs", SourceText.From(source1, Encoding.UTF8), filePath: sourceFile.Path); + + var project = document1.Project; + workspace.ChangeSolution(project.Solution); + + var (debuggeeModuleInfo, _) = EmitLibrary(source1, project.Id, sourceFilePath: sourceFile.Path); + + var service = CreateEditAndContinueService(workspace); + + // do not initialize the document state - we will detect the state based on the PDB content. + var debuggingSession = StartDebuggingSession(service, initialState: CommittedSolution.DocumentState.None); + + service.StartEditSession(); + + // change the source (rude edit) before the library is loaded: + workspace.ChangeDocument(document1.Id, SourceText.From("class C { public void Renamed() { } }")); + var document2 = workspace.CurrentSolution.Projects.Single().Documents.Single(); + + // Rude Edits reported: + var diagnostics = await service.GetDocumentDiagnosticsAsync(document2, CancellationToken.None).ConfigureAwait(false); + AssertEx.Equal( + new[] { "ENC0020: " + string.Format(FeaturesResources.Renaming_0_will_prevent_the_debug_session_from_continuing, FeaturesResources.method) }, + diagnostics.Select(d => $"{d.Id}: {d.GetMessage()}")); + + var solutionStatus = await service.GetSolutionUpdateStatusAsync(sourceFilePath: null, CancellationToken.None).ConfigureAwait(false); + Assert.Equal(SolutionUpdateStatus.Blocked, solutionStatus); + + var (solutionStatusEmit, deltas) = await service.EmitSolutionUpdateAsync(CancellationToken.None).ConfigureAwait(false); + Assert.Equal(SolutionUpdateStatus.Blocked, solutionStatusEmit); + Assert.Empty(deltas); + + // load library to the debuggee: + LoadLibraryToDebuggee(debuggeeModuleInfo); + + // Rude Edits still reported: + diagnostics = await service.GetDocumentDiagnosticsAsync(document2, CancellationToken.None).ConfigureAwait(false); + AssertEx.Equal( + new[] { "ENC0020: " + string.Format(FeaturesResources.Renaming_0_will_prevent_the_debug_session_from_continuing, FeaturesResources.method) }, + diagnostics.Select(d => $"{d.Id}: {d.GetMessage()}")); + + solutionStatus = await service.GetSolutionUpdateStatusAsync(sourceFilePath: null, CancellationToken.None).ConfigureAwait(false); + Assert.Equal(SolutionUpdateStatus.Blocked, solutionStatus); + + (solutionStatusEmit, deltas) = await service.EmitSolutionUpdateAsync(CancellationToken.None).ConfigureAwait(false); + Assert.Equal(SolutionUpdateStatus.Blocked, solutionStatusEmit); + Assert.Empty(deltas); + + service.EndEditSession(); + service.EndDebuggingSession(); + } + [Fact] public async Task BreakMode_SyntaxError() { diff --git a/src/Features/Core/Portable/EditAndContinue/CommittedSolution.cs b/src/Features/Core/Portable/EditAndContinue/CommittedSolution.cs index fe6c9bf8be88f..cf3c27ab87db9 100644 --- a/src/Features/Core/Portable/EditAndContinue/CommittedSolution.cs +++ b/src/Features/Core/Portable/EditAndContinue/CommittedSolution.cs @@ -32,9 +32,32 @@ internal sealed class CommittedSolution internal enum DocumentState { None = 0, - OutOfSync = 1, - MatchesDebuggee = 2, - DesignTimeOnly = 3, + + /// + /// The document belongs to a project whose compiled module has not been loaded yet. + /// This document state may change to to , + /// or once the module has been loaded. + /// + ModuleNotLoaded = 1, + + /// + /// The current document content does not match the content the module was compiled with. + /// This document state may change to or . + /// + OutOfSync = 2, + + /// + /// The current document content matches the content the module was compiled with. + /// This is a final state. Once a document is in this state it won't switch to a different one. + /// + MatchesDebuggee = 3, + + /// + /// The document is not compiled into the module. It's only included in the project + /// to support design-time features such as completion, etc. + /// This is a final state. Once a document is in this state it won't switch to a different one. + /// + DesignTimeOnly = 4, } /// @@ -138,6 +161,10 @@ public Task OnSourceFileUpdatedAsync(DocumentId documentId, CancellationToken ca case DocumentState.DesignTimeOnly: return (null, documentState); + case DocumentState.ModuleNotLoaded: + // module might have been loaded since the last time we checked + break; + case DocumentState.OutOfSync: if (reloadOutOfSyncDocument) { @@ -152,11 +179,14 @@ public Task OnSourceFileUpdatedAsync(DocumentId documentId, CancellationToken ca } } - var (matchingSourceText, isMissing) = await TryGetPdbMatchingSourceTextAsync(document.FilePath, document.Project.Id, cancellationToken).ConfigureAwait(false); + var (matchingSourceText, isLoaded, isMissing) = await TryGetPdbMatchingSourceTextAsync(document.FilePath, document.Project.Id, cancellationToken).ConfigureAwait(false); lock (_guard) { - if (_documentState.TryGetValue(documentId, out var documentState) && documentState != DocumentState.OutOfSync) + // only OutOfSync and ModuleNotLoaded states can be changed: + if (_documentState.TryGetValue(documentId, out var documentState) && + documentState != DocumentState.OutOfSync && + documentState != DocumentState.ModuleNotLoaded) { return (document, documentState); } @@ -166,11 +196,20 @@ public Task OnSourceFileUpdatedAsync(DocumentId documentId, CancellationToken ca if (isMissing) { - // Source file is not listed in the PDB. This may happen for a couple of reasons: - // The library wasn't built with that source file - the file has been added before debugging session started but after build captured it. - // This is the case for WPF .g.i.cs files. - matchingDocument = null; - newState = DocumentState.DesignTimeOnly; + if (isLoaded) + { + // Source file is not listed in the PDB. This may happen for a couple of reasons: + // The library wasn't built with that source file - the file has been added before debugging session started but after build captured it. + // This is the case for WPF .g.i.cs files. + matchingDocument = null; + newState = DocumentState.DesignTimeOnly; + } + else + { + // The module the document is compiled into has not been loaded yet. + matchingDocument = document; + newState = DocumentState.ModuleNotLoaded; + } } else if (matchingSourceText != null) { @@ -216,34 +255,34 @@ public void CommitSolution(Solution solution, ImmutableArray updatedDo } } - private async Task<(SourceText? Source, bool IsMissing)> TryGetPdbMatchingSourceTextAsync(string sourceFilePath, ProjectId projectId, CancellationToken cancellationToken) + private async Task<(SourceText? Source, bool IsLoaded, bool IsMissing)> TryGetPdbMatchingSourceTextAsync(string sourceFilePath, ProjectId projectId, CancellationToken cancellationToken) { - var (symChecksum, algorithm) = await TryReadSourceFileChecksumFromPdb(sourceFilePath, projectId, cancellationToken).ConfigureAwait(false); + var (symChecksum, algorithm, isLoaded) = await TryReadSourceFileChecksumFromPdb(sourceFilePath, projectId, cancellationToken).ConfigureAwait(false); if (symChecksum.IsDefault) { - return (Source: null, IsMissing: true); + return (Source: null, isLoaded, IsMissing: true); } if (!PathUtilities.IsAbsolute(sourceFilePath)) { EditAndContinueWorkspaceService.Log.Write("Error calculating checksum for source file '{0}': path not absolute", sourceFilePath); - return (Source: null, IsMissing: false); + return (Source: null, isLoaded, IsMissing: false); } try { using var fileStream = new FileStream(sourceFilePath, FileMode.Open, FileAccess.Read, FileShare.Read | FileShare.Delete); var sourceText = SourceText.From(fileStream, checksumAlgorithm: algorithm); - return (sourceText.GetChecksum().SequenceEqual(symChecksum) ? sourceText : null, IsMissing: false); + return (sourceText.GetChecksum().SequenceEqual(symChecksum) ? sourceText : null, isLoaded, IsMissing: false); } catch (Exception e) { EditAndContinueWorkspaceService.Log.Write("Error calculating checksum for source file '{0}': '{1}'", sourceFilePath, e.Message); - return (Source: null, IsMissing: false); + return (Source: null, isLoaded, IsMissing: false); } } - private async Task<(ImmutableArray Checksum, SourceHashAlgorithm Algorithm)> TryReadSourceFileChecksumFromPdb(string sourceFilePath, ProjectId projectId, CancellationToken cancellationToken) + private async Task<(ImmutableArray Checksum, SourceHashAlgorithm Algorithm, bool IsLoaded)> TryReadSourceFileChecksumFromPdb(string sourceFilePath, ProjectId projectId, CancellationToken cancellationToken) { try { @@ -259,40 +298,42 @@ public void CommitSolution(Solution solution, ImmutableArray updatedDo // Dispatch to a background thread - reading symbols requires MTA thread. if (Thread.CurrentThread.GetApartmentState() != ApartmentState.MTA) { - return await Task.Factory.StartNew(() => - { - try - { - return ReadChecksum(); - } - catch (Exception e) when (FatalError.ReportWithoutCrashUnlessCanceled(e)) - { - EditAndContinueWorkspaceService.Log.Write("Source '{0}' doesn't match PDB: unexpected exception: {1}", sourceFilePath, e.Message); - return default; - } - }, cancellationToken, TaskCreationOptions.None, TaskScheduler.Default).ConfigureAwait(false); + return await Task.Factory.StartNew(ReadChecksum, cancellationToken, TaskCreationOptions.None, TaskScheduler.Default).ConfigureAwait(false); } else { return ReadChecksum(); } - (ImmutableArray Checksum, SourceHashAlgorithm Algorithm) ReadChecksum() + (ImmutableArray Checksum, SourceHashAlgorithm Algorithm, bool IsLoaded) ReadChecksum() { - var moduleInfo = _debuggingSession.DebugeeModuleMetadataProvider.TryGetBaselineModuleInfo(mvid); + DebuggeeModuleInfo? moduleInfo; + bool isLoaded = false; + try + { + moduleInfo = _debuggingSession.DebugeeModuleMetadataProvider.TryGetBaselineModuleInfo(mvid); + } + catch (Exception e) when (FatalError.ReportWithoutCrashUnlessCanceled(e)) + { + EditAndContinueWorkspaceService.Log.Write("Source '{0}' doesn't match PDB: unexpected exception: {1}", sourceFilePath, e.Message); + return (default, default, isLoaded); + } + if (moduleInfo == null) { - EditAndContinueWorkspaceService.Log.Write("Source '{0}' doesn't match PDB: can't get baseline SymReader", sourceFilePath); - return default; + EditAndContinueWorkspaceService.Log.Write("Source '{0}' doesn't match PDB: module not loaded", sourceFilePath); + return (default, default, isLoaded); } + isLoaded = true; + try { var symDocument = moduleInfo.SymReader.GetDocument(sourceFilePath); if (symDocument == null) { EditAndContinueWorkspaceService.Log.Write("Source '{0}' doesn't match PDB: no SymDocument", sourceFilePath); - return default; + return (default, default, isLoaded); } var symAlgorithm = SourceHashAlgorithms.GetSourceHashAlgorithm(symDocument.GetHashAlgorithm()); @@ -300,17 +341,17 @@ public void CommitSolution(Solution solution, ImmutableArray updatedDo { // unknown algorithm: EditAndContinueWorkspaceService.Log.Write("Source '{0}' doesn't match PDB: unknown checksum alg", sourceFilePath); - return default; + return (default, default, isLoaded); } var symChecksum = symDocument.GetChecksum().ToImmutableArray(); - return (symChecksum, symAlgorithm); + return (symChecksum, symAlgorithm, isLoaded); } catch (Exception e) { EditAndContinueWorkspaceService.Log.Write("Source '{0}' doesn't match PDB: error reading symbols: {1}", sourceFilePath, e.Message); - return default; + return (default, default, isLoaded); } } } diff --git a/src/Features/Core/Portable/EditAndContinue/EditSession.cs b/src/Features/Core/Portable/EditAndContinue/EditSession.cs index 053b36c901e48..2e0f95364f173 100644 --- a/src/Features/Core/Portable/EditAndContinue/EditSession.cs +++ b/src/Features/Core/Portable/EditAndContinue/EditSession.cs @@ -350,6 +350,9 @@ internal async Task> GetBaseActi continue; default: + // Include the document regardless of whether the module it was built into has been loaded or not. + // If the module has been built it might get loaded later during the debugging session, + // at which point we apply all changes that have been made to the project so far. changedDocuments.Add((oldDocument, document)); break; } diff --git a/src/Features/Core/Portable/EditAndContinue/IDebuggeeModuleMetadataProvider.cs b/src/Features/Core/Portable/EditAndContinue/IDebuggeeModuleMetadataProvider.cs index ba11ff1b69a78..b9fb70eb50cb3 100644 --- a/src/Features/Core/Portable/EditAndContinue/IDebuggeeModuleMetadataProvider.cs +++ b/src/Features/Core/Portable/EditAndContinue/IDebuggeeModuleMetadataProvider.cs @@ -17,6 +17,7 @@ internal interface IDebuggeeModuleMetadataProvider /// Shall only be called while in debug mode. /// Shall only be called on MTA thread. /// + /// Null, if the module with the specified MVID is not loaded. DebuggeeModuleInfo? TryGetBaselineModuleInfo(Guid mvid); /// From ec210604b0692be15726d90febd2154d3ac04a9e Mon Sep 17 00:00:00 2001 From: Tomas Matousek Date: Fri, 15 Nov 2019 16:05:00 -0800 Subject: [PATCH 2/5] Fall back to reading output PDB for docuemnt checksum validation if module is not loaded --- ...itAndContinueMethodDebugInfoReaderTests.cs | 12 +- .../EditAndContinueWorkspaceServiceTests.cs | 80 ++++++-- .../Helpers/SymReaderTestHelpers.cs | 25 +-- .../DebugInformationReaderProvider.cs | 1 + .../EditAndContinue/CommittedSolution.cs | 184 +++++++++++------- .../EditAndContinueMethodDebugInfoReader.cs | 47 +++++ .../FX/ImmutableArrayTestExtensions.cs | 30 +-- 7 files changed, 263 insertions(+), 116 deletions(-) diff --git a/src/EditorFeatures/Test/EditAndContinue/EditAndContinueMethodDebugInfoReaderTests.cs b/src/EditorFeatures/Test/EditAndContinue/EditAndContinueMethodDebugInfoReaderTests.cs index 9bca46e321374..d2a96bf9a1d82 100644 --- a/src/EditorFeatures/Test/EditAndContinue/EditAndContinueMethodDebugInfoReaderTests.cs +++ b/src/EditorFeatures/Test/EditAndContinue/EditAndContinueMethodDebugInfoReaderTests.cs @@ -1,6 +1,7 @@ // Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; using System.IO; +using System.Linq; using System.Reflection.Metadata; using System.Reflection.Metadata.Ecma335; using Microsoft.CodeAnalysis.CSharp.Test.Utilities; @@ -54,7 +55,7 @@ public static void Main() } } "; - var compilation = CSharpTestBase.CreateCompilationWithMscorlib40AndSystemCore(source, options: TestOptions.DebugDll); + var compilation = CSharpTestBase.CreateCompilationWithMscorlib40AndSystemCore(source, options: TestOptions.DebugDll, sourceFileName: "/a/c.cs"); var pdbStream = new MemoryStream(); compilation.EmitToArray(new EmitOptions(debugInformationFormat: format), pdbStream: pdbStream); @@ -95,6 +96,15 @@ public static void Main() localSig = reader.GetLocalSignature(MetadataTokens.MethodDefinitionHandle(1)); Assert.Equal(default, localSig); + + // document checksums: + Assert.False(reader.TryGetDocumentChecksum("/b/c.cs", out _, out _)); + Assert.False(reader.TryGetDocumentChecksum("/a/d.cs", out _, out _)); + Assert.False(reader.TryGetDocumentChecksum("/A/C.cs", out _, out _)); + + Assert.True(reader.TryGetDocumentChecksum("/a/c.cs", out var actualChecksum, out var actualAlgorithm)); + Assert.Equal("21-C8-B2-D7-A3-6B-49-C7-57-DF-67-B8-1F-75-DF-6A-64-FD-59-22", BitConverter.ToString(actualChecksum.ToArray())); + Assert.Equal(new Guid("ff1816ec-aa5e-4d10-87f7-6f4963833460"), actualAlgorithm); } } } diff --git a/src/EditorFeatures/Test/EditAndContinue/EditAndContinueWorkspaceServiceTests.cs b/src/EditorFeatures/Test/EditAndContinue/EditAndContinueWorkspaceServiceTests.cs index 3ff10770913e8..80ccbf0aa1321 100644 --- a/src/EditorFeatures/Test/EditAndContinue/EditAndContinueWorkspaceServiceTests.cs +++ b/src/EditorFeatures/Test/EditAndContinue/EditAndContinueWorkspaceServiceTests.cs @@ -147,19 +147,33 @@ TService IDocumentServiceProvider.GetService() private void LoadLibraryToDebuggee(DebuggeeModuleInfo debuggeeModuleInfo) => _mockDebugeeModuleMetadataProvider.TryGetBaselineModuleInfo = mvid => debuggeeModuleInfo; - private (DebuggeeModuleInfo, Guid) EmitLibrary(string source, ProjectId projectId, string assemblyName = "", string sourceFilePath = "test1.cs") + private (DebuggeeModuleInfo, Guid) EmitLibrary(string source, ProjectId projectId, string assemblyName = "", string sourceFilePath = "test1.cs", DebugInformationFormat pdbFormat = DebugInformationFormat.PortablePdb) { var sourceText = SourceText.From(new MemoryStream(Encoding.UTF8.GetBytes(source)), encoding: Encoding.UTF8, checksumAlgorithm: SourceHashAlgorithm.Sha256); var tree = SyntaxFactory.ParseSyntaxTree(sourceText, TestOptions.RegularPreview, sourceFilePath); var compilation = CSharpTestBase.CreateCompilationWithMscorlib40(tree, options: TestOptions.DebugDll, assemblyName: assemblyName); - var (peImage, symReader) = SymReaderTestHelpers.EmitAndOpenDummySymReader(compilation, DebugInformationFormat.PortablePdb); + + var (peImage, pdbImage) = compilation.EmitToArrays(new EmitOptions(debugInformationFormat: pdbFormat)); + var symReader = SymReaderTestHelpers.OpenDummySymReader(pdbImage); var moduleMetadata = ModuleMetadata.CreateFromImage(peImage); var moduleId = moduleMetadata.GetModuleVersionId(); var debuggeeModuleInfo = new DebuggeeModuleInfo(moduleMetadata, symReader); // associate the binaries with the project - _mockCompilationOutputsService.Outputs.Add(projectId, new MockCompilationOutputs(moduleId)); + _mockCompilationOutputsService.Outputs.Add(projectId, new MockCompilationOutputs(moduleId) + { + OpenPdbStreamImpl = () => + { + var pdbStream = new MemoryStream(); + pdbImage.WriteToStream(pdbStream); + pdbStream.Position = 0; + return pdbStream; + } + }); + + // library not loaded yet: + _mockDebugeeModuleMetadataProvider.TryGetBaselineModuleInfo = mvid => null; return (debuggeeModuleInfo, moduleId); } @@ -583,15 +597,17 @@ public async Task BreakMode_DesignTimeOnlyDocument_Dynamic() Assert.Empty(deltas); } - [Fact] - public async Task BreakMode_DesignTimeOnlyDocument_Wpf() + [Theory] + [InlineData(true)] + [InlineData(false)] + public async Task BreakMode_DesignTimeOnlyDocument_Wpf(bool delayLoad) { var sourceA = "class A { public void M() { } }"; var sourceB = "class B { public void M() { } }"; var sourceC = "class C { public void M() { } }"; var dir = Temp.CreateDirectory(); - var sourceFile = dir.CreateFile("a.cs").WriteAllText(sourceA); + var sourceFileA = dir.CreateFile("a.cs").WriteAllText(sourceA); using var workspace = new TestWorkspace(); @@ -599,7 +615,7 @@ public async Task BreakMode_DesignTimeOnlyDocument_Wpf() var documentA = workspace.CurrentSolution. AddProject("test", "test", LanguageNames.CSharp). AddMetadataReferences(TargetFrameworkUtil.GetReferences(TargetFramework.Mscorlib40)). - AddDocument("a.cs", SourceText.From(sourceA, Encoding.UTF8), filePath: sourceFile.Path); + AddDocument("a.cs", SourceText.From(sourceA, Encoding.UTF8), filePath: sourceFileA.Path); var documentB = documentA.Project. AddDocument("b.g.i.cs", SourceText.From(sourceB, Encoding.UTF8), filePath: "b.g.i.cs"); @@ -610,7 +626,12 @@ public async Task BreakMode_DesignTimeOnlyDocument_Wpf() workspace.ChangeSolution(documentC.Project.Solution); // only compile A; B and C are design-time-only: - var (_, moduleId) = EmitAndLoadLibraryToDebuggee(sourceA, documentA.Project.Id, sourceFilePath: sourceFile.Path); + var (moduleInfo, moduleId) = EmitLibrary(sourceA, documentA.Project.Id, sourceFilePath: sourceFileA.Path); + + if (!delayLoad) + { + LoadLibraryToDebuggee(moduleInfo); + } var service = CreateEditAndContinueService(workspace); @@ -636,6 +657,19 @@ public async Task BreakMode_DesignTimeOnlyDocument_Wpf() Assert.Equal(SolutionUpdateStatus.None, solutionStatusEmit); Assert.Empty(_emitDiagnosticsUpdated); + if (delayLoad) + { + LoadLibraryToDebuggee(moduleInfo); + + // validate solution update status and emit: + solutionStatus = await service.GetSolutionUpdateStatusAsync(sourceFilePath: null, CancellationToken.None).ConfigureAwait(false); + Assert.Equal(SolutionUpdateStatus.None, solutionStatus); + + (solutionStatusEmit, deltas) = await service.EmitSolutionUpdateAsync(CancellationToken.None).ConfigureAwait(false); + Assert.Equal(SolutionUpdateStatus.None, solutionStatusEmit); + Assert.Empty(_emitDiagnosticsUpdated); + } + service.EndEditSession(); service.EndDebuggingSession(); } @@ -1483,8 +1517,10 @@ public async Task BreakMode_ValidSignificantChange_FileUpdateBeforeDebuggingSess service.EndDebuggingSession(); } - [Fact] - public async Task BreakMode_ValidSignificantChange_DocumentOutOfSync2() + [Theory] + [InlineData(true)] + [InlineData(false)] + public async Task BreakMode_ValidSignificantChange_DocumentOutOfSync(bool delayLoad) { var sourceOnDisk = "class C1 { void M() { System.Console.WriteLine(1); } }"; @@ -1502,11 +1538,16 @@ public async Task BreakMode_ValidSignificantChange_DocumentOutOfSync2() var project = document1.Project; workspace.ChangeSolution(project.Solution); - var (_, moduleId) = EmitAndLoadLibraryToDebuggee(sourceOnDisk, project.Id, sourceFilePath: sourceFile.Path); + var (moduleInfo, moduleId) = EmitLibrary(sourceOnDisk, project.Id, sourceFilePath: sourceFile.Path); + + if (!delayLoad) + { + LoadLibraryToDebuggee(moduleInfo); + } var service = CreateEditAndContinueService(workspace); - var debuggingSession = StartDebuggingSession(service, initialState: CommittedSolution.DocumentState.None); + StartDebuggingSession(service, initialState: CommittedSolution.DocumentState.None); service.StartEditSession(); VerifyReanalyzeInvocation(workspace, null, ImmutableArray.Empty, false); @@ -1528,15 +1569,15 @@ public async Task BreakMode_ValidSignificantChange_DocumentOutOfSync2() workspace.ChangeDocument(document1.Id, SourceText.From(sourceOnDisk, Encoding.UTF8)); var document3 = workspace.CurrentSolution.Projects.Single().Documents.Single(); - var diagnostics2 = await service.GetDocumentDiagnosticsAsync(document3, CancellationToken.None).ConfigureAwait(false); - Assert.Empty(diagnostics2); + var diagnostics = await service.GetDocumentDiagnosticsAsync(document3, CancellationToken.None).ConfigureAwait(false); + Assert.Empty(diagnostics); // the content of the file is now exactly the same as the compiled document, so there is no change to be applied: - var solutionStatus2 = await service.GetSolutionUpdateStatusAsync(sourceFilePath: null, CancellationToken.None).ConfigureAwait(false); - Assert.Equal(SolutionUpdateStatus.None, solutionStatus2); + solutionStatus = await service.GetSolutionUpdateStatusAsync(sourceFilePath: null, CancellationToken.None).ConfigureAwait(false); + Assert.Equal(SolutionUpdateStatus.None, solutionStatus); - var (solutionStatusEmit2, deltas2) = await service.EmitSolutionUpdateAsync(CancellationToken.None).ConfigureAwait(false); - Assert.Equal(SolutionUpdateStatus.None, solutionStatusEmit2); + (solutionStatusEmit, _) = await service.EmitSolutionUpdateAsync(CancellationToken.None).ConfigureAwait(false); + Assert.Equal(SolutionUpdateStatus.None, solutionStatusEmit); service.EndEditSession(); @@ -1815,7 +1856,8 @@ public async Task TwoUpdatesWithLoadedAndUnloadedModule() var compilationA = CSharpTestBase.CreateCompilationWithMscorlib40(source1, options: TestOptions.DebugDll, assemblyName: "A"); var compilationB = CSharpTestBase.CreateCompilationWithMscorlib45(source1, options: TestOptions.DebugDll, assemblyName: "B"); - var (peImageA, symReaderA) = SymReaderTestHelpers.EmitAndOpenDummySymReader(compilationA, DebugInformationFormat.PortablePdb); + var (peImageA, pdbImageA) = compilationA.EmitToArrays(new EmitOptions(debugInformationFormat: DebugInformationFormat.PortablePdb)); + var symReaderA = SymReaderTestHelpers.OpenDummySymReader(pdbImageA); var moduleMetadataA = ModuleMetadata.CreateFromImage(peImageA); var moduleFileA = Temp.CreateFile("A.dll").WriteAllBytes(peImageA); diff --git a/src/EditorFeatures/Test/EditAndContinue/Helpers/SymReaderTestHelpers.cs b/src/EditorFeatures/Test/EditAndContinue/Helpers/SymReaderTestHelpers.cs index 7b641ac9b5c75..76619314c2564 100644 --- a/src/EditorFeatures/Test/EditAndContinue/Helpers/SymReaderTestHelpers.cs +++ b/src/EditorFeatures/Test/EditAndContinue/Helpers/SymReaderTestHelpers.cs @@ -2,6 +2,7 @@ using System; using System.Collections.Immutable; using System.IO; +using System.Linq; using System.Reflection; using Microsoft.CodeAnalysis.Emit; using Microsoft.CodeAnalysis.Test.Utilities; @@ -30,32 +31,32 @@ public bool TryGetTypeReferenceInfo(int typeReferenceToken, out string namespace => throw new NotImplementedException(); } - public static (ImmutableArray PEImage, ISymUnmanagedReader5 SymReader) EmitAndOpenDummySymReader(Compilation compilation, DebugInformationFormat pdbFormat) + public static (ImmutableArray PEImage, ImmutableArray PdbImage) EmitToArrays(this Compilation compilation, EmitOptions options) + { + var pdbStream = new MemoryStream(); + var peImage = compilation.EmitToArray(options, pdbStream: pdbStream); + return (peImage, pdbStream.ToImmutable()); + } + + public static ISymUnmanagedReader5 OpenDummySymReader(ImmutableArray pdbImage) { var symBinder = new SymBinder(); var metadataImportProvider = new DummyMetadataImportProvider(); var pdbStream = new MemoryStream(); - var peImage = compilation.EmitToArray(new EmitOptions(debugInformationFormat: pdbFormat), pdbStream: pdbStream); - pdbStream.Position = 0; + pdbImage.WriteToStream(pdbStream); var pdbStreamCom = SymUnmanagedStreamFactory.CreateStream(pdbStream); - - ISymUnmanagedReader5 symReader5; - if (pdbFormat == DebugInformationFormat.PortablePdb) + if (pdbImage.Length > 4 && pdbImage[0] == 'B' && pdbImage[1] == 'S' && pdbImage[2] == 'J' && pdbImage[3] == 'B') { int hr = symBinder.GetReaderFromPdbStream(metadataImportProvider, pdbStreamCom, out var symReader); Assert.Equal(0, hr); - symReader5 = (ISymUnmanagedReader5)symReader; + return (ISymUnmanagedReader5)symReader; } else { - symReader5 = SymUnmanagedReaderFactory.CreateReader(pdbStream, new DummySymReaderMetadataProvider()); + return SymUnmanagedReaderFactory.CreateReader(pdbStream, new DummySymReaderMetadataProvider()); } - - return (peImage, symReader5); } - - } } diff --git a/src/Features/Core/Portable/Debugging/DebugInformationReaderProvider.cs b/src/Features/Core/Portable/Debugging/DebugInformationReaderProvider.cs index 5accbd5234feb..eba9992f6ba1f 100644 --- a/src/Features/Core/Portable/Debugging/DebugInformationReaderProvider.cs +++ b/src/Features/Core/Portable/Debugging/DebugInformationReaderProvider.cs @@ -1,6 +1,7 @@ // Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; +using System.Collections.Immutable; using System.IO; using System.Reflection; using System.Reflection.Metadata; diff --git a/src/Features/Core/Portable/EditAndContinue/CommittedSolution.cs b/src/Features/Core/Portable/EditAndContinue/CommittedSolution.cs index cf3c27ab87db9..5265f56e53d19 100644 --- a/src/Features/Core/Portable/EditAndContinue/CommittedSolution.cs +++ b/src/Features/Core/Portable/EditAndContinue/CommittedSolution.cs @@ -34,11 +34,14 @@ internal enum DocumentState None = 0, /// - /// The document belongs to a project whose compiled module has not been loaded yet. - /// This document state may change to to , - /// or once the module has been loaded. + /// The current document content matches the content the built module was compiled with. + /// The document content is matched with the build output instead of the loaded module + /// since the module hasn't been loaded yet. + /// + /// This document state may change to to or + /// or once the module has been loaded. /// - ModuleNotLoaded = 1, + MatchesBuildOutput = 1, /// /// The current document content does not match the content the module was compiled with. @@ -47,7 +50,7 @@ internal enum DocumentState OutOfSync = 2, /// - /// The current document content matches the content the module was compiled with. + /// The current document content matches the content the loaded module was compiled with. /// This is a final state. Once a document is in this state it won't switch to a different one. /// MatchesDebuggee = 3, @@ -60,6 +63,13 @@ internal enum DocumentState DesignTimeOnly = 4, } + private enum SourceHashOrigin + { + None = 0, + LoadedPdb = 1, + BuiltPdb = 2 + } + /// /// Implements workaround for https://github.com/dotnet/project-system/issues/5457. /// @@ -161,8 +171,10 @@ public Task OnSourceFileUpdatedAsync(DocumentId documentId, CancellationToken ca case DocumentState.DesignTimeOnly: return (null, documentState); - case DocumentState.ModuleNotLoaded: - // module might have been loaded since the last time we checked + case DocumentState.MatchesBuildOutput: + // Module might have been loaded since the last time we checked, + // let's check whether that is so and the document now matches the debuggee. + // CONSIDER: Reusing the state until we receive module load event. break; case DocumentState.OutOfSync: @@ -179,14 +191,14 @@ public Task OnSourceFileUpdatedAsync(DocumentId documentId, CancellationToken ca } } - var (matchingSourceText, isLoaded, isMissing) = await TryGetPdbMatchingSourceTextAsync(document.FilePath, document.Project.Id, cancellationToken).ConfigureAwait(false); + var (matchingSourceText, checksumOrigin, isDocumentMissing) = await TryGetPdbMatchingSourceTextAsync(document.FilePath, document.Project.Id, cancellationToken).ConfigureAwait(false); lock (_guard) { - // only OutOfSync and ModuleNotLoaded states can be changed: + // only listed document states can be changed: if (_documentState.TryGetValue(documentId, out var documentState) && documentState != DocumentState.OutOfSync && - documentState != DocumentState.ModuleNotLoaded) + documentState != DocumentState.MatchesBuildOutput) { return (document, documentState); } @@ -194,22 +206,20 @@ public Task OnSourceFileUpdatedAsync(DocumentId documentId, CancellationToken ca DocumentState newState; Document? matchingDocument; - if (isMissing) + if (checksumOrigin == SourceHashOrigin.None) { - if (isLoaded) - { - // Source file is not listed in the PDB. This may happen for a couple of reasons: - // The library wasn't built with that source file - the file has been added before debugging session started but after build captured it. - // This is the case for WPF .g.i.cs files. - matchingDocument = null; - newState = DocumentState.DesignTimeOnly; - } - else - { - // The module the document is compiled into has not been loaded yet. - matchingDocument = document; - newState = DocumentState.ModuleNotLoaded; - } + // PDB for the module not found (neither loaded nor in built outputs): + Debug.Assert(isDocumentMissing); + return (null, DocumentState.DesignTimeOnly); + } + + if (isDocumentMissing) + { + // Source file is not listed in the PDB. This may happen for a couple of reasons: + // The library wasn't built with that source file - the file has been added before debugging session started but after build captured it. + // This is the case for WPF .g.i.cs files. + matchingDocument = null; + newState = DocumentState.DesignTimeOnly; } else if (matchingSourceText != null) { @@ -223,7 +233,7 @@ public Task OnSourceFileUpdatedAsync(DocumentId documentId, CancellationToken ca matchingDocument = _solution.GetDocument(documentId); } - newState = DocumentState.MatchesDebuggee; + newState = (checksumOrigin == SourceHashOrigin.LoadedPdb) ? DocumentState.MatchesDebuggee : DocumentState.MatchesBuildOutput; } else { @@ -255,34 +265,34 @@ public void CommitSolution(Solution solution, ImmutableArray updatedDo } } - private async Task<(SourceText? Source, bool IsLoaded, bool IsMissing)> TryGetPdbMatchingSourceTextAsync(string sourceFilePath, ProjectId projectId, CancellationToken cancellationToken) + private async Task<(SourceText? Source, SourceHashOrigin ChecksumOrigin, bool IsDocumentMissing)> TryGetPdbMatchingSourceTextAsync(string sourceFilePath, ProjectId projectId, CancellationToken cancellationToken) { - var (symChecksum, algorithm, isLoaded) = await TryReadSourceFileChecksumFromPdb(sourceFilePath, projectId, cancellationToken).ConfigureAwait(false); + var (symChecksum, algorithm, origin) = await TryReadSourceFileChecksumFromPdb(sourceFilePath, projectId, cancellationToken).ConfigureAwait(false); if (symChecksum.IsDefault) { - return (Source: null, isLoaded, IsMissing: true); + return (Source: null, origin, IsDocumentMissing: true); } if (!PathUtilities.IsAbsolute(sourceFilePath)) { EditAndContinueWorkspaceService.Log.Write("Error calculating checksum for source file '{0}': path not absolute", sourceFilePath); - return (Source: null, isLoaded, IsMissing: false); + return (Source: null, origin, IsDocumentMissing: false); } try { using var fileStream = new FileStream(sourceFilePath, FileMode.Open, FileAccess.Read, FileShare.Read | FileShare.Delete); var sourceText = SourceText.From(fileStream, checksumAlgorithm: algorithm); - return (sourceText.GetChecksum().SequenceEqual(symChecksum) ? sourceText : null, isLoaded, IsMissing: false); + return (sourceText.GetChecksum().SequenceEqual(symChecksum) ? sourceText : null, origin, IsDocumentMissing: false); } catch (Exception e) { EditAndContinueWorkspaceService.Log.Write("Error calculating checksum for source file '{0}': '{1}'", sourceFilePath, e.Message); - return (Source: null, isLoaded, IsMissing: false); + return (Source: null, origin, IsDocumentMissing: false); } } - private async Task<(ImmutableArray Checksum, SourceHashAlgorithm Algorithm, bool IsLoaded)> TryReadSourceFileChecksumFromPdb(string sourceFilePath, ProjectId projectId, CancellationToken cancellationToken) + private async Task<(ImmutableArray Checksum, SourceHashAlgorithm Algorithm, SourceHashOrigin Origin)> TryReadSourceFileChecksumFromPdb(string sourceFilePath, ProjectId projectId, CancellationToken cancellationToken) { try { @@ -293,65 +303,97 @@ public void CommitSolution(Solution solution, ImmutableArray updatedDo return default; } - cancellationToken.ThrowIfCancellationRequested(); + // Dispatch to a background thread - reading symbols from debuggee requires MTA thread. + var (checksum, algorithmId, origin) = (Thread.CurrentThread.GetApartmentState() != ApartmentState.MTA) ? + await Task.Factory.StartNew(ReadChecksum, cancellationToken, TaskCreationOptions.None, TaskScheduler.Default).ConfigureAwait(false) : + ReadChecksum(); - // Dispatch to a background thread - reading symbols requires MTA thread. - if (Thread.CurrentThread.GetApartmentState() != ApartmentState.MTA) + if (checksum.IsDefault) { - return await Task.Factory.StartNew(ReadChecksum, cancellationToken, TaskCreationOptions.None, TaskScheduler.Default).ConfigureAwait(false); + return (default, default, origin); } - else + + var algorithm = SourceHashAlgorithms.GetSourceHashAlgorithm(algorithmId); + if (algorithm == SourceHashAlgorithm.None) { - return ReadChecksum(); + // unknown algorithm: + EditAndContinueWorkspaceService.Log.Write("Source '{0}' doesn't match PDB: unknown checksum alg", sourceFilePath); + return (default, default, origin); } - (ImmutableArray Checksum, SourceHashAlgorithm Algorithm, bool IsLoaded) ReadChecksum() + return (checksum, algorithm, origin); + + (ImmutableArray Checksum, Guid AlgorithmId, SourceHashOrigin Origin) ReadChecksum() { - DebuggeeModuleInfo? moduleInfo; - bool isLoaded = false; try { - moduleInfo = _debuggingSession.DebugeeModuleMetadataProvider.TryGetBaselineModuleInfo(mvid); - } - catch (Exception e) when (FatalError.ReportWithoutCrashUnlessCanceled(e)) - { - EditAndContinueWorkspaceService.Log.Write("Source '{0}' doesn't match PDB: unexpected exception: {1}", sourceFilePath, e.Message); - return (default, default, isLoaded); - } + // first try to check against loaded module + cancellationToken.ThrowIfCancellationRequested(); - if (moduleInfo == null) - { - EditAndContinueWorkspaceService.Log.Write("Source '{0}' doesn't match PDB: module not loaded", sourceFilePath); - return (default, default, isLoaded); - } + var moduleInfo = _debuggingSession.DebugeeModuleMetadataProvider.TryGetBaselineModuleInfo(mvid); + if (moduleInfo != null) + { + try + { + if (EditAndContinueMethodDebugInfoReader.TryGetDocumentChecksum(moduleInfo.SymReader, sourceFilePath, out var checksum, out var algorithmId)) + { + return (checksum, algorithmId, SourceHashOrigin.LoadedPdb); + } + + EditAndContinueWorkspaceService.Log.Write("Source '{0}' doesn't match loaded PDB: no SymDocument", sourceFilePath); + } + catch (Exception e) + { + EditAndContinueWorkspaceService.Log.Write("Source '{0}' doesn't match loaded PDB: error reading symbols: {1}", sourceFilePath, e.Message); + } - isLoaded = true; + return (default, default, SourceHashOrigin.LoadedPdb); + } - try - { - var symDocument = moduleInfo.SymReader.GetDocument(sourceFilePath); - if (symDocument == null) + // if the module is not loaded check against build output: + cancellationToken.ThrowIfCancellationRequested(); + + var compilationOutputs = _debuggingSession.CompilationOutputsProvider.GetCompilationOutputs(projectId); + + DebugInformationReaderProvider? debugInfoReaderProvider; + try { - EditAndContinueWorkspaceService.Log.Write("Source '{0}' doesn't match PDB: no SymDocument", sourceFilePath); - return (default, default, isLoaded); + debugInfoReaderProvider = compilationOutputs.OpenPdb(); + } + catch (Exception e) + { + EditAndContinueWorkspaceService.Log.Write("Source '{0}' doesn't match output PDB: error opening PDB: {1}", sourceFilePath, e.Message); + debugInfoReaderProvider = null; } - var symAlgorithm = SourceHashAlgorithms.GetSourceHashAlgorithm(symDocument.GetHashAlgorithm()); - if (symAlgorithm == SourceHashAlgorithm.None) + if (debugInfoReaderProvider == null) { - // unknown algorithm: - EditAndContinueWorkspaceService.Log.Write("Source '{0}' doesn't match PDB: unknown checksum alg", sourceFilePath); - return (default, default, isLoaded); + EditAndContinueWorkspaceService.Log.Write("Source '{0}' doesn't match output PDB: PDB not found", sourceFilePath); + return (default, default, SourceHashOrigin.None); } - var symChecksum = symDocument.GetChecksum().ToImmutableArray(); + try + { + var debugInfoReader = debugInfoReaderProvider.CreateEditAndContinueMethodDebugInfoReader(); + if (debugInfoReader.TryGetDocumentChecksum(sourceFilePath, out var checksum, out var algorithmId)) + { + return (checksum, algorithmId, SourceHashOrigin.BuiltPdb); + } - return (symChecksum, symAlgorithm, isLoaded); + EditAndContinueWorkspaceService.Log.Write("Source '{0}' doesn't match output PDB: no SymDocument", sourceFilePath); + return (default, default, SourceHashOrigin.BuiltPdb); + } + catch (Exception e) + { + EditAndContinueWorkspaceService.Log.Write("Source '{0}' doesn't match output PDB: error reading symbols: {1}", sourceFilePath, e.Message); + } + + return (default, default, SourceHashOrigin.BuiltPdb); } - catch (Exception e) + catch (Exception e) when (FatalError.ReportWithoutCrashUnlessCanceled(e)) { - EditAndContinueWorkspaceService.Log.Write("Source '{0}' doesn't match PDB: error reading symbols: {1}", sourceFilePath, e.Message); - return (default, default, isLoaded); + EditAndContinueWorkspaceService.Log.Write("Source '{0}' doesn't match PDB: unexpected exception: {1}", sourceFilePath, e.Message); + return default; } } } diff --git a/src/Features/Core/Portable/EditAndContinue/EditAndContinueMethodDebugInfoReader.cs b/src/Features/Core/Portable/EditAndContinue/EditAndContinueMethodDebugInfoReader.cs index c36f21dad0c9a..0fad86dec333c 100644 --- a/src/Features/Core/Portable/EditAndContinue/EditAndContinueMethodDebugInfoReader.cs +++ b/src/Features/Core/Portable/EditAndContinue/EditAndContinueMethodDebugInfoReader.cs @@ -4,6 +4,7 @@ using System.Collections.Immutable; using System.Diagnostics; using System.IO; +using System.Linq; using System.Reflection.Metadata; using System.Reflection.Metadata.Ecma335; using System.Runtime.InteropServices; @@ -24,6 +25,13 @@ internal abstract class EditAndContinueMethodDebugInfoReader public abstract EditAndContinueMethodDebugInformation GetDebugInfo(MethodDefinitionHandle methodHandle); public abstract StandaloneSignatureHandle GetLocalSignature(MethodDefinitionHandle methodHandle); + /// + /// Reads document checksum. + /// + /// True if a document with given path is listed in the PDB. + /// Error reading debug information from the PDB. + public abstract bool TryGetDocumentChecksum(string documentPath, out ImmutableArray checksum, out Guid algorithmId); + private sealed class Native : EditAndContinueMethodDebugInfoReader { private readonly ISymUnmanagedReader5 _symReader; @@ -89,6 +97,9 @@ public override EditAndContinueMethodDebugInformation GetDebugInfo(MethodDefinit throw new InvalidDataException(e.Message, e); } } + + public override bool TryGetDocumentChecksum(string documentPath, out ImmutableArray checksum, out Guid algorithmId) + => TryGetDocumentChecksum(_symReader, documentPath, out checksum, out algorithmId); } private sealed class Portable : EditAndContinueMethodDebugInfoReader @@ -137,6 +148,24 @@ private static bool TryGetCustomDebugInformation(MetadataReader reader, EntityHa return foundAny; } + + public override bool TryGetDocumentChecksum(string documentPath, out ImmutableArray checksum, out Guid algorithmId) + { + foreach (var documentHandle in _pdbReader.Documents) + { + var document = _pdbReader.GetDocument(documentHandle); + if (_pdbReader.StringComparer.Equals(document.Name, documentPath)) + { + checksum = _pdbReader.GetBlobContent(document.Hash); + algorithmId = _pdbReader.GetGuid(document.HashAlgorithm); + return true; + } + } + + checksum = default; + algorithmId = default; + return false; + } } /// @@ -188,5 +217,23 @@ public unsafe static EditAndContinueMethodDebugInfoReader Create(ISymUnmanagedRe /// public unsafe static EditAndContinueMethodDebugInfoReader Create(MetadataReader pdbReader) => new Portable(pdbReader ?? throw new ArgumentNullException(nameof(pdbReader))); + + internal static bool TryGetDocumentChecksum(ISymUnmanagedReader5 symReader, string documentPath, out ImmutableArray checksum, out Guid algorithmId) + { + var symDocument = symReader.GetDocument(documentPath); + + // Make sure the full path matches. + // Native SymReader allows partial match on the document file name. + if (symDocument == null || !StringComparer.Ordinal.Equals(symDocument.GetName(), documentPath)) + { + checksum = default; + algorithmId = default; + return false; + } + + algorithmId = symDocument.GetHashAlgorithm(); + checksum = symDocument.GetChecksum().ToImmutableArray(); + return true; + } } } diff --git a/src/Test/Utilities/Portable/FX/ImmutableArrayTestExtensions.cs b/src/Test/Utilities/Portable/FX/ImmutableArrayTestExtensions.cs index 26467302920ee..b251bbee54518 100644 --- a/src/Test/Utilities/Portable/FX/ImmutableArrayTestExtensions.cs +++ b/src/Test/Utilities/Portable/FX/ImmutableArrayTestExtensions.cs @@ -13,6 +13,8 @@ namespace Microsoft.CodeAnalysis.Test.Utilities /// public static class ImmutableArrayTestExtensions { + private const int BufferSize = 4096; + /// /// Writes read-only array of bytes to the specified file. /// @@ -20,22 +22,24 @@ public static class ImmutableArrayTestExtensions /// File path. internal static void WriteToFile(this ImmutableArray bytes, string path) { - Debug.Assert(!bytes.IsDefault); + using var fileStream = new FileStream(path, FileMode.Create, FileAccess.Write, FileShare.Read, BufferSize); + WriteToStream(bytes, fileStream); + } + internal static void WriteToStream(this ImmutableArray bytes, Stream stream) + { const int bufferSize = 4096; - using (FileStream fileStream = new FileStream(path, FileMode.Create, FileAccess.Write, FileShare.Read, bufferSize)) - { - // PERF: Consider using an ObjectPool here - byte[] buffer = new byte[Math.Min(bufferSize, bytes.Length)]; - int offset = 0; - while (offset < bytes.Length) - { - int length = Math.Min(bufferSize, bytes.Length - offset); - bytes.CopyTo(offset, buffer, 0, length); - fileStream.Write(buffer, 0, length); - offset += length; - } + // PERF: Consider using an ObjectPool here + var buffer = new byte[Math.Min(bufferSize, bytes.Length)]; + + int offset = 0; + while (offset < bytes.Length) + { + int length = Math.Min(bufferSize, bytes.Length - offset); + bytes.CopyTo(offset, buffer, 0, length); + stream.Write(buffer, 0, length); + offset += length; } } } From 00d5a4ad3f7fe6411f714906383fffeebaf8ef42 Mon Sep 17 00:00:00 2001 From: Tomas Matousek Date: Fri, 15 Nov 2019 16:18:47 -0800 Subject: [PATCH 3/5] Do not re-read checksum from build output --- .../EditAndContinue/CommittedSolution.cs | 22 +++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/src/Features/Core/Portable/EditAndContinue/CommittedSolution.cs b/src/Features/Core/Portable/EditAndContinue/CommittedSolution.cs index 5265f56e53d19..20236db5765dd 100644 --- a/src/Features/Core/Portable/EditAndContinue/CommittedSolution.cs +++ b/src/Features/Core/Portable/EditAndContinue/CommittedSolution.cs @@ -147,6 +147,7 @@ public Task OnSourceFileUpdatedAsync(DocumentId documentId, CancellationToken ca public async Task<(Document? Document, DocumentState State)> GetDocumentAndStateAsync(DocumentId documentId, CancellationToken cancellationToken, bool reloadOutOfSyncDocument = false) { Document? document; + var matchLoadedModulesOnly = false; lock (_guard) { @@ -174,7 +175,9 @@ public Task OnSourceFileUpdatedAsync(DocumentId documentId, CancellationToken ca case DocumentState.MatchesBuildOutput: // Module might have been loaded since the last time we checked, // let's check whether that is so and the document now matches the debuggee. + // Do not try to read the information from on-disk module again. // CONSIDER: Reusing the state until we receive module load event. + matchLoadedModulesOnly = true; break; case DocumentState.OutOfSync: @@ -191,7 +194,7 @@ public Task OnSourceFileUpdatedAsync(DocumentId documentId, CancellationToken ca } } - var (matchingSourceText, checksumOrigin, isDocumentMissing) = await TryGetPdbMatchingSourceTextAsync(document.FilePath, document.Project.Id, cancellationToken).ConfigureAwait(false); + var (matchingSourceText, checksumOrigin, isDocumentMissing) = await TryGetPdbMatchingSourceTextAsync(document.FilePath, document.Project.Id, matchLoadedModulesOnly, cancellationToken).ConfigureAwait(false); lock (_guard) { @@ -208,6 +211,12 @@ public Task OnSourceFileUpdatedAsync(DocumentId documentId, CancellationToken ca if (checksumOrigin == SourceHashOrigin.None) { + // We know the document matches the build output and the module is still not loaded. + if (matchLoadedModulesOnly) + { + return (document, DocumentState.MatchesBuildOutput); + } + // PDB for the module not found (neither loaded nor in built outputs): Debug.Assert(isDocumentMissing); return (null, DocumentState.DesignTimeOnly); @@ -265,9 +274,9 @@ public void CommitSolution(Solution solution, ImmutableArray updatedDo } } - private async Task<(SourceText? Source, SourceHashOrigin ChecksumOrigin, bool IsDocumentMissing)> TryGetPdbMatchingSourceTextAsync(string sourceFilePath, ProjectId projectId, CancellationToken cancellationToken) + private async Task<(SourceText? Source, SourceHashOrigin ChecksumOrigin, bool IsDocumentMissing)> TryGetPdbMatchingSourceTextAsync(string sourceFilePath, ProjectId projectId, bool matchLoadedModulesOnly, CancellationToken cancellationToken) { - var (symChecksum, algorithm, origin) = await TryReadSourceFileChecksumFromPdb(sourceFilePath, projectId, cancellationToken).ConfigureAwait(false); + var (symChecksum, algorithm, origin) = await TryReadSourceFileChecksumFromPdb(sourceFilePath, projectId, matchLoadedModulesOnly, cancellationToken).ConfigureAwait(false); if (symChecksum.IsDefault) { return (Source: null, origin, IsDocumentMissing: true); @@ -292,7 +301,7 @@ public void CommitSolution(Solution solution, ImmutableArray updatedDo } } - private async Task<(ImmutableArray Checksum, SourceHashAlgorithm Algorithm, SourceHashOrigin Origin)> TryReadSourceFileChecksumFromPdb(string sourceFilePath, ProjectId projectId, CancellationToken cancellationToken) + private async Task<(ImmutableArray Checksum, SourceHashAlgorithm Algorithm, SourceHashOrigin Origin)> TryReadSourceFileChecksumFromPdb(string sourceFilePath, ProjectId projectId, bool matchLoadedModulesOnly, CancellationToken cancellationToken) { try { @@ -350,6 +359,11 @@ await Task.Factory.StartNew(ReadChecksum, cancellationToken, TaskCreationOptions return (default, default, SourceHashOrigin.LoadedPdb); } + if (matchLoadedModulesOnly) + { + return (default, default, SourceHashOrigin.None); + } + // if the module is not loaded check against build output: cancellationToken.ThrowIfCancellationRequested(); From deb9b05ba6adad522947571cd1f7af8d49f92c27 Mon Sep 17 00:00:00 2001 From: Tomas Matousek Date: Fri, 15 Nov 2019 16:25:40 -0800 Subject: [PATCH 4/5] Fix comments --- .../Core/Portable/Debugging/DebugInformationReaderProvider.cs | 1 - .../Core/Portable/EditAndContinue/CommittedSolution.cs | 4 ++-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/Features/Core/Portable/Debugging/DebugInformationReaderProvider.cs b/src/Features/Core/Portable/Debugging/DebugInformationReaderProvider.cs index eba9992f6ba1f..5accbd5234feb 100644 --- a/src/Features/Core/Portable/Debugging/DebugInformationReaderProvider.cs +++ b/src/Features/Core/Portable/Debugging/DebugInformationReaderProvider.cs @@ -1,7 +1,6 @@ // Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; -using System.Collections.Immutable; using System.IO; using System.Reflection; using System.Reflection.Metadata; diff --git a/src/Features/Core/Portable/EditAndContinue/CommittedSolution.cs b/src/Features/Core/Portable/EditAndContinue/CommittedSolution.cs index 20236db5765dd..148dcfdb29b7b 100644 --- a/src/Features/Core/Portable/EditAndContinue/CommittedSolution.cs +++ b/src/Features/Core/Portable/EditAndContinue/CommittedSolution.cs @@ -38,8 +38,8 @@ internal enum DocumentState /// The document content is matched with the build output instead of the loaded module /// since the module hasn't been loaded yet. /// - /// This document state may change to to or - /// or once the module has been loaded. + /// This document state may change to to , , + /// or or once the module has been loaded. /// MatchesBuildOutput = 1, From 0740e88953d4efd96c56dea9810997123e2a55d4 Mon Sep 17 00:00:00 2001 From: Tomas Matousek Date: Fri, 15 Nov 2019 16:28:00 -0800 Subject: [PATCH 5/5] Fix comment --- src/Features/Core/Portable/EditAndContinue/CommittedSolution.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Features/Core/Portable/EditAndContinue/CommittedSolution.cs b/src/Features/Core/Portable/EditAndContinue/CommittedSolution.cs index 148dcfdb29b7b..4c1ad079a6d01 100644 --- a/src/Features/Core/Portable/EditAndContinue/CommittedSolution.cs +++ b/src/Features/Core/Portable/EditAndContinue/CommittedSolution.cs @@ -38,7 +38,7 @@ internal enum DocumentState /// The document content is matched with the build output instead of the loaded module /// since the module hasn't been loaded yet. /// - /// This document state may change to to , , + /// This document state may change to , , /// or or once the module has been loaded. /// MatchesBuildOutput = 1,