Skip to content

Commit 45e8f87

Browse files
andrewbranchandrewbranch
authored
Decouple snapshot ownership from project.Session so api.Session only uses one in LSP mode (#64163)
1 parent b6634d8 commit 45e8f87

18 files changed

Lines changed: 731 additions & 394 deletions

‎tsc/internal/api/server.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ func (s *StdioServer) Run(ctx context.Context) error {
8383
fs = callbackFS
8484
}
8585

86-
projectSession := project.NewSession(&project.SessionInit{
86+
sessionInit := &project.SessionInit{
8787
BackgroundCtx: ctx,
8888
Logger: nil, // TODO: Add logging support
8989
FS: fs,
@@ -95,9 +95,9 @@ func (s *StdioServer) Run(ctx context.Context) error {
9595
RunExternalCode: s.options.RunExternalCode,
9696
},
9797
Spawner: s.options.ContentMapperSpawner,
98-
})
98+
}
9999

100-
session := NewSession(projectSession, &SessionOptions{
100+
session := NewStandaloneSession(sessionInit, &SessionOptions{
101101
UseBinaryResponses: !s.options.Async, // Only msgpack uses binary responses
102102
})
103103
defer session.Close()

‎tsc/internal/api/session.go‎

Lines changed: 145 additions & 63 deletions
Large diffs are not rendered by default.

‎tsc/internal/api/session_apistate_test.go‎

Lines changed: 45 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,51 @@ import (
55
"testing"
66

77
"github.com/microsoft/TypeScript/tsc/internal/bundled"
8+
"github.com/microsoft/TypeScript/tsc/internal/core"
89
"github.com/microsoft/TypeScript/tsc/internal/lsp/lsproto"
910
"github.com/microsoft/TypeScript/tsc/internal/testutil/projecttestutil"
1011
"github.com/microsoft/TypeScript/tsc/internal/tspath"
1112
"gotest.tools/v3/assert"
1213
)
1314

15+
func TestStandaloneSessionUsesSnapshotHostWithoutProjectSession(t *testing.T) {
16+
t.Parallel()
17+
if !bundled.Embedded {
18+
t.Skip("bundled files are not embedded")
19+
}
20+
21+
const configFileName = "/home/projects/p/tsconfig.json"
22+
init, _ := projecttestutil.GetSessionInitOptions(map[string]any{
23+
configFileName: `{ "compilerOptions": { "strict": true } }`,
24+
"/home/projects/p/src/index.ts": `export const x = 1;`,
25+
}, nil, &projecttestutil.TypingsInstallerOptions{})
26+
session := NewStandaloneSession(init, nil)
27+
defer session.Close()
28+
29+
firstResponse, err := session.handleUpdateSnapshot(context.Background(), &UpdateSnapshotParams{
30+
OpenFiles: []DocumentIdentifier{{FileName: "/home/projects/p/src/index.ts"}},
31+
})
32+
assert.NilError(t, err)
33+
assert.Equal(t, firstResponse.Snapshot, SnapshotID(1))
34+
assert.Equal(t, len(firstResponse.Projects), 1)
35+
36+
response, err := session.handleUpdateSnapshot(context.Background(), &UpdateSnapshotParams{
37+
OpenProjects: []DocumentIdentifier{{FileName: configFileName}},
38+
})
39+
assert.NilError(t, err)
40+
assert.Equal(t, response.Snapshot, SnapshotID(2))
41+
42+
programResponse, err := session.handleCreateProgram(context.Background(), &CreateProgramParams{
43+
RootFiles: []DocumentIdentifier{{FileName: "/home/projects/p/src/index.ts"}},
44+
CreateProgramOptions: CreateProgramOptions{
45+
CompilerOptions: core.CompilerOptions{NoLib: core.TSTrue},
46+
},
47+
})
48+
assert.NilError(t, err)
49+
assert.Assert(t, programResponse.Project != nil)
50+
assert.Equal(t, programResponse.Snapshot, SnapshotID(4))
51+
}
52+
1453
// TestSessionTracksAndReleasesAPIRefs verifies that an API session holds at most
1554
// one ref per opened project/file (opens are idempotent) and releases exactly
1655
// those refs when the session is closed, so it never leaks or over-releases refs
@@ -30,7 +69,8 @@ func TestSessionTracksAndReleasesAPIRefs(t *testing.T) {
3069
}
3170
projectSession, _ := projecttestutil.Setup(files)
3271
defer projectSession.Close()
33-
session := NewSession(projectSession, nil)
72+
session := NewLSPSession(projectSession, nil)
73+
assert.Assert(t, session.compatibilitySnapshot == nil)
3474

3575
_, err := session.handleUpdateSnapshot(context.Background(), &UpdateSnapshotParams{
3676
OpenProjects: []DocumentIdentifier{{FileName: configFileName}},
@@ -63,7 +103,7 @@ func TestSessionTracksAndReleasesAPIRefs(t *testing.T) {
63103
}
64104
projectSession, _ := projecttestutil.Setup(files)
65105
defer projectSession.Close()
66-
session := NewSession(projectSession, nil)
106+
session := NewLSPSession(projectSession, nil)
67107
defer session.Close()
68108

69109
_, err := session.handleUpdateSnapshot(context.Background(), &UpdateSnapshotParams{
@@ -98,7 +138,7 @@ func TestSessionTracksAndReleasesAPIRefs(t *testing.T) {
98138
}
99139
projectSession, _ := projecttestutil.Setup(files)
100140
defer projectSession.Close()
101-
session := NewSession(projectSession, nil)
141+
session := NewLSPSession(projectSession, nil)
102142

103143
_, err := session.handleUpdateSnapshot(context.Background(), &UpdateSnapshotParams{
104144
OpenFiles: []DocumentIdentifier{{FileName: fileName}},
@@ -151,7 +191,7 @@ func TestSessionTracksAndReleasesAPIRefs(t *testing.T) {
151191
}
152192
projectSession, _ := projecttestutil.Setup(files)
153193
defer projectSession.Close()
154-
session := NewSession(projectSession, nil)
194+
session := NewLSPSession(projectSession, nil)
155195
defer session.Close()
156196

157197
// Open via a relative path; it should be tracked under the absolute path
@@ -230,7 +270,7 @@ func TestUpdateSnapshotResponseSkipsUnloadedAncestorProject(t *testing.T) {
230270
assert.Assert(t, ancestorProject != nil)
231271
assert.Assert(t, ancestorProject.CommandLine == nil)
232272

233-
session := NewSession(projectSession, nil)
273+
session := NewLSPSession(projectSession, nil)
234274
defer session.Close()
235275

236276
response, err := session.handleUpdateSnapshot(context.Background(), &UpdateSnapshotParams{

‎tsc/internal/api/session_completion_test.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ func TestCompletionSymbolTypeIsResolvable(t *testing.T) {
3838
}
3939
projectSession, _ := projecttestutil.Setup(files)
4040
defer projectSession.Close()
41-
session := NewSession(projectSession, nil)
41+
session := NewLSPSession(projectSession, nil)
4242
defer session.Close()
4343

4444
snapshotResp, err := session.handleUpdateSnapshot(t.Context(), &UpdateSnapshotParams{
@@ -110,7 +110,7 @@ func TestCompletionOnInferredProject(t *testing.T) {
110110
}
111111
projectSession, _ := projecttestutil.Setup(files)
112112
defer projectSession.Close()
113-
session := NewSession(projectSession, nil)
113+
session := NewLSPSession(projectSession, nil)
114114
defer session.Close()
115115

116116
snapshotResp, err := session.handleUpdateSnapshot(t.Context(), &UpdateSnapshotParams{
@@ -156,7 +156,7 @@ func TestCompletionRetriesWithAutoImports(t *testing.T) {
156156
IncludeCompletionsForImportStatements: core.TSTrue,
157157
})
158158

159-
session := NewSession(projectSession, nil)
159+
session := NewLSPSession(projectSession, nil)
160160
defer session.Close()
161161

162162
snapshotResp, err := session.handleUpdateSnapshot(t.Context(), &UpdateSnapshotParams{

‎tsc/internal/api/session_createprogram_test.go‎

Lines changed: 26 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ func TestCreateProgram(t *testing.T) {
2121
})
2222
defer projectSession.Close()
2323

24-
session := NewSession(projectSession, nil)
24+
session := NewLSPSession(projectSession, nil)
2525
defer session.Close()
2626
ctx := context.Background()
2727

@@ -46,6 +46,7 @@ func TestCreateProgram(t *testing.T) {
4646
})
4747
assert.NilError(t, err)
4848
assert.Assert(t, response.Snapshot != baseResponse.Snapshot)
49+
assert.Equal(t, response.Snapshot, SnapshotID(4))
4950
assert.Equal(t, session.latestSnapshot, baseResponse.Snapshot)
5051
assert.Assert(t, response.Project != nil)
5152
assert.DeepEqual(t, response.Project.RootFiles, []string{fileName})
@@ -118,7 +119,7 @@ func TestCreateProgramWithNoRootFiles(t *testing.T) {
118119
projectSession, _ := projecttestutil.Setup(map[string]any{})
119120
defer projectSession.Close()
120121

121-
session := NewSession(projectSession, nil)
122+
session := NewLSPSession(projectSession, nil)
122123
defer session.Close()
123124

124125
response, err := session.handleCreateProgram(context.Background(), &CreateProgramParams{
@@ -138,6 +139,24 @@ func TestCreateProgramWithNoRootFiles(t *testing.T) {
138139
assert.Equal(t, len(project.Program.GetSourceFiles()), 0)
139140
}
140141

142+
func TestCreateProgramFileChangesRequireOldProgram(t *testing.T) {
143+
t.Parallel()
144+
145+
projectSession, _ := projecttestutil.Setup(map[string]any{})
146+
defer projectSession.Close()
147+
148+
session := NewLSPSession(projectSession, nil)
149+
defer session.Close()
150+
151+
_, err := session.handleCreateProgram(context.Background(), &CreateProgramParams{
152+
CreateProgramOptions: CreateProgramOptions{
153+
CompilerOptions: core.CompilerOptions{NoLib: core.TSTrue},
154+
},
155+
FileChanges: &APIFileChanges{InvalidateAll: true},
156+
})
157+
assert.ErrorContains(t, err, "fileChanges requires an oldProgram")
158+
}
159+
141160
func TestCreateProgramRemovesAllRootFiles(t *testing.T) {
142161
t.Parallel()
143162

@@ -147,7 +166,7 @@ func TestCreateProgramRemovesAllRootFiles(t *testing.T) {
147166
})
148167
defer projectSession.Close()
149168

150-
session := NewSession(projectSession, nil)
169+
session := NewLSPSession(projectSession, nil)
151170
defer session.Close()
152171
ctx := context.Background()
153172

@@ -196,7 +215,7 @@ func TestCreateProgramPreservesRootFileOrder(t *testing.T) {
196215
})
197216
defer projectSession.Close()
198217

199-
session := NewSession(projectSession, nil)
218+
session := NewLSPSession(projectSession, nil)
200219
defer session.Close()
201220
ctx := context.Background()
202221

@@ -236,7 +255,7 @@ func TestCreateProgramReusesProgram(t *testing.T) {
236255
})
237256
defer projectSession.Close()
238257

239-
session := NewSession(projectSession, nil)
258+
session := NewLSPSession(projectSession, nil)
240259
defer session.Close()
241260
ctx := context.Background()
242261

@@ -315,7 +334,7 @@ func TestCreateProgramProjectReferencesAndReuse(t *testing.T) {
315334
})
316335
defer projectSession.Close()
317336

318-
session := NewSession(projectSession, nil)
337+
session := NewLSPSession(projectSession, nil)
319338
defer session.Close()
320339
ctx := context.Background()
321340
libReference := &core.ProjectReference{Path: libConfigName, OriginalPath: libConfigName}
@@ -391,7 +410,7 @@ func TestCreateProgramFromConfiguredProgramDoesNotRetainOtherProjects(t *testing
391410
})
392411
defer projectSession.Close()
393412

394-
session := NewSession(projectSession, nil)
413+
session := NewLSPSession(projectSession, nil)
395414
defer session.Close()
396415
ctx := context.Background()
397416

‎tsc/internal/api/session_temporary_test.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ func TestUpdateTemporarySnapshot(t *testing.T) {
3030
}
3131
projectSession, _ := projecttestutil.Setup(files)
3232
defer projectSession.Close()
33-
session := NewSession(projectSession, nil)
33+
session := NewLSPSession(projectSession, nil)
3434
defer session.Close()
3535

3636
ctx := context.Background()
@@ -114,7 +114,7 @@ func TestUpdateTemporarySnapshotAddsUnopenedFile(t *testing.T) {
114114
}
115115
projectSession, _ := projecttestutil.Setup(files)
116116
defer projectSession.Close()
117-
session := NewSession(projectSession, nil)
117+
session := NewLSPSession(projectSession, nil)
118118
defer session.Close()
119119

120120
ctx := context.Background()
@@ -147,7 +147,7 @@ func TestUpdateTemporarySnapshotRejectsUnsupportedExtension(t *testing.T) {
147147

148148
projectSession, _ := projecttestutil.Setup(map[string]any{})
149149
defer projectSession.Close()
150-
session := NewSession(projectSession, nil)
150+
session := NewLSPSession(projectSession, nil)
151151
defer session.Close()
152152

153153
ctx := context.Background()
@@ -176,7 +176,7 @@ func TestUpdateTemporarySnapshotUsesClientSnapshotAsBase(t *testing.T) {
176176
}
177177
projectSession, _ := projecttestutil.Setup(files)
178178
defer projectSession.Close()
179-
session := NewSession(projectSession, nil)
179+
session := NewLSPSession(projectSession, nil)
180180
defer session.Close()
181181

182182
ctx := context.Background()

‎tsc/internal/ipc/conn_async.go‎

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ type AsyncConn struct {
3232
pendingMu sync.Mutex
3333
terminal error
3434
writeMu sync.Mutex
35+
handlers sync.WaitGroup
3536
}
3637

3738
// NewAsyncConn creates a new async connection with the given transport and handler.
@@ -64,7 +65,12 @@ func (c *AsyncConn) SetCollectTiming(enabled bool) {
6465
// Run starts processing messages on the connection.
6566
// It blocks until the context is cancelled or an error occurs.
6667
func (c *AsyncConn) Run(ctx context.Context) (err error) {
67-
defer func() { c.closePendingCalls(err) }()
68+
handlerCtx, cancelHandlers := context.WithCancel(ctx)
69+
defer func() {
70+
c.closePendingCalls(err)
71+
cancelHandlers()
72+
c.handlers.Wait()
73+
}()
6874
for {
6975
if ctx.Err() != nil {
7076
return ctx.Err()
@@ -81,9 +87,13 @@ func (c *AsyncConn) Run(ctx context.Context) (err error) {
8187
if msg.IsResponse() {
8288
c.handleResponse(msg)
8389
} else if msg.IsRequest() {
84-
go c.handleRequest(ctx, msg)
90+
c.handlers.Go(func() {
91+
c.handleRequest(handlerCtx, msg)
92+
})
8593
} else if msg.IsNotification() {
86-
go c.handleNotification(ctx, msg)
94+
c.handlers.Go(func() {
95+
c.handleNotification(handlerCtx, msg)
96+
})
8797
}
8898
}
8999
}

0 commit comments

Comments
 (0)