Skip to content

Commit

Permalink
Fix non-leaf active statement spans when entering break mode with una…
Browse files Browse the repository at this point in the history 
…pplied run-mode changes (#52157)
  • Loading branch information
@tmat
tmat authored Mar 26, 2021
1 parent 7ff7779 commit 9e701ca
Show file tree
Hide file tree
Showing 6 changed files with 188 additions and 48 deletions.
19 changes: 7 additions & 12 deletions src/EditorFeatures/CSharpTest/EditAndContinue/ActiveStatementTrackingServiceTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,12 @@
#nullable disable

using System.Collections.Immutable;
using System.IO;
using System.Linq;
using System.Text;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis.EditAndContinue;
using Microsoft.CodeAnalysis.EditAndContinue.UnitTests;
using Microsoft.CodeAnalysis.Editor.Implementation.EditAndContinue;
using Microsoft.CodeAnalysis.Editor.UnitTests;
using Microsoft.CodeAnalysis.Editor.UnitTests.Workspaces;
using Microsoft.CodeAnalysis.Host;
using Microsoft.CodeAnalysis.Test.Utilities;
using Microsoft.CodeAnalysis.Text;
using Microsoft.VisualStudio.Debugger.Contracts.EditAndContinue;
Expand Down Expand Up @@ -197,7 +192,7 @@ public async Task TrackingService_GetLatestSpansAsync(bool scheduleInitialTracki

if (scheduleInitialTrackingBeforeOpenDoc)
{
await trackingSession.TrackActiveSpansAsync().ConfigureAwait(false);
await trackingSession.TrackActiveSpansAsync(solution, CancellationToken.None);

var spans1 = trackingSession.Test_GetTrackingSpans();
AssertEx.Equal(new[]
Expand All @@ -206,25 +201,25 @@ public async Task TrackingService_GetLatestSpansAsync(bool scheduleInitialTracki
$"V0 →←@[20..25): IsLeafFrame"
}, spans1[document1.Id].Select(s => $"{s.Span}: {s.Flags}"));

var spans2 = await trackingSession.GetSpansAsync(document1, CancellationToken.None).ConfigureAwait(false);
var spans2 = await trackingSession.GetSpansAsync(document1, CancellationToken.None);
AssertEx.Equal(new[] { "[10..15)", "[20..25)" }, spans2.Select(s => s.ToString()));

var spans3 = await trackingSession.GetSpansAsync(document2, CancellationToken.None).ConfigureAwait(false);
var spans3 = await trackingSession.GetSpansAsync(document2, CancellationToken.None);
Assert.Empty(spans3);
}

var spans4 = await trackingSession.GetAdjustedTrackingSpansAsync(document1, snapshot1, CancellationToken.None).ConfigureAwait(false);
var spans4 = await trackingSession.GetAdjustedTrackingSpansAsync(document1, snapshot1, CancellationToken.None);
AssertEx.Equal(new[]
{
$"V0 →←@[11..16): IsNonLeafFrame",
$"V0 →←@[21..26): IsLeafFrame"
}, spans4.Select(s => $"{s.Span}: {s.Flags}"));

AssertEx.Empty(await trackingSession.GetAdjustedTrackingSpansAsync(document2, snapshot2, CancellationToken.None).ConfigureAwait(false));
AssertEx.Empty(await trackingSession.GetAdjustedTrackingSpansAsync(document2, snapshot2, CancellationToken.None));

if (!scheduleInitialTrackingBeforeOpenDoc)
{
await trackingSession.TrackActiveSpansAsync().ConfigureAwait(false);
await trackingSession.TrackActiveSpansAsync(solution, CancellationToken.None);

var spans5 = trackingSession.Test_GetTrackingSpans();
AssertEx.Equal(new[]
Expand All @@ -237,7 +232,7 @@ public async Task TrackingService_GetLatestSpansAsync(bool scheduleInitialTracki
// we are not able to determine active statements in a document:
encService.GetAdjustedActiveStatementSpansImpl = (_, _) => default;

var spans6 = await trackingSession.GetAdjustedTrackingSpansAsync(document1, snapshot1, CancellationToken.None).ConfigureAwait(false);
var spans6 = await trackingSession.GetAdjustedTrackingSpansAsync(document1, snapshot1, CancellationToken.None);
AssertEx.Equal(new[]
{
$"V0 →←@[11..16): IsNonLeafFrame",
Expand Down
12 changes: 5 additions & 7 deletions src/EditorFeatures/Core/Implementation/EditAndContinue/ActiveStatementTrackingService.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ public ActiveStatementTrackingService(Workspace workspace)
_spanProvider = new RemoteEditAndContinueServiceProxy(_workspace);
}

public void StartTracking()
public async ValueTask StartTrackingAsync(Solution solution, CancellationToken cancellationToken)
{
var newSession = new TrackingSession(_workspace, _spanProvider);
if (Interlocked.CompareExchange(ref _session, newSession, null) != null)
Expand All @@ -69,8 +69,7 @@ public void StartTracking()
Contract.Fail("Can only track active statements for a single edit session.");
}

// fire and forget on a background thread:
_ = Task.Run(() => newSession.TrackActiveSpansAsync()).ReportNonFatalErrorAsync();
await newSession.TrackActiveSpansAsync(solution, cancellationToken).ConfigureAwait(false);

TrackingChanged?.Invoke();
}
Expand Down Expand Up @@ -164,7 +163,7 @@ private async Task TrackActiveSpansAsync(Document document, CancellationToken ca
}
}

internal async Task TrackActiveSpansAsync()
internal async Task TrackActiveSpansAsync(Solution solution, CancellationToken cancellationToken)
{
try
{
Expand All @@ -174,8 +173,7 @@ internal async Task TrackActiveSpansAsync()
return;
}

var currentSolution = _workspace.CurrentSolution;
var baseActiveStatementSpans = await _spanProvider.GetBaseActiveStatementSpansAsync(currentSolution, openDocumentIds, _cancellationSource.Token).ConfigureAwait(false);
var baseActiveStatementSpans = await _spanProvider.GetBaseActiveStatementSpansAsync(solution, openDocumentIds, cancellationToken).ConfigureAwait(false);
if (baseActiveStatementSpans.IsDefault)
{
// Edit session not in progress.
Expand All @@ -187,7 +185,7 @@ internal async Task TrackActiveSpansAsync()

foreach (var id in openDocumentIds)
{
documents.Add(await currentSolution.GetDocumentAsync(id, includeSourceGenerated: true, _cancellationSource.Token).ConfigureAwait(false));
documents.Add(await solution.GetDocumentAsync(id, includeSourceGenerated: true, cancellationToken).ConfigureAwait(false));
}

lock (_trackingSpans)
Expand Down
3 changes: 2 additions & 1 deletion src/EditorFeatures/Core/Implementation/EditAndContinue/IActiveStatementTrackingService.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@ namespace Microsoft.CodeAnalysis.Editor.Implementation.EditAndContinue
{
internal interface IActiveStatementTrackingService : IWorkspaceService
{
void StartTracking();
ValueTask StartTrackingAsync(Solution solution, CancellationToken cancellationToken);

void EndTracking();

/// <summary>
Expand Down
166 changes: 143 additions & 23 deletions src/EditorFeatures/Test/EditAndContinue/EditAndContinueWorkspaceServiceTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2882,18 +2882,18 @@ public async Task ActiveStatements()
var baseSpans = await service.GetBaseActiveStatementSpansAsync(workspace.CurrentSolution, ImmutableArray.Create(document1.Id), CancellationToken.None);
AssertEx.Equal(new[]
{
$"({activeLineSpan11}, IsNonLeafFrame)",
$"({activeLineSpan12}, IsLeafFrame)"
}, baseSpans.Single().Select(s => s.ToString()));
(activeLineSpan11, ActiveStatementFlags.IsNonLeafFrame),
(activeLineSpan12, ActiveStatementFlags.IsLeafFrame)
}, baseSpans.Single());

var trackedActiveSpans1 = ImmutableArray.Create(activeSpan11, activeSpan12);

var currentSpans = await service.GetAdjustedActiveStatementSpansAsync(document1, (_) => new(trackedActiveSpans1), CancellationToken.None);
AssertEx.Equal(new[]
{
$"({activeLineSpan11}, IsNonLeafFrame)",
$"({activeLineSpan12}, IsLeafFrame)"
}, currentSpans.Select(s => s.ToString()));
(activeLineSpan11, ActiveStatementFlags.IsNonLeafFrame),
(activeLineSpan12, ActiveStatementFlags.IsLeafFrame)
}, currentSpans);

Assert.Equal(activeLineSpan11,
await service.GetCurrentActiveStatementPositionAsync(document1.Project.Solution, (_, _) => new(trackedActiveSpans1), activeInstruction1, CancellationToken.None));
Expand All @@ -2908,19 +2908,12 @@ public async Task ActiveStatements()
// tracking span update triggered by the edit:
var trackedActiveSpans2 = ImmutableArray.Create(activeSpan21, activeSpan22);

baseSpans = await service.GetBaseActiveStatementSpansAsync(workspace.CurrentSolution, ImmutableArray.Create(document2.Id), CancellationToken.None);
AssertEx.Equal(new[]
{
$"({activeLineSpan11}, IsNonLeafFrame)",
$"({activeLineSpan12}, IsLeafFrame)"
}, baseSpans.Single().Select(s => s.ToString()));

currentSpans = await service.GetAdjustedActiveStatementSpansAsync(document2, _ => new(trackedActiveSpans2), CancellationToken.None);
AssertEx.Equal(new[]
{
$"({adjustedActiveLineSpan1}, IsNonLeafFrame)",
$"({adjustedActiveLineSpan2}, IsLeafFrame)"
}, currentSpans.Select(s => s.ToString()));
(adjustedActiveLineSpan1, ActiveStatementFlags.IsNonLeafFrame),
(adjustedActiveLineSpan2, ActiveStatementFlags.IsLeafFrame)
}, currentSpans);

Assert.Equal(adjustedActiveLineSpan1,
await service.GetCurrentActiveStatementPositionAsync(workspace.CurrentSolution, (_, _) => new(trackedActiveSpans2), activeInstruction1, CancellationToken.None));
Expand Down Expand Up @@ -2981,10 +2974,6 @@ public async Task ActiveStatements_SyntaxErrorOrOutOfSyncDocument(bool isOutOfSy
EnterBreakState(service, activeStatements);
var editSession = service.Test_GetEditSession();

// change the source (valid edit):
workspace.ChangeDocument(documentId, sourceTextV2);
var document2 = workspace.CurrentSolution.GetDocument(documentId);

var baseSpans = await service.GetBaseActiveStatementSpansAsync(workspace.CurrentSolution, ImmutableArray.Create(documentId), CancellationToken.None);

if (isOutOfSync)
Expand All @@ -2995,11 +2984,15 @@ public async Task ActiveStatements_SyntaxErrorOrOutOfSyncDocument(bool isOutOfSy
{
AssertEx.Equal(new[]
{
$"({activeLineSpan11}, IsNonLeafFrame)",
$"({activeLineSpan12}, IsLeafFrame)"
}, baseSpans.Single().Select(s => s.ToString()));
(activeLineSpan11, ActiveStatementFlags.IsNonLeafFrame),
(activeLineSpan12, ActiveStatementFlags.IsLeafFrame)
}, baseSpans.Single());
}

// change the source (valid edit):
workspace.ChangeDocument(documentId, sourceTextV2);
var document2 = workspace.CurrentSolution.GetDocument(documentId);

// no active statements due to syntax error or out-of-sync document:
var currentSpans = await service.GetAdjustedActiveStatementSpansAsync(document2, s_noDocumentActiveSpans, CancellationToken.None);
Assert.True(currentSpans.IsDefault);
Expand Down Expand Up @@ -3160,5 +3153,132 @@ static void F()

ExitBreakState();
}

/// <summary>
/// Scenario:
/// - F5
/// - edit, but not apply the edits
/// - break
/// </summary>
/// <returns></returns>
[Fact]
public async Task BreakInPresenceOfUnappliedChanges()
{
var markedSource1 =
@"class Test
{
static bool B() => true;
static void G() { while (B()); <AS:0>}</AS:0>
static void F()
{
<AS:1>G();</AS:1>
}
}";

var markedSource2 =
@"class Test
{
static bool B() => true;
static void G() { while (B()); <AS:0>}</AS:0>
static void F()
{
B();
<AS:1>G();</AS:1>
}
}";

var markedSource3 =
@"class Test
{
static bool B() => true;
static void G() { while (B()); <AS:0>}</AS:0>
static void F()
{
B();
B();
<AS:1>G();</AS:1>
}
}";

var moduleId = EmitAndLoadLibraryToDebuggee(ActiveStatementsDescription.ClearTags(markedSource1));

using var workspace = CreateWorkspace();
var project = AddDefaultTestProject(workspace, ActiveStatementsDescription.ClearTags(markedSource2));
var documentId = project.DocumentIds.Single();
var solution = project.Solution;

var service = CreateEditAndContinueService();
var debuggingSession = await StartDebuggingSessionAsync(service, solution);

// Update to snapshot 2, but don't apply

solution = solution.WithDocumentText(documentId, SourceText.From(ActiveStatementsDescription.ClearTags(markedSource2), Encoding.UTF8));

// EnC update F v2 -> v3

EnterBreakState(service, GetActiveStatementDebugInfos(
new[] { markedSource1 },
modules: new[] { moduleId, moduleId },
methodRowIds: new[] { 2, 3 },
methodVersions: new[] { 1, 1 },
flags: new[]
{
ActiveStatementFlags.MethodUpToDate | ActiveStatementFlags.IsLeafFrame, // G
ActiveStatementFlags.MethodUpToDate | ActiveStatementFlags.IsNonLeafFrame, // F
}));

// check that the active statement is mapped correctly to snapshot v2:
var expectedSpanG1 = new LinePositionSpan(new LinePosition(3, 41), new LinePosition(3, 42));
var expectedSpanF1 = new LinePositionSpan(new LinePosition(8, 14), new LinePosition(8, 18));

var activeInstructionF1 = new ManagedInstructionId(new ManagedMethodId(moduleId, 0x06000003, version: 1), ilOffset: 0);
var span = await service.GetCurrentActiveStatementPositionAsync(solution, s_noSolutionActiveSpans, activeInstructionF1, CancellationToken.None);
Assert.Equal(expectedSpanF1, span);

var spans = (await service.GetBaseActiveStatementSpansAsync(solution, ImmutableArray.Create(documentId), CancellationToken.None)).Single();
AssertEx.Equal(new[]
{
(expectedSpanG1, ActiveStatementFlags.MethodUpToDate | ActiveStatementFlags.IsLeafFrame),
(expectedSpanF1, ActiveStatementFlags.MethodUpToDate | ActiveStatementFlags.IsNonLeafFrame)
}, spans);

solution = solution.WithDocumentText(documentId, SourceText.From(ActiveStatementsDescription.ClearTags(markedSource3), Encoding.UTF8));

// check that the active statement is mapped correctly to snapshot v3:
var expectedSpanG2 = new LinePositionSpan(new LinePosition(3, 41), new LinePosition(3, 42));
var expectedSpanF2 = new LinePositionSpan(new LinePosition(9, 14), new LinePosition(9, 18));

span = await service.GetCurrentActiveStatementPositionAsync(solution, s_noSolutionActiveSpans, activeInstructionF1, CancellationToken.None);
Assert.Equal(expectedSpanF2, span);

spans = (await service.GetBaseActiveStatementSpansAsync(solution, ImmutableArray.Create(documentId), CancellationToken.None)).Single();
AssertEx.Equal(new[]
{
(expectedSpanG2, ActiveStatementFlags.MethodUpToDate | ActiveStatementFlags.IsLeafFrame),
(expectedSpanF2, ActiveStatementFlags.MethodUpToDate | ActiveStatementFlags.IsNonLeafFrame)
}, spans);

// no rude edits:
var document1 = solution.GetDocument(documentId);
var diagnostics = await service.GetDocumentDiagnosticsAsync(document1, s_noDocumentActiveSpans, CancellationToken.None);
Assert.Empty(diagnostics);

var (updates, emitDiagnostics) = await EmitSolutionUpdateAsync(service, solution);
Assert.Empty(emitDiagnostics);
Assert.Equal(0x06000003, updates.Updates.Single().UpdatedMethods.Single());
Assert.Equal(ManagedModuleUpdateStatus.Ready, updates.Status);

CommitSolutionUpdate(service);

AssertEx.Equal(new[]
{
"0x06000003 v1 | AS (7,14)-(7,18) δ=2",
}, InspectNonRemappableRegions(debuggingSession.NonRemappableRegions));

ExitBreakState();
}
}
}
30 changes: 26 additions & 4 deletions src/Features/Core/Portable/EditAndContinue/EditAndContinueWorkspaceService.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -335,14 +335,36 @@ public void DiscardSolutionUpdate()

