Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Race condition bug in Formatter when requesting formatted changes for a sub-span in a file #67887

Closed
mavasani opened this issue Apr 20, 2023 · 1 comment
Labels
Area-IDE Bug Feature - IDE0055 Formatting Tenet-Reliability Customer telemetry indicates that the product is failing in a crash/hang/dataloss manner. untriaged Issues and PRs which have not yet been triaged by a lead

Comments

@mavasani
Copy link
Contributor

mavasani commented Apr 20, 2023

As part of testing the changes in #67818, I have identified a race condition bug in the Formatter wherein it intermittently reports (incorrect) non-zero formatting changes when asked for formatting changes for a sub-span but reports zero formatting changes when asked for full file span.

Test code where I get wrong IDE0055 violations from lightbulb (span-based) invocation:

image

Below is the local (temporary) product change in FormattingAnalyzerHelper.cs to handle optional FilterSpan from analysis context. For the above case, I am hitting the breakpoint at line formattingBug = true, indicating the formatter race condition bug I explained above:

image

            var node = context.FilterSpan.HasValue ? root.FindNode(context.FilterSpan.GetValueOrDefault()) : root;
            var formattingChanges = Formatter.GetFormattedTextChanges(node, formattingProvider, options, cancellationToken);

            var formattingBug = false;
            if (formattingChanges.Count > 0 && context.FilterSpan.HasValue)
            {
                var rootChanges = Formatter.GetFormattedTextChanges(root, formattingProvider, options, cancellationToken);
                if (rootChanges.Count == 0)
                    formattingBug = true;
            }
@mavasani mavasani added Bug Area-IDE Tenet-Reliability Customer telemetry indicates that the product is failing in a crash/hang/dataloss manner. Feature - IDE0055 Formatting labels Apr 20, 2023
@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged Issues and PRs which have not yet been triaged by a lead label Apr 20, 2023
@mavasani
Copy link
Contributor Author

Turns out this is not a Formatter bug, just that I was using the wrong Formatter API for get formatted text changes.

// Wrong API
var node = context.FilterSpan.HasValue ? root.FindNode(context.FilterSpan.GetValueOrDefault()) : root;
var formattingChanges = Formatter.GetFormattedTextChanges(node, formattingProvider, options, cancellationToken);

// Correct API
var span = context.FilterSpan.HasValue ? context.FilterSpan.GetValueOrDefault() : root.FullSpan;
var spans = SpecializedCollections.SingletonEnumerable(span);
var formattingChanges = Formatter.GetFormattedTextChanges(root, spans, formattingProvider, options, cancellationToken);

This was showing up as a race condition because sometimes the background analysis would compute and cache full document diagnostics before the lightbulb span based diagnostics are requested. After making the above changes, I no longer see this issue.

@mavasani mavasani closed this as not planned Won't fix, can't repro, duplicate, stale Apr 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-IDE Bug Feature - IDE0055 Formatting Tenet-Reliability Customer telemetry indicates that the product is failing in a crash/hang/dataloss manner. untriaged Issues and PRs which have not yet been triaged by a lead
Projects
None yet
Development

No branches or pull requests

1 participant