Skip to content

Commit f4298f1

Browse files
sheetalkamatmhegazy
authored andcommitted
[release-2.4] Fixes the memory leak because of project and its corresponding script info even after project is removed (#16538)
* Dont create script snapshots for files that arent source files * Cleanup script infos that are not part of any project when the project is closed or inferred projects are refreshed Also dispose some pointers so that the closures get disposed with project and script infos * Move the cleanup of script infos to next file open This helps in reusing script infos even if the project is closed but next open recreates the same project * Add comment for deletion of orphan script infos in file open
1 parent 8101dc8 commit f4298f1

File tree

5 files changed

+87
-32
lines changed

5 files changed

+87
-32
lines changed

src/harness/unittests/tsserverProjectSystem.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2204,7 +2204,8 @@ namespace ts.projectSystem {
22042204
projectService.closeClientFile(f1.path);
22052205
projectService.checkNumberOfProjects({});
22062206

2207-
for (const f of [f2, f3]) {
2207+
for (const f of [f1, f2, f3]) {
2208+
// There shouldnt be any script info as we closed the file that resulted in creation of it
22082209
const scriptInfo = projectService.getScriptInfoForNormalizedPath(server.toNormalizedPath(f.path));
22092210
assert.equal(scriptInfo.containingProjects.length, 0, `expect 0 containing projects for '${f.path}'`);
22102211
}

src/server/editorServices.ts

Lines changed: 49 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -565,10 +565,17 @@ namespace ts.server {
565565
}
566566
else {
567567
if (info && (!info.isScriptOpen())) {
568-
// file has been changed which might affect the set of referenced files in projects that include
569-
// this file and set of inferred projects
570-
info.reloadFromFile();
571-
this.updateProjectGraphs(info.containingProjects);
568+
if (info.containingProjects.length === 0) {
569+
// Orphan script info, remove it as we can always reload it on next open
570+
info.stopWatcher();
571+
this.filenameToScriptInfo.remove(info.path);
572+
}
573+
else {
574+
// file has been changed which might affect the set of referenced files in projects that include
575+
// this file and set of inferred projects
576+
info.reloadFromFile();
577+
this.updateProjectGraphs(info.containingProjects);
578+
}
572579
}
573580
}
574581
}
@@ -829,10 +836,29 @@ namespace ts.server {
829836
this.assignScriptInfoToInferredProjectIfNecessary(f, /*addToListOfOpenFiles*/ false);
830837
}
831838
}
839+
840+
// Cleanup script infos that arent part of any project is postponed to
841+
// next file open so that if file from same project is opened we wont end up creating same script infos
832842
}
833-
if (info.containingProjects.length === 0) {
834-
// if there are not projects that include this script info - delete it
835-
this.filenameToScriptInfo.remove(info.path);
843+
844+
// If the current info is being just closed - add the watcher file to track changes
845+
// But if file was deleted, handle that part
846+
if (this.host.fileExists(info.fileName)) {
847+
this.watchClosedScriptInfo(info);
848+
}
849+
else {
850+
this.handleDeletedFile(info);
851+
}
852+
}
853+
854+
private deleteOrphanScriptInfoNotInAnyProject() {
855+
for (const path of this.filenameToScriptInfo.getKeys()) {
856+
const info = this.filenameToScriptInfo.get(path);
857+
if (!info.isScriptOpen() && info.containingProjects.length === 0) {
858+
// if there are not projects that include this script info - delete it
859+
info.stopWatcher();
860+
this.filenameToScriptInfo.remove(info.path);
861+
}
836862
}
837863
}
838864

@@ -1303,6 +1329,14 @@ namespace ts.server {
13031329
return this.getScriptInfoForNormalizedPath(toNormalizedPath(uncheckedFileName));
13041330
}
13051331

