internal/lsp: hold the gc details lock when storing diagnostics

At long last, with Pontus's help reproing on Github actions, we have
tracked down the race to the gc_details diagnostics.

Since toggling gc_details does not increment the snapshot ID, we have to
use careful locking to ensure that the gc_details diagnostics we store
are consistent with the current state of the gc_details toggle.

Updates golang/go#44099
Fixes golang/go#44826

Change-Id: I7b9108a829c98a84360c9012c1b60f4990839b5a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/304169
Trust: Robert Findley <[email protected]>
Run-TryBot: Robert Findley <[email protected]>
gopls-CI: kokoro <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Reviewed-by: Heschi Kreinick <[email protected]>

Author: findleyr

internal/lsp/diagnostics.go

@@ -285,15 +285,25 @@ func (s *Server) diagnosePkg(ctx context.Context, snapshot source.Snapshot, pkg
 		if err != nil {
 			event.Error(ctx, "warning: gc details", err, tag.Snapshot.Of(snapshot.ID()), tag.Package.Of(pkg.ID()))
 		}
-		for id, diags := range gcReports {
-			fh := snapshot.FindFile(id.URI)
-			// Don't publish gc details for unsaved buffers, since the underlying
-			// logic operates on the file on disk.
-			if fh == nil || !fh.Saved() {
-				continue
+		s.gcOptimizationDetailsMu.Lock()
+		_, enableGCDetails := s.gcOptimizationDetails[pkg.ID()]
+
+		// NOTE(golang/go#44826): hold the gcOptimizationDetails lock, and re-check
+		// whether gc optimization details are enabled, while storing gc_details
+		// results. This ensures that the toggling of GC details and clearing of
+		// diagnostics does not race with storing the results here.
+		if enableGCDetails {
+			for id, diags := range gcReports {
+				fh := snapshot.FindFile(id.URI)
+				// Don't publish gc details for unsaved buffers, since the underlying
+				// logic operates on the file on disk.
+				if fh == nil || !fh.Saved() {
+					continue
+				}
+				s.storeDiagnostics(snapshot, id.URI, gcDetailsSource, diags)
 			}
-			s.storeDiagnostics(snapshot, id.URI, gcDetailsSource, diags)
 		}
+		s.gcOptimizationDetailsMu.Unlock()
 	}
 }