Fix auto-import bucket state race and CreateProgram/UpdateProgram race (#3590)

Author: andrewbranch

internal/compiler/program.go

@@ -273,9 +273,14 @@ func NewProgram(opts ProgramOptions) *Program {
 
 // Return an updated program for which it is known that only the file with the given path has changed.
 // In addition to a new program, return a boolean indicating whether the data of the old program was reused.
-func (p *Program) UpdateProgram(changedFilePath tspath.Path, newHost CompilerHost) (*Program, bool) {
+// createCheckerPool, if non-nil, overrides the CreateCheckerPool stored in the old program's options,
+// ensuring each caller uses a fresh closure and avoiding data races on captured variables.
+func (p *Program) UpdateProgram(changedFilePath tspath.Path, newHost CompilerHost, createCheckerPool func(*Program) CheckerPool) (*Program, bool) {
 	newOpts := p.opts
 	newOpts.Host = newHost
+	if createCheckerPool != nil {
+		newOpts.CreateCheckerPool = createCheckerPool
+	}
 
 	oldFile := p.filesByPath[changedFilePath]
 	newFile := newHost.GetSourceFile(oldFile.ParseOptions())

internal/fourslash/tests/autoImportSymlinkedMonorepoSourceUpdate_test.go

@@ -0,0 +1,86 @@
+package fourslash_test
+
+import (
+	"testing"
+
+	"github.com/microsoft/typescript-go/internal/fourslash"
+	"github.com/microsoft/typescript-go/internal/testutil"
+)
+
+const TestAutoImportSymlinkedMonorepoSourceUpdateScenario = `
+// @Filename: /home/src/workspaces/project/tsconfig.base.json
+{
+  "compilerOptions": {
+    "module": "nodenext",
+    "moduleResolution": "nodenext",
+	"composite": true
+  }
+}
+
+// @Filename: /home/src/workspaces/project/packages/foo/package.json
+{
+  "name": "@packages/foo",
+  "type": "module",
+  "exports": {
+    ".": {
+	  "types": "./src/index.ts",
+	  "default": "./dist/index.js"
+	}
+  }
+}
+
+// @Filename: /home/src/workspaces/project/packages/foo/tsconfig.json
+{ "extends": "../../tsconfig.base.json" }
+
+// @Filename: /home/src/workspaces/project/packages/foo/src/index.ts
+/*fooEdit*/
+
+// @Filename: /home/src/workspaces/project/packages/bar/package.json
+{
+  "name": "@packages/bar",
+  "type": "module",
+  "exports": {
+    ".": {
+	  "types": "./src/index.ts",
+	  "default": "./dist/index.js"
+	}
+  },
+  "dependencies": {
+    "@packages/foo": "*"
+  }
+}
+
+// @Filename: /home/src/workspaces/project/packages/bar/tsconfig.json
+{ "extends": "../../tsconfig.base.json" }
+
+// @Filename: /home/src/workspaces/project/packages/bar/src/index.ts
+/*fooCompletion*/
+
+// @Filename: /home/src/workspaces/project/package.json
+{ "workspaces": ["packages/*"], "type": "module" }
+
+// @link: /home/src/workspaces/project/packages/bar -> /home/src/workspaces/project/node_modules/@packages/bar
+// @link: /home/src/workspaces/project/packages/foo -> /home/src/workspaces/project/node_modules/@packages/foo
+`
+
+func TestAutoImportSymlinkedMonorepoSourceUpdate(t *testing.T) {
+	t.Parallel()
+
+	defer testutil.RecoverAndFail(t, "Panic on fourslash test")
+
+	f, done := fourslash.NewFourslash(t, nil /*capabilities*/, TestAutoImportSymlinkedMonorepoSourceUpdateScenario)
+
+	defer done()
+
+	// Force auto import to build the cache (no exports yet).
+	f.GoToMarker(t, "fooCompletion")
+	f.BaselineAutoImportsCompletions(t, []string{"fooCompletion"})
+
+	// Add a new export to the symlinked source package.
+	f.GoToMarker(t, "fooEdit")
+	f.Insert(t, "\nexport function foo() {}")
+
+	// The new export should appear via granular cache update.
+	f.GoToMarker(t, "fooCompletion")
+	f.BaselineAutoImportsCompletions(t, []string{"fooCompletion"})
+}

internal/ls/autoimport/registry.go

@@ -57,13 +57,20 @@ type BucketState struct {
 	// the bucket was built. If changed, the bucket should be rebuilt.
 	fileExcludePatterns []string
 	// dirtyPackages is the set of package names that need to be re-indexed.
-	// This is used for granular updates: when a file in a project reference package
+	// This is used for granular updates: when a file in a local workspace package
 	// changes, only that package needs to be re-extracted rather than rebuilding
-	// the entire node_modules bucket. If nil, no granular updates are pending.
+	// the entire node_modules bucket.
+	// If nil, no granular updates are pending.
 	// If set but multipleFilesDirty is true, the entire bucket needs to be rebuilt.
 	dirtyPackages *collections.Set[string]
 }
 
+func (b BucketState) Clone() BucketState {
+	b.fileExcludePatterns = slices.Clone(b.fileExcludePatterns)
+	b.dirtyPackages = b.dirtyPackages.Clone()
+	return b
+}
+
 func (b BucketState) Dirty() bool {
 	return b.multipleFilesDirty || b.dirtyFile != "" || b.newProgramStructure > 0 || b.dirtyPackages.Len() > 0
 }
@@ -98,31 +105,50 @@ type RegistryBucket struct {
 
 	// Paths maps file paths to package names. For project buckets, the package name
 	// is always empty string. For node_modules buckets, this enables reverse lookup
-	// from path to package for granular updates. Only paths eligible for granular
-	// update (project reference packages) have entries here.
+	// from path to package for granular updates. Only paths for local workspace
+	// packages (symlinked and within the workspace root) have entries here, since
+	// their realpaths are outside node_modules and need reverse lookup for dirty
+	// detection.
+	//
+	// Paths is considered immutable after the bucket is finalized.
+	// It should be fully replaced rather than mutated while changing a bucket.
 	Paths map[tspath.Path]string
 	// PackageFiles maps package names to their file paths and file names.
 	// All package directory names in node_modules are keys; indexed packages have
 	// non-nil maps with path→fileName entries, unindexed packages have nil maps.
 	// This enables efficient removal of a package's files during granular updates
 	// without iterating through all entries. Only defined for node_modules buckets.
+	//
+	// PackageFiles is considered immutable after the bucket is finalized.
+	// It should be fully replaced rather than mutated while changing a bucket.
 	PackageFiles map[string]map[tspath.Path]string
 	// ResolvedPackageNames is only defined for project buckets. It is the set of
 	// package names that were resolved from imports in the project's program files.
 	// This is passed to node_modules buckets so they include packages that are
 	// directly imported even if not listed in package.json dependencies.
+	//
+	// ResolvedPackageNames is considered immutable after the bucket is finalized.
+	// It should be fully replaced rather than mutated while changing a bucket.
 	ResolvedPackageNames *collections.Set[string]
 	// DependencyNames is only defined for node_modules buckets. It is the set of
 	// package names that will be included in the bucket if present in the directory,
 	// computed from package.json dependencies plus resolved package names from
 	// active programs. If nil, all packages are included because at least one open
 	// file has access to this node_modules directory without being filtered by a
 	// package.json.
+	//
+	// DependencyNames is considered immutable after the bucket is finalized.
+	// It should be fully replaced rather than mutated while changing a bucket.
 	DependencyNames *collections.Set[string]
 	// AmbientModuleNames is only defined for node_modules buckets. It is the set of
 	// ambient module names found while extracting exports in the bucket.
+	//
+	// AmbientModuleNames is considered immutable after the bucket is finalized.
+	// It should be fully replaced rather than mutated while changing a bucket.
 	AmbientModuleNames map[string][]string
-	Index              *Index[*Export]
+	// Index is considered immutable after the bucket is finalized.
+	// It should be cloned and replaced rather than mutated while changing a bucket.
+	Index *Index[*Export]
 }
 
 func newRegistryBucket() *RegistryBucket {
@@ -136,7 +162,7 @@ func newRegistryBucket() *RegistryBucket {
 
 func (b *RegistryBucket) Clone() *RegistryBucket {
 	return &RegistryBucket{
-		state:                b.state,
+		state:                b.state.Clone(),
 		Paths:                b.Paths,
 		PackageFiles:         b.PackageFiles,
 		ResolvedPackageNames: b.ResolvedPackageNames,
@@ -615,8 +641,8 @@ func (b *registryBuilder) markBucketsDirty(change RegistryChange, logger *loggin
 						}
 					}
 				} else {
-					// Check if this path (possibly a realpath of a symlinked package) is in any bucket's Paths.
-					// This handles symlinked packages where the realpath doesn't contain /node_modules/.
+					// Check if this path (possibly a realpath of a workspace package) is in any bucket's Paths.
+					// This handles local workspace packages where the realpath doesn't contain /node_modules/.
 					for bucketDirPath := range cleanNodeModulesBuckets {
 						entry := core.FirstResult(b.nodeModules.Get(bucketDirPath))
 						if packageName, ok := entry.Value().Paths[path]; ok {
@@ -1184,6 +1210,7 @@ type discoveredPackage struct {
 	typesPackageJson *packagejson.InfoCacheEntry
 	typesRealpath    string
 	dirPath          tspath.Path // bucket directory path (used as extraction context)
+	isLocal          bool        // true if realpath is within the workspace root
 }
 
 // perPackageExtractionResult holds the extraction output for one physical package.
@@ -1197,7 +1224,7 @@ type perPackageExtractionResult struct {
 	statsExports                     int
 	statsUsedChecker                 int
 	skippedEntrypoints               int
-	isProjectReference               bool
+	isSymlinked                      bool
 	failedAmbientModuleLookupSources map[tspath.Path]*failedAmbientModuleLookupSource
 	failedAmbientModuleLookupTargets *collections.Set[string]
 }
@@ -1208,7 +1235,7 @@ type packageExtractionResult struct {
 	packageFiles                             map[string]map[tspath.Path]string
 	ambientModuleNames                       map[string][]string
 	entrypoints                              [][]*module.ResolvedEntrypoint
-	projectReferencePackages                 *collections.Set[string]
+	workspacePackages                        *collections.Set[string]
 	possibleFailedAmbientModuleLookupSources *collections.SyncMap[tspath.Path, *failedAmbientModuleLookupSource]
 	possibleFailedAmbientModuleLookupTargets *collections.SyncSet[string]
 	stats                                    extractorStats
@@ -1241,13 +1268,21 @@ func (b *registryBuilder) discoverBucketPackages(
 		if typesPackageJson != nil {
 			typesRealpath = b.host.FS().Realpath(typesPackageJson.PackageDirectory)
 		}
+		isLocal := realpath != "" &&
+			!strings.Contains(realpath, "/node_modules/") &&
+			tspath.ContainsPath(
+				b.host.GetCurrentDirectory(),
+				realpath,
+				tspath.ComparePathsOptions{UseCaseSensitiveFileNames: b.host.FS().UseCaseSensitiveFileNames()},
+			)
 		result = append(result, &discoveredPackage{
 			packageName:      packageName,
 			packageJson:      packageJson,
 			realpath:         realpath,
 			typesPackageJson: typesPackageJson,
 			typesRealpath:    typesRealpath,
 			dirPath:          dirPath,
+			isLocal:          isLocal,
 		})
 	}
 	return result
@@ -1309,7 +1344,6 @@ func (b *registryBuilder) extractPackage(
 			fileName = toSymlink(inputFileName)
 			realpathFileName = inputFileName
 			realpathPath = b.base.toPath(realpathFileName)
-			result.isProjectReference = true
 		}
 
 		if !seenFiles.AddIfAbsent(realpathPath) {
@@ -1318,6 +1352,7 @@ func (b *registryBuilder) extractPackage(
 		if fileName != realpathFileName {
 			symlinkPath := b.base.toPath(fileName)
 			symlinks[realpathPath] = pathAndFileName{path: symlinkPath, fileName: fileName}
+			result.isSymlinked = true
 		}
 		wg.Go(func() {
 			file := b.host.GetSourceFile(realpathFileName, realpathPath)
@@ -1377,7 +1412,7 @@ func installExtractions(
 		exports:                                  make(map[tspath.Path][]*Export),
 		packageFiles:                             make(map[string]map[tspath.Path]string),
 		ambientModuleNames:                       make(map[string][]string),
-		projectReferencePackages:                 &collections.Set[string]{},
+		workspacePackages:                        &collections.Set[string]{},
 		possibleFailedAmbientModuleLookupSources: &collections.SyncMap[tspath.Path, *failedAmbientModuleLookupSource]{},
 		possibleFailedAmbientModuleLookupTargets: &collections.SyncSet[string]{},
 	}
@@ -1407,8 +1442,8 @@ func installExtractions(
 		for target := range extraction.failedAmbientModuleLookupTargets.Keys() {
 			result.possibleFailedAmbientModuleLookupTargets.Add(target)
 		}
-		if extraction.isProjectReference {
-			result.projectReferencePackages.Add(pkg.packageName)
+		if extraction.isSymlinked && pkg.isLocal {
+			result.workspacePackages.Add(pkg.packageName)
 		}
 		result.stats.exports.Add(int32(extraction.statsExports))
 		result.stats.usedChecker.Add(int32(extraction.statsUsedChecker))
@@ -1444,9 +1479,9 @@ func (b *registryBuilder) buildNodeModulesBucket(
 	}
 
 	// Build Paths as reverse mapping from path to package name.
-	// Only include paths for packages that are project references (eligible for granular updates).
+	// Only include paths for local workspace packages (eligible for granular updates).
 	paths := make(map[tspath.Path]string)
-	for pkgName := range extraction.projectReferencePackages.Keys() {
+	for pkgName := range extraction.workspacePackages.Keys() {
 		if files, ok := extraction.packageFiles[pkgName]; ok {
 			for path := range files {
 				paths[path] = pkgName
@@ -1545,8 +1580,8 @@ func (b *registryBuilder) updateNodeModulesBucket(
 		}
 		newPaths[path] = pkgName
 	}
-	// Add paths for newly extracted project reference packages
-	for pkgName := range extraction.projectReferencePackages.Keys() {
+	// Add paths for newly extracted workspace packages
+	for pkgName := range extraction.workspacePackages.Keys() {
 		if files, ok := extraction.packageFiles[pkgName]; ok {
 			for path := range files {
 				newPaths[path] = pkgName