From f560bc877f30412e048ba6da827d032b46f8302f Mon Sep 17 00:00:00 2001 From: Robert Findley Date: Tue, 26 Jul 2022 17:58:48 -0400 Subject: [PATCH] internal/lsp/cache: don't set context cancellation as a critical err Fix a tricky race in gopls/internal/regtest/diagnostics.Test_Issue38211: When reloading the workspace, we can encounter context cancellation if the snapshot is cancelled, and can write this cancellation as a critical error *before* the context is cloned, leading to a state where there is a critical error that won't go away. This should resolve test flakes reported in golang/go#44098. For golang/go#44098 Change-Id: I41c0f49b2fe999131f4c31166e69b2cde85470b7 Reviewed-on: https://go-review.googlesource.com/c/tools/+/419502 gopls-CI: kokoro TryBot-Result: Gopher Robot Run-TryBot: Robert Findley Reviewed-by: Bryan Mills --- internal/lsp/cache/view.go | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/internal/lsp/cache/view.go b/internal/lsp/cache/view.go index f974cfcf57..0a64b76306 100644 --- a/internal/lsp/cache/view.go +++ b/internal/lsp/cache/view.go @@ -656,9 +656,14 @@ func (s *snapshot) loadWorkspace(ctx context.Context, firstAttempt bool) { } }() - // If we have multiple modules, we need to load them by paths. - var scopes []interface{} - var modDiagnostics []*source.Diagnostic + // TODO(rFindley): we should only locate template files on the first attempt, + // or guard it via a different mechanism. + s.locateTemplateFiles(ctx) + + // Collect module paths to load by parsing go.mod files. If a module fails to + // parse, capture the parsing failure as a critical diagnostic. + var scopes []interface{} // scopes to load + var modDiagnostics []*source.Diagnostic // diagnostics for broken go.mod files addError := func(uri span.URI, err error) { modDiagnostics = append(modDiagnostics, &source.Diagnostic{ URI: uri, @@ -668,20 +673,22 @@ func (s *snapshot) loadWorkspace(ctx context.Context, firstAttempt bool) { }) } - // TODO(rFindley): we should only locate template files on the first attempt, - // or guard it via a different mechanism. - s.locateTemplateFiles(ctx) - if len(s.workspace.getActiveModFiles()) > 0 { for modURI := range s.workspace.getActiveModFiles() { + // Be careful not to add context cancellation errors as critical module + // errors. fh, err := s.GetFile(ctx, modURI) if err != nil { - addError(modURI, err) + if ctx.Err() == nil { + addError(modURI, err) + } continue } parsed, err := s.ParseMod(ctx, fh) if err != nil { - addError(modURI, err) + if ctx.Err() == nil { + addError(modURI, err) + } continue } if parsed.File == nil || parsed.File.Module == nil {