Skip to content

Commit 3e97dac

Browse files
authored
Add locks on concurrent alias following checker accesses under incremental mode (#2051)
1 parent 8fdec88 commit 3e97dac

File tree

14 files changed

+857
-19
lines changed

14 files changed

+857
-19
lines changed

internal/compiler/checkerpool.go

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import (
1414
type CheckerPool interface {
1515
GetChecker(ctx context.Context) (*checker.Checker, func())
1616
GetCheckerForFile(ctx context.Context, file *ast.SourceFile) (*checker.Checker, func())
17+
GetCheckerForFileExclusive(ctx context.Context, file *ast.SourceFile) (*checker.Checker, func())
1718
GetAllCheckers(ctx context.Context) ([]*checker.Checker, func())
1819
Files(checker *checker.Checker) iter.Seq[*ast.SourceFile]
1920
}
@@ -24,6 +25,7 @@ type checkerPool struct {
2425

2526
createCheckersOnce sync.Once
2627
checkers []*checker.Checker
28+
locks []sync.Mutex
2729
fileAssociations map[*ast.SourceFile]*checker.Checker
2830
}
2931

@@ -34,6 +36,7 @@ func newCheckerPool(checkerCount int, program *Program) *checkerPool {
3436
program: program,
3537
checkerCount: checkerCount,
3638
checkers: make([]*checker.Checker, checkerCount),
39+
locks: make([]sync.Mutex, checkerCount),
3740
}
3841

3942
return pool
@@ -45,6 +48,16 @@ func (p *checkerPool) GetCheckerForFile(ctx context.Context, file *ast.SourceFil
4548
return checker, noop
4649
}
4750

51+
func (p *checkerPool) GetCheckerForFileExclusive(ctx context.Context, file *ast.SourceFile) (*checker.Checker, func()) {
52+
c, done := p.GetCheckerForFile(ctx, file)
53+
idx := slices.Index(p.checkers, c)
54+
p.locks[idx].Lock()
55+
return c, sync.OnceFunc(func() {
56+
p.locks[idx].Unlock()
57+
done()
58+
})
59+
}
60+
4861
func (p *checkerPool) GetChecker(ctx context.Context) (*checker.Checker, func()) {
4962
p.createCheckers()
5063
checker := p.checkers[0]

internal/compiler/program.go

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -383,6 +383,12 @@ func (p *Program) GetTypeCheckerForFile(ctx context.Context, file *ast.SourceFil
383383
return p.checkerPool.GetCheckerForFile(ctx, file)
384384
}
385385

386+
// Return a checker for the given file, locked to the current thread to prevent data races from multiple threads
387+
// accessing the same checker. The lock will be released when the `done` function is called.
388+
func (p *Program) GetTypeCheckerForFileExclusive(ctx context.Context, file *ast.SourceFile) (*checker.Checker, func()) {
389+
return p.checkerPool.GetCheckerForFileExclusive(ctx, file)
390+
}
391+
386392
func (p *Program) GetResolvedModule(file ast.HasFileName, moduleReference string, mode core.ResolutionMode) *module.ResolvedModule {
387393
if resolutions, ok := p.resolvedModules[file.Path()]; ok {
388394
if resolved, ok := resolutions[module.ModeAwareCacheKey{Name: moduleReference, Mode: mode}]; ok {
@@ -1273,6 +1279,10 @@ func (p *Program) InstantiationCount() int {
12731279
return count
12741280
}
12751281

1282+
func (p *Program) Program() *Program {
1283+
return p
1284+
}
1285+
12761286
func (p *Program) GetSourceFileMetaData(path tspath.Path) ast.SourceFileMetaData {
12771287
return p.sourceFileMetaDatas[path]
12781288
}
@@ -1437,6 +1447,7 @@ func CombineEmitResults(results []*EmitResult) *EmitResult {
14371447

14381448
type ProgramLike interface {
14391449
Options() *core.CompilerOptions
1450+
GetSourceFile(path string) *ast.SourceFile
14401451
GetSourceFiles() []*ast.SourceFile
14411452
GetConfigFileParsingDiagnostics() []*ast.Diagnostic
14421453
GetSyntacticDiagnostics(ctx context.Context, file *ast.SourceFile) []*ast.Diagnostic
@@ -1446,7 +1457,11 @@ type ProgramLike interface {
14461457
GetGlobalDiagnostics(ctx context.Context) []*ast.Diagnostic
14471458
GetSemanticDiagnostics(ctx context.Context, file *ast.SourceFile) []*ast.Diagnostic
14481459
GetDeclarationDiagnostics(ctx context.Context, file *ast.SourceFile) []*ast.Diagnostic
1460+
GetSuggestionDiagnostics(ctx context.Context, file *ast.SourceFile) []*ast.Diagnostic
14491461
Emit(ctx context.Context, options EmitOptions) *EmitResult
1462+
CommonSourceDirectory() string
1463+
IsSourceFileDefaultLibrary(path tspath.Path) bool
1464+
Program() *Program
14501465
}
14511466

14521467
func HandleNoEmitOnError(ctx context.Context, program ProgramLike, file *ast.SourceFile) *EmitResult {

internal/execute/incremental/affectedfileshandler.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -229,7 +229,7 @@ func (h *affectedFilesHandler) handleDtsMayChangeOfAffectedFile(dtsMayChange dts
229229
break
230230
}
231231
if typeChecker == nil {
232-
typeChecker, done = h.program.program.GetTypeCheckerForFile(h.ctx, affectedFile)
232+
typeChecker, done = h.program.program.GetTypeCheckerForFileExclusive(h.ctx, affectedFile)
233233
}
234234
aliased := checker.SkipAlias(exported, typeChecker)
235235
if aliased == exported {

internal/execute/incremental/program.go

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,12 +84,36 @@ func (p *Program) Options() *core.CompilerOptions {
8484
return p.snapshot.options
8585
}
8686

87+
// CommonSourceDirectory implements compiler.AnyProgram interface.
88+
func (p *Program) CommonSourceDirectory() string {
89+
p.panicIfNoProgram("CommonSourceDirectory")
90+
return p.program.CommonSourceDirectory()
91+
}
92+
93+
// Program implements compiler.AnyProgram interface.
94+
func (p *Program) Program() *compiler.Program {
95+
p.panicIfNoProgram("Program")
96+
return p.program
97+
}
98+
99+
// IsSourceFileDefaultLibrary implements compiler.AnyProgram interface.
100+
func (p *Program) IsSourceFileDefaultLibrary(path tspath.Path) bool {
101+
p.panicIfNoProgram("IsSourceFileDefaultLibrary")
102+
return p.program.IsSourceFileDefaultLibrary(path)
103+
}
104+
87105
// GetSourceFiles implements compiler.AnyProgram interface.
88106
func (p *Program) GetSourceFiles() []*ast.SourceFile {
89107
p.panicIfNoProgram("GetSourceFiles")
90108
return p.program.GetSourceFiles()
91109
}
92110

111+
// GetSourceFile implements compiler.AnyProgram interface.
112+
func (p *Program) GetSourceFile(path string) *ast.SourceFile {
113+
p.panicIfNoProgram("GetSourceFile")
114+
return p.program.GetSourceFile(path)
115+
}
116+
93117
// GetConfigFileParsingDiagnostics implements compiler.AnyProgram interface.
94118
func (p *Program) GetConfigFileParsingDiagnostics() []*ast.Diagnostic {
95119
p.panicIfNoProgram("GetConfigFileParsingDiagnostics")
@@ -172,6 +196,12 @@ func (p *Program) GetDeclarationDiagnostics(ctx context.Context, file *ast.Sourc
172196
return nil
173197
}
174198

199+
// GetSuggestionDiagnostics implements compiler.AnyProgram interface.
200+
func (p *Program) GetSuggestionDiagnostics(ctx context.Context, file *ast.SourceFile) []*ast.Diagnostic {
201+
p.panicIfNoProgram("GetSuggestionDiagnostics")
202+
return p.program.GetSuggestionDiagnostics(ctx, file) // TODO: incremental suggestion diagnostics (only relevant in editor incremental builder?)
203+
}
204+
175205
// GetModeForUsageLocation implements compiler.AnyProgram interface.
176206
func (p *Program) Emit(ctx context.Context, options compiler.EmitOptions) *compiler.EmitResult {
177207
p.panicIfNoProgram("Emit")

internal/execute/incremental/programtosnapshot.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -263,7 +263,7 @@ func getReferencedFiles(program *compiler.Program, file *ast.SourceFile) *collec
263263
// We need to use a set here since the code can contain the same import twice,
264264
// but that will only be one dependency.
265265
// To avoid invernal conversion, the key of the referencedFiles map must be of type Path
266-
checker, done := program.GetTypeCheckerForFile(context.TODO(), file)
266+
checker, done := program.GetTypeCheckerForFileExclusive(context.TODO(), file)
267267
defer done()
268268
for _, importName := range file.Imports() {
269269
addReferencedFilesFromImportLiteral(file, &referencedFiles, checker, importName)

internal/project/checkerpool.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ type CheckerPool struct {
2020
cond *sync.Cond
2121
createCheckersOnce sync.Once
2222
checkers []*checker.Checker
23+
locks []sync.Mutex
2324
inUse map[*checker.Checker]bool
2425
fileAssociations map[*ast.SourceFile]int
2526
requestAssociations map[string]int
@@ -33,6 +34,7 @@ func newCheckerPool(maxCheckers int, program *compiler.Program, log func(msg str
3334
program: program,
3435
maxCheckers: maxCheckers,
3536
checkers: make([]*checker.Checker, maxCheckers),
37+
locks: make([]sync.Mutex, maxCheckers),
3638
inUse: make(map[*checker.Checker]bool),
3739
requestAssociations: make(map[string]int),
3840
log: log,
@@ -75,6 +77,12 @@ func (p *CheckerPool) GetCheckerForFile(ctx context.Context, file *ast.SourceFil
7577
return checker, p.createRelease(requestID, index, checker)
7678
}
7779

80+
// GetCheckerForFileExclusive is the same as GetCheckerForFile but also locks a mutex associated with the checker.
81+
// Call `done` to free the lock.
82+
func (p *CheckerPool) GetCheckerForFileExclusive(ctx context.Context, file *ast.SourceFile) (*checker.Checker, func()) {
83+
panic("unimplemented") // implement if used by LS
84+
}
85+
7886
func (p *CheckerPool) GetChecker(ctx context.Context) (*checker.Checker, func()) {
7987
p.mu.Lock()
8088
defer p.mu.Unlock()

internal/testrunner/compiler_runner.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -528,7 +528,8 @@ func createHarnessTestFile(unit *testUnit, currentDirectory string) *harnessutil
528528

529529
func (c *compilerTest) verifyUnionOrdering(t *testing.T) {
530530
t.Run("union ordering", func(t *testing.T) {
531-
checkers, done := c.result.Program.GetTypeCheckers(t.Context())
531+
p := c.result.Program.Program()
532+
checkers, done := p.GetTypeCheckers(t.Context())
532533
defer done()
533534
for _, c := range checkers {
534535
for union := range c.UnionTypes() {

internal/testutil/harnessutil/harnessutil.go

Lines changed: 37 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import (
2121
"github.com/microsoft/typescript-go/internal/collections"
2222
"github.com/microsoft/typescript-go/internal/compiler"
2323
"github.com/microsoft/typescript-go/internal/core"
24+
"github.com/microsoft/typescript-go/internal/execute/incremental"
2425
"github.com/microsoft/typescript-go/internal/outputpaths"
2526
"github.com/microsoft/typescript-go/internal/parser"
2627
"github.com/microsoft/typescript-go/internal/repo"
@@ -687,13 +688,13 @@ func compileFilesWithHost(
687688
}
688689
emitResult := program.Emit(ctx, compiler.EmitOptions{})
689690

690-
return newCompilationResult(config.CompilerOptions(), program, emitResult, diagnostics, harnessOptions)
691+
return newCompilationResult(host, config.CompilerOptions(), program, emitResult, diagnostics, harnessOptions)
691692
}
692693

693694
type CompilationResult struct {
694695
Diagnostics []*ast.Diagnostic
695696
Result *compiler.EmitResult
696-
Program *compiler.Program
697+
Program compiler.ProgramLike
697698
Options *core.CompilerOptions
698699
HarnessOptions *HarnessOptions
699700
JS collections.OrderedMap[string, *TestFile]
@@ -705,6 +706,7 @@ type CompilationResult struct {
705706
inputs []*TestFile
706707
inputsAndOutputs collections.OrderedMap[string, *CompilationOutput]
707708
Trace string
709+
Host compiler.CompilerHost
708710
}
709711

710712
type CompilationOutput struct {
@@ -715,8 +717,9 @@ type CompilationOutput struct {
715717
}
716718

717719
func newCompilationResult(
720+
host compiler.CompilerHost,
718721
options *core.CompilerOptions,
719-
program *compiler.Program,
722+
program compiler.ProgramLike,
720723
result *compiler.EmitResult,
721724
diagnostics []*ast.Diagnostic,
722725
harnessOptions *HarnessOptions,
@@ -731,9 +734,10 @@ func newCompilationResult(
731734
Program: program,
732735
Options: options,
733736
HarnessOptions: harnessOptions,
737+
Host: host,
734738
}
735739

736-
fs := program.Host().FS().(*OutputRecorderFS)
740+
fs := host.FS().(*OutputRecorderFS)
737741
if fs != nil && program != nil {
738742
// Corsa, unlike Strada, can use multiple threads for emit. As a result, the order of outputs is non-deterministic.
739743
// To make the order deterministic, we sort the outputs by the order of the inputs.
@@ -803,7 +807,7 @@ func compareTestFiles(a *TestFile, b *TestFile) int {
803807
}
804808

805809
func (c *CompilationResult) getOutputPath(path string, ext string) string {
806-
path = tspath.ResolvePath(c.Program.GetCurrentDirectory(), path)
810+
path = tspath.ResolvePath(c.Host.GetCurrentDirectory(), path)
807811
var outDir string
808812
if ext == ".d.ts" || ext == ".d.mts" || ext == ".d.cts" || (strings.HasSuffix(ext, ".ts") && strings.Contains(ext, ".d.")) {
809813
outDir = c.Options.DeclarationDir
@@ -817,17 +821,17 @@ func (c *CompilationResult) getOutputPath(path string, ext string) string {
817821
common := c.Program.CommonSourceDirectory()
818822
if common != "" {
819823
path = tspath.GetRelativePathFromDirectory(common, path, tspath.ComparePathsOptions{
820-
UseCaseSensitiveFileNames: c.Program.UseCaseSensitiveFileNames(),
821-
CurrentDirectory: c.Program.GetCurrentDirectory(),
824+
UseCaseSensitiveFileNames: c.Host.FS().UseCaseSensitiveFileNames(),
825+
CurrentDirectory: c.Host.GetCurrentDirectory(),
822826
})
823-
path = tspath.CombinePaths(tspath.ResolvePath(c.Program.GetCurrentDirectory(), c.Options.OutDir), path)
827+
path = tspath.CombinePaths(tspath.ResolvePath(c.Host.GetCurrentDirectory(), c.Options.OutDir), path)
824828
}
825829
}
826830
return tspath.ChangeExtension(path, ext)
827831
}
828832

829833
func (r *CompilationResult) FS() vfs.FS {
830-
return r.Program.Host().FS()
834+
return r.Host.FS()
831835
}
832836

833837
func (r *CompilationResult) GetNumberOfJSFiles(includeJson bool) int {
@@ -852,7 +856,7 @@ func (c *CompilationResult) Outputs() []*TestFile {
852856
}
853857

854858
func (c *CompilationResult) GetInputsAndOutputsForFile(path string) *CompilationOutput {
855-
return c.inputsAndOutputs.GetOrZero(tspath.ResolvePath(c.Program.GetCurrentDirectory(), path))
859+
return c.inputsAndOutputs.GetOrZero(tspath.ResolvePath(c.Host.GetCurrentDirectory(), path))
856860
}
857861

858862
func (c *CompilationResult) GetInputsForFile(path string) []*TestFile {
@@ -915,7 +919,24 @@ func (c *CompilationResult) GetSourceMapRecord() string {
915919
return sourceMapRecorder.String()
916920
}
917921

918-
func createProgram(host compiler.CompilerHost, config *tsoptions.ParsedCommandLine) *compiler.Program {
922+
type testBuildInfoReader struct {
923+
inner incremental.BuildInfoReader
924+
}
925+
926+
func (t *testBuildInfoReader) ReadBuildInfo(config *tsoptions.ParsedCommandLine) *incremental.BuildInfo {
927+
r := t.inner.ReadBuildInfo(config)
928+
if r == nil {
929+
return nil
930+
}
931+
r.Version = core.Version()
932+
return r
933+
}
934+
935+
func getTestBuildInfoReader(host compiler.CompilerHost) *testBuildInfoReader {
936+
return &testBuildInfoReader{inner: incremental.NewBuildInfoReader(host)}
937+
}
938+
939+
func createProgram(host compiler.CompilerHost, config *tsoptions.ParsedCommandLine) compiler.ProgramLike {
919940
var singleThreaded core.Tristate
920941
if testutil.TestProgramIsSingleThreaded() {
921942
singleThreaded = core.TSTrue
@@ -927,6 +948,11 @@ func createProgram(host compiler.CompilerHost, config *tsoptions.ParsedCommandLi
927948
SingleThreaded: singleThreaded,
928949
}
929950
program := compiler.NewProgram(programOptions)
951+
if config.CompilerOptions().Incremental.IsTrue() {
952+
oldProgram := incremental.ReadBuildInfoProgram(config, getTestBuildInfoReader(host), host)
953+
incrementalProgram := incremental.NewProgram(program, oldProgram, incremental.CreateHost(host), false)
954+
return incrementalProgram
955+
}
930956
return program
931957
}
932958

internal/testutil/tsbaseline/js_emit_baseline.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -201,7 +201,7 @@ func prepareDeclarationCompilationContext(
201201
var sourceFileName string
202202

203203
if len(options.OutDir) != 0 {
204-
sourceFilePath := tspath.GetNormalizedAbsolutePath(sourceFile.FileName(), result.Program.GetCurrentDirectory())
204+
sourceFilePath := tspath.GetNormalizedAbsolutePath(sourceFile.FileName(), result.Host.GetCurrentDirectory())
205205
sourceFilePath = strings.Replace(sourceFilePath, result.Program.CommonSourceDirectory(), "", 1)
206206
sourceFileName = tspath.CombinePaths(options.OutDir, sourceFilePath)
207207
} else {

internal/testutil/tsbaseline/type_symbol_baseline.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ func DoTypeAndSymbolBaseline(
3131
t *testing.T,
3232
baselinePath string,
3333
header string,
34-
program *compiler.Program,
34+
program compiler.ProgramLike,
3535
allFiles []*harnessutil.TestFile,
3636
opts baseline.Options,
3737
skipTypeBaselines bool,
@@ -259,13 +259,13 @@ func iterateBaseline(allFiles []*harnessutil.TestFile, fullWalker *typeWriterWal
259259
}
260260

261261
type typeWriterWalker struct {
262-
program *compiler.Program
262+
program compiler.ProgramLike
263263
hadErrorBaseline bool
264264
currentSourceFile *ast.SourceFile
265265
declarationTextCache map[*ast.Node]string
266266
}
267267

268-
func newTypeWriterWalker(program *compiler.Program, hadErrorBaseline bool) *typeWriterWalker {
268+
func newTypeWriterWalker(program compiler.ProgramLike, hadErrorBaseline bool) *typeWriterWalker {
269269
return &typeWriterWalker{
270270
program: program,
271271
hadErrorBaseline: hadErrorBaseline,
@@ -276,7 +276,7 @@ func newTypeWriterWalker(program *compiler.Program, hadErrorBaseline bool) *type
276276
func (walker *typeWriterWalker) getTypeCheckerForCurrentFile() (*checker.Checker, func()) {
277277
// If we don't use the right checker for the file, its contents won't be up to date
278278
// since the types/symbols baselines appear to depend on files having been checked.
279-
return walker.program.GetTypeCheckerForFile(context.Background(), walker.currentSourceFile)
279+
return walker.program.Program().GetTypeCheckerForFile(context.Background(), walker.currentSourceFile)
280280
}
281281

282282
type typeWriterResult struct {

0 commit comments

Comments
 (0)