foreach (var documentId in documentIds)
{
if (baseActiveStatements.DocumentMap.TryGetValue(documentId, out var documentActiveStatements))
if (baseActiveStatements.DocumentMap.TryGetValue(documentId, out var documentBaseActiveStatements))
{
var document = await solution.GetDocumentAsync(documentId, includeSourceGenerated: true, cancellationToken).ConfigureAwait(false);
var (baseDocument, _) = await lastCommittedSolution.GetDocumentAndStateAsync(documentId, document, cancellationToken).ConfigureAwait(false);
if (baseDocument != null)
if (baseDocument != null && document != null)
{
spans.Add(documentActiveStatements.SelectAsArray(s => (s.Span, s.Flags)));
continue;
// Calculate diff with no prior knowledge of active statement spans.
// Translates active statements in the last committed solution snapshot to the current solution.
// Do not cache this result, it's only needed when entering a break mode.
// Optimize for the common case of the identical document.
if (baseDocument == document)
{
spans.Add(documentBaseActiveStatements.SelectAsArray(s => (s.Span, s.Flags)));
continue;
}

var analyzer = document.Project.LanguageServices.GetRequiredService<IEditAndContinueAnalyzer>();

var analysis = await analyzer.AnalyzeDocumentAsync(
baseDocument.Project,
documentBaseActiveStatements,
document,
newActiveStatementSpans: ImmutableArray<TextSpan>.Empty,
cancellationToken).ConfigureAwait(false);

if (!analysis.ActiveStatements.IsDefault)
{
spans.Add(analysis.ActiveStatements.SelectAsArray(s => (s.Span, s.Flags)));
continue;
}
}
}

Expand Down
6 changes: 5 additions & 1 deletion ...alStudio/Core/Def/Implementation/EditAndContinue/ManagedEditAndContinueLanguageService.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,8 @@ public async Task EnterBreakStateAsync(CancellationToken cancellationToken)
return;
}

var solution = _proxy.Workspace.CurrentSolution;

try
{
await _proxy.BreakStateEnteredAsync(_diagnosticService, cancellationToken).ConfigureAwait(false);
Expand All @@ -95,7 +97,9 @@ public async Task EnterBreakStateAsync(CancellationToken cancellationToken)
return;
}

_activeStatementTrackingService.StartTracking();
// Start tracking after we entered break state so that break-state session is active.
// This is potentially costly operation but entering break state is non-blocking so it should be ok to await.
await _activeStatementTrackingService.StartTrackingAsync(solution, cancellationToken).ConfigureAwait(false);
}

public Task ExitBreakStateAsync(CancellationToken cancellationToken)
Expand Down

0 comments on commit 9e701ca

Please sign in to comment.