From 9ab2d800b2c6ffc3c18089502ae25686c5042d97 Mon Sep 17 00:00:00 2001
From: Heschi Kreinick <heschi@google.com>
Date: Mon, 27 Jan 2020 16:59:43 -0500
Subject: [PATCH] internal/lsp/cache: handle go.mod conflicts in go list

The go command gets mad if go.mod has changed since it started, e.g.
because a new dependency was added by a concurrent go list call. Retry
loads if they hit a concurrency problem. See the comment for more
details.

Testing this is awkward. I ran a background script that constantly
modified the go.mod file and observed that gopls waited until it was
killed and then recovered.

Updates golang/go#36772.

Change-Id: I5636c99a5a94b415c4a6fbb71869b07e31d3fed0
Reviewed-on: https://go-review.googlesource.com/c/tools/+/216543
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
---
 internal/lsp/cache/load.go |  2 +-
 internal/lsp/cache/view.go | 51 +++++++++++++++++++++++++++++++++++++-
 2 files changed, 51 insertions(+), 2 deletions(-)

diff --git a/internal/lsp/cache/load.go b/internal/lsp/cache/load.go
index 144bf0009d..46ecd7e606 100644
--- a/internal/lsp/cache/load.go
+++ b/internal/lsp/cache/load.go
@@ -78,7 +78,7 @@ func (s *snapshot) load(ctx context.Context, scopes ...interface{}) ([]*metadata
 	defer done()
 
 	cfg := s.view.Config(ctx)
-	pkgs, err := packages.Load(cfg, query...)
+	pkgs, err := s.view.loadPackages(cfg, query...)
 
 	// If the context was canceled, return early. Otherwise, we might be
 	// type-checking an incomplete result. Check the context directly,
diff --git a/internal/lsp/cache/view.go b/internal/lsp/cache/view.go
index a56a9deec0..75c35d7d81 100644
--- a/internal/lsp/cache/view.go
+++ b/internal/lsp/cache/view.go
@@ -17,6 +17,7 @@ import (
 	"os"
 	"path/filepath"
 	"reflect"
+	"regexp"
 	"strings"
 	"sync"
 	"time"
@@ -39,7 +40,7 @@ type view struct {
 
 	options source.Options
 
-	// mu protects all mutable state of the view.
+	// mu protects most mutable state of the view.
 	mu sync.Mutex
 
 	// baseCtx is the context handed to NewView. This is the parent of all
@@ -110,6 +111,10 @@ type view struct {
 
 	// `go env` variables that need to be tracked.
 	gopath, gocache string
+
+	// LoadMu guards packages.Load calls and associated state.
+	loadMu         sync.Mutex
+	serializeLoads int
 }
 
 type builtinPackageHandle struct {
@@ -761,6 +766,50 @@ func (v *view) getGoEnv(ctx context.Context, env []string) (string, error) {
 	return "", nil
 }
 
+// 1.13: go: updates to go.mod needed, but contents have changed
+// 1.14: go: updating go.mod: existing contents have changed since last read
+var modConcurrencyError = regexp.MustCompile(`go:.*go.mod.*contents have changed`)
+
+// LoadPackages calls packages.Load, serializing requests if they fight over
+// go.mod changes.
+func (v *view) loadPackages(cfg *packages.Config, patterns ...string) ([]*packages.Package, error) {
+	// We want to run go list calls concurrently as much as possible. However,
+	// if go.mod updates are needed, only one can make them and the others will
+	// fail. We need to retry in those cases, but we don't want to thrash so
+	// badly we never recover. To avoid that, once we've seen one concurrency
+	// error, start serializing everything until the backlog has cleared out.
+	// This could all be avoided on 1.14 by using multiple -modfiles.
+
+	v.loadMu.Lock()
+	var locked bool // If true, we hold the mutex and have incremented.
+	if v.serializeLoads == 0 {
+		v.loadMu.Unlock()
+	} else {
+		locked = true
+		v.serializeLoads++
+	}
+	defer func() {
+		if locked {
+			v.serializeLoads--
+			v.loadMu.Unlock()
+		}
+	}()
+
+	for {
+		pkgs, err := packages.Load(cfg, patterns...)
+		if err == nil || !modConcurrencyError.MatchString(err.Error()) {
+			return pkgs, err
+		}
+
+		log.Error(cfg.Context, "Load concurrency error, will retry serially", err)
+		if !locked {
+			v.loadMu.Lock()
+			v.serializeLoads++
+			locked = true
+		}
+	}
+}
+
 // This function will return the main go.mod file for this folder if it exists and whether the -modfile
 // flag exists for this version of go.
 func (v *view) modfileFlagExists(ctx context.Context, env []string) (bool, error) {