From bc073721adb669c8cd012a7085592a35ac9d36c3 Mon Sep 17 00:00:00 2001
From: Rob Findley <rfindley@google.com>
Date: Wed, 4 Mar 2020 13:55:41 -0500
Subject: [PATCH] internal/lsp/cache: include session IDs in some cache keys

When caching file data specific to a session (anything with a Version or
tied to a view), we now need to be more careful about the existence of
multiple sessions.

This change fixes a few places where we appear to cache session data
without explicitly referring to the session. In principal this could
cause data corruption in multi-session gopls instances, but I have not
been able to force this to occur in either manual or automated testing.

Also fix a data race to the unsaved overlays:
https://storage.googleapis.com/go-build-log/588ee798/linux-amd64-race_d0762522.log

Updates golang/go#34111

Change-Id: I47117da1aba1afc2e211785544ad3f8b3416d15d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/222059
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rohan Challa <rohan@golang.org>
---
 internal/lsp/cache/mod.go     | 21 ++++++++++++++-------
 internal/lsp/cache/session.go |  1 +
 internal/lsp/source/view.go   |  6 +++++-
 3 files changed, 20 insertions(+), 8 deletions(-)

diff --git a/internal/lsp/cache/mod.go b/internal/lsp/cache/mod.go
index f1805ee03c..84bed0e615 100644
--- a/internal/lsp/cache/mod.go
+++ b/internal/lsp/cache/mod.go
@@ -32,12 +32,14 @@ const (
 )
 
 type modKey struct {
-	cfg   string
-	gomod string
-	view  string
+	sessionID string
+	cfg       string
+	gomod     string
+	view      string
 }
 
 type modTidyKey struct {
+	sessionID       string
 	cfg             string
 	gomod           string
 	imports         string
@@ -137,9 +139,10 @@ func (s *snapshot) ModHandle(ctx context.Context, fh source.FileHandle) source.M
 	cfg := s.Config(ctx)
 
 	key := modKey{
-		cfg:   hashConfig(cfg),
-		gomod: fh.Identity().String(),
-		view:  folder,
+		sessionID: s.view.session.id,
+		cfg:       hashConfig(cfg),
+		gomod:     fh.Identity().String(),
+		view:      folder,
 	}
 	h := s.view.session.cache.store.Bind(key, func(ctx context.Context) interface{} {
 		ctx, done := trace.StartSpan(ctx, "cache.ModHandle", telemetry.File.Of(uri))
@@ -295,10 +298,14 @@ func (s *snapshot) ModTidyHandle(ctx context.Context, realfh source.FileHandle)
 	if err != nil {
 		return nil, err
 	}
+	s.mu.Lock()
+	overlayHash := hashUnsavedOverlays(s.files)
+	s.mu.Unlock()
 	key := modTidyKey{
+		sessionID:       s.view.session.id,
 		view:            folder,
 		imports:         imports,
-		unsavedOverlays: hashUnsavedOverlays(s.files),
+		unsavedOverlays: overlayHash,
 		gomod:           realfh.Identity().Identifier,
 		cfg:             hashConfig(cfg),
 	}
diff --git a/internal/lsp/cache/session.go b/internal/lsp/cache/session.go
index 4575d6d56a..1655262c51 100644
--- a/internal/lsp/cache/session.go
+++ b/internal/lsp/cache/session.go
@@ -54,6 +54,7 @@ func (o *overlay) Identity() source.FileIdentity {
 	return source.FileIdentity{
 		URI:        o.uri,
 		Identifier: o.hash,
+		SessionID:  o.session.id,
 		Version:    o.version,
 		Kind:       o.kind,
 	}
diff --git a/internal/lsp/source/view.go b/internal/lsp/source/view.go
index 0c0f5f50dd..6da78605da 100644
--- a/internal/lsp/source/view.go
+++ b/internal/lsp/source/view.go
@@ -327,7 +327,11 @@ type FileHandle interface {
 type FileIdentity struct {
 	URI span.URI
 
-	// Version is the version of the file, as specified by the client.
+	// SessionID is the ID of the LSP session.
+	SessionID string
+
+	// Version is the version of the file, as specified by the client. It should
+	// only be set in combination with SessionID.
 	Version float64
 
 	// Identifier represents a unique identifier for the file.