internal/lsp: add a mutex around the view's options

The options can be modified without the view being recreated, so we need
a mutex there.

Change-Id: I87e881835622a941fce98e4a1062aa41acd84fcb
Reviewed-on: https://go-review.googlesource.com/c/tools/+/227022
Reviewed-by: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>

internal/lsp/cache/os_darwin.go

@@ -1,6 +1,7 @@
 // Copyright 2020 The Go Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style
 // license that can be found in the LICENSE file.
+
 package cache
 
 import (

internal/lsp/cache/snapshot.go

@@ -89,8 +89,12 @@ func (s *snapshot) View() source.View {
 // Config returns the configuration used for the snapshot's interaction with the
 // go/packages API.
 func (s *snapshot) Config(ctx context.Context) *packages.Config {
-	env, buildFlags := s.view.env()
-	cfg := &packages.Config{
+	s.view.optionsMu.Lock()
+	env, buildFlags := s.view.envLocked()
+	verboseOutput := s.view.options.VerboseOutput
+	s.view.optionsMu.Unlock()
+
+	return &packages.Config{
 		Env:        env,
 		Dir:        s.view.folder.Filename(),
 		Context:    ctx,
@@ -107,13 +111,12 @@ func (s *snapshot) Config(ctx context.Context) *packages.Config {
 			panic("go/packages must not be used to parse files")
 		},
 		Logf: func(format string, args ...interface{}) {
-			if s.view.options.VerboseOutput {
+			if verboseOutput {
 				event.Print(ctx, fmt.Sprintf(format, args...))
 			}
 		},
 		Tests: true,
 	}
-	return cfg
 }
 
 func (s *snapshot) buildOverlay() map[string][]byte {

internal/lsp/cache/view.go

@@ -37,7 +37,8 @@ type view struct {
 	session *Session
 	id      string
 
-	options source.Options
+	optionsMu sync.Mutex
+	options   source.Options
 
 	// mu protects most mutable state of the view.
 	mu sync.Mutex
@@ -172,6 +173,8 @@ func (v *view) Folder() span.URI {
 }
 
 func (v *view) Options() source.Options {
+	v.optionsMu.Lock()
+	defer v.optionsMu.Unlock()
 	return v.options
 }
 
@@ -189,16 +192,19 @@ func minorOptionsChange(a, b source.Options) bool {
 
 func (v *view) SetOptions(ctx context.Context, options source.Options) (source.View, error) {
 	// no need to rebuild the view if the options were not materially changed
+	v.optionsMu.Lock()
 	if minorOptionsChange(v.options, options) {
 		v.options = options
+		v.optionsMu.Unlock()
 		return v, nil
 	}
+	v.optionsMu.Unlock()
 	newView, _, err := v.session.updateView(ctx, v, options)
 	return newView, err
 }
 
 func (v *view) Rebuild(ctx context.Context) (source.Snapshot, error) {
-	_, snapshot, err := v.session.updateView(ctx, v, v.options)
+	_, snapshot, err := v.session.updateView(ctx, v, v.Options())
 	return snapshot, err
 }
 
@@ -263,7 +269,9 @@ func (v *view) buildBuiltinPackage(ctx context.Context, goFiles []string) error
 }
 
 func (v *view) WriteEnv(ctx context.Context, w io.Writer) error {
-	env, buildFlags := v.env()
+	v.optionsMu.Lock()
+	env, buildFlags := v.envLocked()
+	v.optionsMu.Unlock()
 	// TODO(rstambler): We could probably avoid running this by saving the
 	// output on original create, but I'm not sure if it's worth it.
 	inv := gocommand.Invocation{
@@ -348,13 +356,16 @@ func (v *view) refreshProcessEnv() {
 }
 
 func (v *view) buildProcessEnv(ctx context.Context) (*imports.ProcessEnv, error) {
-	env, buildFlags := v.env()
+	v.optionsMu.Lock()
+	env, buildFlags := v.envLocked()
+	localPrefix, verboseOutput := v.options.LocalPrefix, v.options.VerboseOutput
+	v.optionsMu.Unlock()
 	processEnv := &imports.ProcessEnv{
 		WorkingDir:  v.folder.Filename(),
 		BuildFlags:  buildFlags,
-		LocalPrefix: v.options.LocalPrefix,
+		LocalPrefix: localPrefix,
 	}
-	if v.options.VerboseOutput {
+	if verboseOutput {
 		processEnv.Logf = func(format string, args ...interface{}) {
 			event.Print(ctx, fmt.Sprintf(format, args...))
 		}
@@ -382,7 +393,7 @@ func (v *view) buildProcessEnv(ctx context.Context) (*imports.ProcessEnv, error)
 	return processEnv, nil
 }
 
-func (v *view) env() ([]string, []string) {
+func (v *view) envLocked() ([]string, []string) {
 	// We want to run the go commands with the -modfile flag if the version of go
 	// that we are using supports it.
 	buildFlags := v.options.BuildFlags