fix(gazelle): tsconfig read concurrency (#3742)

Fix #547

Manually tested on the [sourcegraph
repo](https://github.com/sourcegraph/sourcegraph/) and can no longer
reproduce.

---------

Co-authored-by: Greg Magolan <[email protected]>
GitOrigin-RevId: b298127493635001d51aae56e6ccf88be36bd538

cmd/aspect/root/BUILD.bazel

@@ -34,10 +34,10 @@ go_library(
         "//cmd/aspect/sync",
         "//cmd/aspect/test",
         "//cmd/aspect/version",
-        "//docs/help/topics",
         "//pkg/aspect/root/flags",
         "//pkg/ioutils",
         "//pkg/plugin/system",
+        "//docs/cli/help",
         "@com_github_fatih_color//:color",
         "@com_github_mattn_go_isatty//:go-isatty",
         "@com_github_spf13_cobra//:cobra",

cmd/aspect/root/root.go

@@ -52,10 +52,10 @@ import (
 	"aspect.build/cli/cmd/aspect/sync"
 	"aspect.build/cli/cmd/aspect/test"
 	"aspect.build/cli/cmd/aspect/version"
-	help_docs "aspect.build/cli/docs/help/topics"
 	"aspect.build/cli/pkg/aspect/root/flags"
 	"aspect.build/cli/pkg/ioutils"
 	"aspect.build/cli/pkg/plugin/system"
+	help_docs "aspect.build/cli/docs/help"
 )
 
 var (

gazelle/js/typescript/config.go

@@ -5,6 +5,7 @@ import (
 	"path"
 	"path/filepath"
 	"strings"
+	"sync"
 )
 
 type workspacePath struct {
@@ -14,9 +15,12 @@ type workspacePath struct {
 }
 
 type TsConfigMap struct {
-	configFiles map[string]*workspacePath
-
-	configs map[string]*TsConfig
+	// `configFiles` is created during the gazelle configure phase which is single threaded so doesn't
+	// require mutex projection. Just `configs` has concurrency considerations since it is lazy
+	// loading on multiple threads in the generate phase.
+	configFiles  map[string]*workspacePath
+	configs      map[string]*TsConfig
+	configsMutex sync.RWMutex
 }
 
 type TsWorkspace struct {
@@ -26,8 +30,9 @@ type TsWorkspace struct {
 func NewTsWorkspace() *TsWorkspace {
 	return &TsWorkspace{
 		cm: &TsConfigMap{
-			configFiles: make(map[string]*workspacePath),
-			configs:     make(map[string]*TsConfig),
+			configFiles:  make(map[string]*workspacePath),
+			configs:      make(map[string]*TsConfig),
+			configsMutex: sync.RWMutex{},
 		},
 	}
 }
@@ -46,21 +51,25 @@ func (tc *TsWorkspace) AddTsConfigFile(root, rel, fileName string) {
 }
 
 func (tc *TsWorkspace) GetTsConfigFile(rel string) *TsConfig {
-	// Previously parsed
+	// No file exists
+	p := tc.cm.configFiles[rel]
+	if p == nil {
+		return nil
+	}
+
+	// Lock the configs mutex
+	tc.cm.configsMutex.Lock()
+	defer tc.cm.configsMutex.Unlock()
+
+	// Check for previously parsed
 	if c := tc.cm.configs[rel]; c != nil {
 		if c == &InvalidTsconfig {
 			return nil
 		}
 		return c
 	}
 
-	// Does not exist
-	p := tc.cm.configFiles[rel]
-	if p == nil {
-		return nil
-	}
-
-	c, err := parseTsConfigJSONFile(tc.cm, p.root, p.rel, p.fileName)
+	c, err := parseTsConfigJSONFile(tc.cm.configs, p.root, p.rel, p.fileName)
 	if err != nil {
 		fmt.Printf("Failed to parse tsconfig file %s: %v\n", path.Join(p.rel, p.fileName), err)
 		return nil
@@ -69,10 +78,6 @@ func (tc *TsWorkspace) GetTsConfigFile(rel string) *TsConfig {
 	return c
 }
 
-func (tc *TsWorkspace) hasConfig(rel string) bool {
-	return tc.cm.configFiles[rel] != nil && tc.cm.configs[rel] != &InvalidTsconfig
-}
-
 func (tc *TsWorkspace) getConfig(f string) (string, *TsConfig) {
 	dir := f
 
@@ -82,7 +87,7 @@ func (tc *TsWorkspace) getConfig(f string) (string, *TsConfig) {
 			dir = ""
 		}
 
-		if tc.hasConfig(dir) {
+		if tc.cm.configFiles[dir] != nil {
 			return dir, tc.GetTsConfigFile(dir)
 		}
 	}

gazelle/js/typescript/tsconfig.go

@@ -89,32 +89,34 @@ func isRelativePath(p string) bool {
 	return strings.HasPrefix(p, "./") || strings.HasPrefix(p, "../")
 }
 
-// Load a tsconfig.json file and return the compilerOptions config
-func parseTsConfigJSONFile(cm *TsConfigMap, root, dir, tsconfig string) (*TsConfig, error) {
-	existing := cm.configs[dir]
+// Load a tsconfig.json file and return the compilerOptions config with
+// recursive protected via a parsed map that is passed in
+func parseTsConfigJSONFile(parsed map[string]*TsConfig, root, dir, tsconfig string) (*TsConfig, error) {
+	existing := parsed[dir]
 	if existing != nil {
 		return existing, nil
 	}
 
 	// Start with invalid to prevent recursing into the same file
-	cm.configs[dir] = &InvalidTsconfig
+	parsed[dir] = &InvalidTsconfig
 
-	content, readErr := os.ReadFile(path.Join(root, dir, tsconfig))
-	if readErr != nil {
-		return nil, readErr
+	content, err := os.ReadFile(path.Join(root, dir, tsconfig))
+	if err != nil {
+		return nil, err
 	}
 
-	config, err := parseTsConfigJSON(cm, root, dir, tsconfig, content)
+	config, err := parseTsConfigJSON(parsed, root, dir, tsconfig, content)
 	if err != nil {
 		return nil, err
 	}
 
-	// Persist the result on success
-	cm.configs[dir] = config
+	// Add to parsed map on success
+	parsed[dir] = config
+
 	return config, nil
 }
 
-func parseTsConfigJSON(cm *TsConfigMap, root, configDir, configName string, tsconfigContent []byte) (*TsConfig, error) {
+func parseTsConfigJSON(parsed map[string]*TsConfig, root, configDir, configName string, tsconfigContent []byte) (*TsConfig, error) {
 	var c tsConfigJSON
 	if err := jsonr.Unmarshal(tsconfigContent, &c); err != nil {
 		return nil, err
@@ -123,7 +125,7 @@ func parseTsConfigJSON(cm *TsConfigMap, root, configDir, configName string, tsco
 	var baseConfig *TsConfig
 	var extends string
 	if c.Extends != "" {
-		base, err := parseTsConfigJSONFile(cm, root, path.Join(configDir, path.Dir(c.Extends)), path.Base(c.Extends))
+		base, err := parseTsConfigJSONFile(parsed, root, path.Join(configDir, path.Dir(c.Extends)), path.Base(c.Extends))
 		if err != nil {
 			BazelLog.Warnf("Failed to load base tsconfig file %s: %v", path.Join(configDir, c.Extends), err)
 		} else if base != nil {

gazelle/js/typescript/tsconfig_test.go

@@ -32,11 +32,7 @@ func assertEqual(t *testing.T, a, b string, msg string) {
 }
 
 func parseTest(t *testing.T, configDir, tsconfigJSON string) *TsConfig {
-	cm := &TsConfigMap{
-		configs: make(map[string]*TsConfig),
-	}
-
-	options, err := parseTsConfigJSON(cm, ".", configDir, "tsconfig.json", []byte(tsconfigJSON))
+	options, err := parseTsConfigJSON(make(map[string]*TsConfig), ".", configDir, "tsconfig.json", []byte(tsconfigJSON))
 	if err != nil {
 		t.Fatalf("failed to parse options: %v\n\n%s", err, tsconfigJSON)
 	}
@@ -98,42 +94,26 @@ func TestIsRelativePath(t *testing.T) {
 
 func TestTsconfigLoad(t *testing.T) {
 	t.Run("parse a tsconfig file extending itself", func(t *testing.T) {
-		cm := &TsConfigMap{
-			configs: make(map[string]*TsConfig),
-		}
-
-		recursive, _ := parseTsConfigJSONFile(cm, ".", "tests", "extends-recursive.json")
+		recursive, _ := parseTsConfigJSONFile(make(map[string]*TsConfig), ".", "tests", "extends-recursive.json")
 
 		assertEqual(t, recursive.Extends, "tests/extends-recursive.json", "should not fail extending itself")
 	})
 
 	t.Run("parse a tsconfig file extending an unknown file", func(t *testing.T) {
-		cm := &TsConfigMap{
-			configs: make(map[string]*TsConfig),
-		}
-
-		recursive, _ := parseTsConfigJSONFile(cm, ".", "tests", "extends-404.json")
+		recursive, _ := parseTsConfigJSONFile(make(map[string]*TsConfig), ".", "tests", "extends-404.json")
 
 		assertEqual(t, recursive.Extends, "tests/does-not-exist.json", "should not fail extending unknown")
 	})
 
 	t.Run("parse a tsconfig file extending a blank string", func(t *testing.T) {
-		cm := &TsConfigMap{
-			configs: make(map[string]*TsConfig),
-		}
-
-		recursive, _ := parseTsConfigJSONFile(cm, ".", "tests", "extends-empty.json")
+		recursive, _ := parseTsConfigJSONFile(make(map[string]*TsConfig), ".", "tests", "extends-empty.json")
 
 		assertEqual(t, recursive.Extends, "", "should not fail extending an empty str")
 	})
 
 	t.Run("parse example tsconfig file with comments, trialing commas", func(t *testing.T) {
 		// See https://github.com/msolo/jsonr/issues/1#event-10736439202
-		cm := &TsConfigMap{
-			configs: make(map[string]*TsConfig),
-		}
-
-		recursive, err := parseTsConfigJSONFile(cm, ".", "tests", "sourcegraph-svelt.json")
+		recursive, err := parseTsConfigJSONFile(make(map[string]*TsConfig), ".", "tests", "sourcegraph-svelt.json")
 		if err != nil {
 			t.Errorf("parseTsConfigJSONFile: %v", err)
 		}

pkg/aspect/outputs/hash.go

@@ -1,19 +1,3 @@
-/*
- * Copyright 2022 Aspect Build Systems, Inc.
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *     http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
 package outputs
 
 import (
@@ -27,8 +11,8 @@ import (
 	"sort"
 	"strings"
 
-	"aspect.build/cli/pkg/bazel"
 	"github.com/alphadose/haxmap"
+	"aspect.build/cli/pkg/bazel"
 	"github.com/rogpeppe/go-internal/dirhash"
 	concurrently "github.com/tejzpr/ordered-concurrently/v3"
 	"github.com/twmb/murmur3"

pkg/plugin/system/bep/BUILD.bazel

@@ -14,8 +14,8 @@ go_library(
         "//pkg/aspecterrors",
         "//pkg/aspectgrpc",
         "//pkg/plugin/system/besproxy",
-        "@com_github_golang_protobuf//ptypes/empty",
         "@go_googleapis//google/devtools/build/v1:build_go_proto",
+        "@io_bazel_rules_go//proto/wkt:empty_go_proto",
         "@org_golang_google_grpc//:go_default_library",
         "@org_golang_google_grpc//credentials/insecure",
         "@org_golang_google_protobuf//types/known/emptypb",