Cleanup script infos that are not part of any project when the project is closed or inferred projects are refreshed

sheetalkamat · sheetalkamat · commit 1bf1209f7ea0 · 2017-06-12T16:52:36.000-07:00
Also dispose some pointers so that the closures get disposed with project and script infos
diff --git a/src/harness/unittests/tsserverProjectSystem.ts b/src/harness/unittests/tsserverProjectSystem.ts
@@ -2205,8 +2205,9 @@ namespace ts.projectSystem {
             projectService.checkNumberOfProjects({});
 
             for (const f of [f2, f3]) {
+                // There shouldnt be any script info as we closed the file that resulted in creation of it
                 const scriptInfo = projectService.getScriptInfoForNormalizedPath(server.toNormalizedPath(f.path));
-                assert.equal(scriptInfo.containingProjects.length, 0, `expect 0 containing projects for '${f.path}'`);
+                assert.equal(scriptInfo, undefined, `expected script info to be closed: '${f.path}'`);
             }
         });
 
diff --git a/src/server/editorServices.ts b/src/server/editorServices.ts
@@ -827,10 +827,20 @@ namespace ts.server {
                         this.assignScriptInfoToInferredProjectIfNecessary(f, /*addToListOfOpenFiles*/ false);
                     }
                 }
+
+                // Cleanup script infos that are not open and not part of any project
+                this.deleteOrphanScriptInfoNotInAnyProject();
             }
-            if (info.containingProjects.length === 0) {
-                // if there are not projects that include this script info - delete it
-                this.filenameToScriptInfo.remove(info.path);
+        }
+
+        private deleteOrphanScriptInfoNotInAnyProject() {
+            for (const path of this.filenameToScriptInfo.getKeys()) {
+                const info = this.filenameToScriptInfo.get(path);
+                if (!info.isScriptOpen() && info.containingProjects.length === 0) {
+                    // if there are not projects that include this script info - delete it
+                    info.stopWatcher();
+                    this.filenameToScriptInfo.remove(info.path);
+                }
             }
         }
 
@@ -1418,6 +1428,8 @@ namespace ts.server {
             for (const p of this.inferredProjects) {
                 p.updateGraph();
             }
+
+            this.deleteOrphanScriptInfoNotInAnyProject();
             this.printProjects();
         }
 