1332+
watchClosedScriptInfo(info: ScriptInfo) {
1333+
// do not watch files with mixed content - server doesn't know how to interpret it
1334+
if (!info.hasMixedContent) {
1335+
const { fileName } = info;
1336+
info.setWatcher(this.host.watchFile(fileName, _ => this.onSourceFileChanged(fileName)));
1337+
}
1338+
}
1339+
13061340
getOrCreateScriptInfoForNormalizedPath(fileName: NormalizedPath, openedByClient: boolean, fileContent?: string, scriptKind?: ScriptKind, hasMixedContent?: boolean) {
13071341
let info = this.getScriptInfoForNormalizedPath(fileName);
13081342
if (!info) {
@@ -1318,15 +1352,13 @@ namespace ts.server {
13181352
}
13191353
}
13201354
else {
1321-
// do not watch files with mixed content - server doesn't know how to interpret it
1322-
if (!hasMixedContent) {
1323-
info.setWatcher(this.host.watchFile(fileName, _ => this.onSourceFileChanged(fileName)));
1324-
}
1355+
this.watchClosedScriptInfo(info);
13251356
}
13261357
}
13271358
}
13281359
if (info) {
13291360
if (openedByClient && !info.isScriptOpen()) {
1361+
info.stopWatcher();
13301362
info.open(fileContent);
13311363
if (hasMixedContent) {
13321364
info.registerFileUpdate();
@@ -1421,6 +1453,7 @@ namespace ts.server {
14211453
for (const p of this.inferredProjects) {
14221454
p.updateGraph();
14231455
}
1456+
14241457
this.printProjects();
14251458
}
14261459

@@ -1454,6 +1487,11 @@ namespace ts.server {
14541487
// at this point if file is the part of some configured/external project then this project should be created
14551488
const info = this.getOrCreateScriptInfoForNormalizedPath(fileName, /*openedByClient*/ true, fileContent, scriptKind, hasMixedContent);
14561489
this.assignScriptInfoToInferredProjectIfNecessary(info, /*addToListOfOpenFiles*/ true);
1490+
// Delete the orphan files here because there might be orphan script infos (which are not part of project)
1491+
// when some file/s were closed which resulted in project removal.
1492+
// It was then postponed to cleanup these script infos so that they can be reused if
1493+
// the file from that old project is reopened because of opening file from here.
1494+
this.deleteOrphanScriptInfoNotInAnyProject();
14571495
this.printProjects();
14581496
return { configFileName, configFileErrors };
14591497
}

src/server/lsHost.ts

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,11 +11,11 @@ namespace ts.server {
1111

1212
private filesWithChangedSetOfUnresolvedImports: Path[];
1313

14-
private readonly resolveModuleName: typeof resolveModuleName;
14+
private resolveModuleName: typeof resolveModuleName;
1515
readonly trace: (s: string) => void;
1616
readonly realpath?: (path: string) => string;
1717

18-
constructor(private readonly host: ServerHost, private readonly project: Project, private readonly cancellationToken: HostCancellationToken) {
18+
constructor(private readonly host: ServerHost, private project: Project, private readonly cancellationToken: HostCancellationToken) {
1919
this.cancellationToken = new ThrottledCancellationToken(cancellationToken, project.projectService.throttleWaitMilliseconds);
2020
this.getCanonicalFileName = ts.createGetCanonicalFileName(this.host.useCaseSensitiveFileNames);
2121

@@ -47,6 +47,11 @@ namespace ts.server {
4747
}
4848
}
4949

50+
dispose() {
51+
this.project = undefined;
52+
this.resolveModuleName = undefined;
53+
}
54+
5055
public startRecordingFilesWithChangedResolutions() {
5156
this.filesWithChangedSetOfUnresolvedImports = [];
5257
}
@@ -238,4 +243,4 @@ namespace ts.server {
238243
this.compilationSettings = opt;
239244
}
240245
}
241-
}
246+
}

src/server/project.ts

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ namespace ts.server {
116116

117117
public languageServiceEnabled = true;
118118

119-
protected readonly lsHost: LSHost;
119+
protected lsHost: LSHost;
120120

121121
builder: Builder;
122122
/**
@@ -297,9 +297,15 @@ namespace ts.server {
297297
this.rootFiles = undefined;
298298
this.rootFilesMap = undefined;
299299
this.program = undefined;
300+
this.builder = undefined;
301+
this.cachedUnresolvedImportsPerFile = undefined;
302+
this.projectErrors = undefined;
303+
this.lsHost.dispose();
304+
this.lsHost = undefined;
300305

301306
// signal language service to release source files acquired from document registry
302307
this.languageService.dispose();
308+
this.languageService = undefined;
303309
}
304310

305311
getCompilerOptions() {
@@ -1043,6 +1049,7 @@ namespace ts.server {
10431049

10441050
if (this.projectFileWatcher) {
10451051
this.projectFileWatcher.close();
1052+
this.projectFileWatcher = undefined;
10461053
}
10471054

10481055
if (this.typeRootsWatchers) {
@@ -1132,4 +1139,4 @@ namespace ts.server {
11321139
this.typeAcquisition = newTypeAcquisition;
11331140
}
11341141
}
1135-
}
1142+
}

src/services/services.ts

Lines changed: 19 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -822,7 +822,7 @@ namespace ts {
822822
private _compilationSettings: CompilerOptions;
823823
private currentDirectory: string;
824824

825-
constructor(private host: LanguageServiceHost, private getCanonicalFileName: (fileName: string) => string) {
825+
constructor(private host: LanguageServiceHost, getCanonicalFileName: (fileName: string) => string) {
826826
// script id => script index
827827
this.currentDirectory = host.getCurrentDirectory();
828828
this.fileNameToEntry = createFileMap<HostFileInformation>();
@@ -857,22 +857,17 @@ namespace ts {
857857
return entry;
858858
}
859859

860-
private getEntry(path: Path): HostFileInformation {
860+
public getEntryByPath(path: Path): HostFileInformation {
861861
return this.fileNameToEntry.get(path);
862862
}
863863

864-
private contains(path: Path): boolean {
864+
public containsEntryByPath(path: Path): boolean {
865865
return this.fileNameToEntry.contains(path);
866866
}
867867

868-
public getOrCreateEntry(fileName: string): HostFileInformation {
869-
const path = toPath(fileName, this.currentDirectory, this.getCanonicalFileName);
870-
return this.getOrCreateEntryByPath(fileName, path);
871-
}
872-
873868
public getOrCreateEntryByPath(fileName: string, path: Path): HostFileInformation {
874-
return this.contains(path)
875-
? this.getEntry(path)
869+
return this.containsEntryByPath(path)
870+
? this.getEntryByPath(path)
876871
: this.createEntry(fileName, path);
877872
}
878873

@@ -889,12 +884,12 @@ namespace ts {
889884
}
890885

891886
public getVersion(path: Path): string {
892-
const file = this.getEntry(path);
887+
const file = this.getEntryByPath(path);
893888
return file && file.version;
894889
}
895890

896891
public getScriptSnapshot(path: Path): IScriptSnapshot {
897-
const file = this.getEntry(path);
892+
const file = this.getEntryByPath(path);
898893
return file && file.scriptSnapshot;
899894
}
900895
}
@@ -1159,12 +1154,19 @@ namespace ts {
11591154
getCurrentDirectory: () => currentDirectory,
11601155
fileExists: (fileName): boolean => {
11611156
// stub missing host functionality
1162-
return hostCache.getOrCreateEntry(fileName) !== undefined;
1157+
const path = toPath(fileName, currentDirectory, getCanonicalFileName);
1158+
return hostCache.containsEntryByPath(path) ?
1159+
!!hostCache.getEntryByPath(path) :
1160+
(host.fileExists && host.fileExists(fileName));
11631161
},
11641162
readFile: (fileName): string => {
11651163
// stub missing host functionality
1166-
const entry = hostCache.getOrCreateEntry(fileName);
1167-
return entry && entry.scriptSnapshot.getText(0, entry.scriptSnapshot.getLength());
1164+
const path = toPath(fileName, currentDirectory, getCanonicalFileName);
1165+
if (hostCache.containsEntryByPath(path)) {
1166+
const entry = hostCache.getEntryByPath(path);
1167+
return entry && entry.scriptSnapshot.getText(0, entry.scriptSnapshot.getLength());
1168+
}
1169+
return host.readFile && host.readFile(fileName);
11681170
},
11691171
directoryExists: directoryName => {
11701172
return directoryProbablyExists(directoryName, host);
@@ -1316,7 +1318,9 @@ namespace ts {
13161318
if (program) {
13171319
forEach(program.getSourceFiles(), f =>
13181320
documentRegistry.releaseDocument(f.fileName, program.getCompilerOptions()));
1321+
program = undefined;
13191322
}
1323+
host = undefined;
13201324
}
13211325

13221326
/// Diagnostics

0 commit comments

Comments
 (0)