diff --git a/src/server/lsHost.ts b/src/server/lsHost.ts
@@ -11,11 +11,11 @@ namespace ts.server {
 
         private filesWithChangedSetOfUnresolvedImports: Path[];
 
-        private readonly resolveModuleName: typeof resolveModuleName;
+        private resolveModuleName: typeof resolveModuleName;
         readonly trace: (s: string) => void;
         readonly realpath?: (path: string) => string;
 
-        constructor(private readonly host: ServerHost, private readonly project: Project, private readonly cancellationToken: HostCancellationToken) {
+        constructor(private readonly host: ServerHost, private project: Project, private readonly cancellationToken: HostCancellationToken) {
             this.cancellationToken = new ThrottledCancellationToken(cancellationToken, project.projectService.throttleWaitMilliseconds);
             this.getCanonicalFileName = ts.createGetCanonicalFileName(this.host.useCaseSensitiveFileNames);
 
@@ -47,6 +47,11 @@ namespace ts.server {
             }
         }
 
+        dispose() {
+            this.project = undefined;
+            this.resolveModuleName = undefined;
+        }
+
         public startRecordingFilesWithChangedResolutions() {
             this.filesWithChangedSetOfUnresolvedImports = [];
         }
@@ -238,4 +243,4 @@ namespace ts.server {
             this.compilationSettings = opt;
         }
     }
-}
+}
diff --git a/src/server/project.ts b/src/server/project.ts
@@ -116,7 +116,7 @@ namespace ts.server {
 
         public languageServiceEnabled = true;
 
-        protected readonly lsHost: LSHost;
+        protected lsHost: LSHost;
 
         builder: Builder;
         /**
@@ -297,9 +297,15 @@ namespace ts.server {
             this.rootFiles = undefined;
             this.rootFilesMap = undefined;
             this.program = undefined;
+            this.builder = undefined;
+            this.cachedUnresolvedImportsPerFile = undefined;
+            this.projectErrors = undefined;
+            this.lsHost.dispose();
+            this.lsHost = undefined;
 
             // signal language service to release source files acquired from document registry
             this.languageService.dispose();
+            this.languageService = undefined;
         }
 
         getCompilerOptions() {
@@ -1043,6 +1049,7 @@ namespace ts.server {
 
             if (this.projectFileWatcher) {
                 this.projectFileWatcher.close();
+                this.projectFileWatcher = undefined;
             }
 
             if (this.typeRootsWatchers) {
@@ -1132,4 +1139,4 @@ namespace ts.server {
             this.typeAcquisition = newTypeAcquisition;
         }
     }
-}
+}
diff --git a/src/services/services.ts b/src/services/services.ts
@@ -1311,7 +1311,9 @@ namespace ts {
             if (program) {
                 forEach(program.getSourceFiles(), f =>
                     documentRegistry.releaseDocument(f.fileName, program.getCompilerOptions()));
+                program = undefined;
             }
+            host = undefined;
         }
 
         /// Diagnostics

Original file line number	Diff line number	Diff line change
`@@ -2205,8 +2205,9 @@ namespace ts.projectSystem {`
`2205`	`2205`	`projectService.checkNumberOfProjects({});`
`2206`	`2206`
`2207`	`2207`	`for (const f of [f2, f3]) {`
	`2208`	`+ // There shouldnt be any script info as we closed the file that resulted in creation of it`
`2208`	`2209`	`const scriptInfo = projectService.getScriptInfoForNormalizedPath(server.toNormalizedPath(f.path));`
`2209`		- assert.equal(scriptInfo.containingProjects.length, 0, `expect 0 containing projects for '${f.path}'`);
	`2210`	+ assert.equal(scriptInfo, undefined, `expected script info to be closed: '${f.path}'`);
`2210`	`2211`	`}`
`2211`	`2212`	`});`
`2212`	`2213`
Original file line number	Diff line number	Diff line change
`@@ -827,10 +827,20 @@ namespace ts.server {`
`827`	`827`	`this.assignScriptInfoToInferredProjectIfNecessary(f, /addToListOfOpenFiles/ false);`
`828`	`828`	`}`
`829`	`829`	`}`
	`830`	`+`
	`831`	`+ // Cleanup script infos that are not open and not part of any project`
	`832`	`+ this.deleteOrphanScriptInfoNotInAnyProject();`
`830`	`833`	`}`
`831`		`- if (info.containingProjects.length === 0) {`
`832`		`- // if there are not projects that include this script info - delete it`
`833`		`- this.filenameToScriptInfo.remove(info.path);`
	`834`	`+ }`
	`835`	`+`
	`836`	`+ private deleteOrphanScriptInfoNotInAnyProject() {`
	`837`	`+ for (const path of this.filenameToScriptInfo.getKeys()) {`
	`838`	`+ const info = this.filenameToScriptInfo.get(path);`
	`839`	`+ if (!info.isScriptOpen() && info.containingProjects.length === 0) {`
	`840`	`+ // if there are not projects that include this script info - delete it`
	`841`	`+ info.stopWatcher();`
	`842`	`+ this.filenameToScriptInfo.remove(info.path);`
	`843`	`+ }`
`834`	`844`	`}`
`835`	`845`	`}`
`836`	`846`
`@@ -1418,6 +1428,8 @@ namespace ts.server {`
`1418`	`1428`	`for (const p of this.inferredProjects) {`
`1419`	`1429`	`p.updateGraph();`
`1420`	`1430`	`}`
	`1431`	`+`
	`1432`	`+ this.deleteOrphanScriptInfoNotInAnyProject();`
`1421`	`1433`	`this.printProjects();`
`1422`	`1434`	`}`
`1423`	`1435`
Original file line number	Diff line number	Diff line change
`@@ -11,11 +11,11 @@ namespace ts.server {`
`11`	`11`
`12`	`12`	`private filesWithChangedSetOfUnresolvedImports: Path[];`
`13`	`13`
`14`		`- private readonly resolveModuleName: typeof resolveModuleName;`
	`14`	`+ private resolveModuleName: typeof resolveModuleName;`
`15`	`15`	`readonly trace: (s: string) => void;`
`16`	`16`	`readonly realpath?: (path: string) => string;`
`17`	`17`
`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) {`
`19`	`19`	`this.cancellationToken = new ThrottledCancellationToken(cancellationToken, project.projectService.throttleWaitMilliseconds);`
`20`	`20`	`this.getCanonicalFileName = ts.createGetCanonicalFileName(this.host.useCaseSensitiveFileNames);`
`21`	`21`
`@@ -47,6 +47,11 @@ namespace ts.server {`
`47`	`47`	`}`
`48`	`48`	`}`
`49`	`49`
	`50`	`+ dispose() {`
	`51`	`+ this.project = undefined;`
	`52`	`+ this.resolveModuleName = undefined;`
	`53`	`+ }`
	`54`	`+`
`50`	`55`	`public startRecordingFilesWithChangedResolutions() {`
`51`	`56`	`this.filesWithChangedSetOfUnresolvedImports = [];`
`52`	`57`	`}`
`@@ -238,4 +243,4 @@ namespace ts.server {`
`238`	`243`	`this.compilationSettings = opt;`
`239`	`244`	`}`
`240`	`245`	`}`
`241`		`-}`
	`246`	`+}`
Original file line number	Diff line number	Diff line change
`@@ -1311,7 +1311,9 @@ namespace ts {`
`1311`	`1311`	`if (program) {`
`1312`	`1312`	`forEach(program.getSourceFiles(), f =>`
`1313`	`1313`	`documentRegistry.releaseDocument(f.fileName, program.getCompilerOptions()));`
	`1314`	`+ program = undefined;`
`1314`	`1315`	`}`
	`1316`	`+ host = undefined;`
`1315`	`1317`	`}`
`1316`	`1318`
`1317`	`1319`	`/// Diagnostics`