Merge pull request microsoft#19053 from Microsoft/resolutionCacheDefensiveChecks

sheetalkamat · web-flow · commit d7269f1386bb · 2017-10-10T17:18:53.000-07:00
Resolution cache defensive checks
diff --git a/src/compiler/resolutionCache.ts b/src/compiler/resolutionCache.ts
@@ -141,7 +141,10 @@ namespace ts {
             resolvedModuleNames.clear();
             resolvedTypeReferenceDirectives.clear();
             allFilesHaveInvalidatedResolution = false;
-            Debug.assert(perDirectoryResolvedModuleNames.size === 0 && perDirectoryResolvedTypeReferenceDirectives.size === 0);
+            // perDirectoryResolvedModuleNames and perDirectoryResolvedTypeReferenceDirectives could be non empty if there was exception during program update
+            // (between startCachingPerDirectoryResolution and finishCachingPerDirectoryResolution)
+            perDirectoryResolvedModuleNames.clear();
+            perDirectoryResolvedTypeReferenceDirectives.clear();
         }
 
         function startRecordingFilesWithChangedResolutions() {
@@ -166,7 +169,10 @@ namespace ts {
         }
 
         function startCachingPerDirectoryResolution() {
-            Debug.assert(perDirectoryResolvedModuleNames.size === 0 && perDirectoryResolvedTypeReferenceDirectives.size === 0);
+            // perDirectoryResolvedModuleNames and perDirectoryResolvedTypeReferenceDirectives could be non empty if there was exception during program update
+            // (between startCachingPerDirectoryResolution and finishCachingPerDirectoryResolution)
+            perDirectoryResolvedModuleNames.clear();
+            perDirectoryResolvedTypeReferenceDirectives.clear();
         }
 
         function finishCachingPerDirectoryResolution() {
diff --git a/src/compiler/watch.ts b/src/compiler/watch.ts
@@ -322,21 +322,21 @@ namespace ts {
 
             if (hasChangedCompilerOptions) {
                 newLine = getNewLineCharacter(compilerOptions, system);
+                if (program && changesAffectModuleResolution(program.getCompilerOptions(), compilerOptions)) {
+                    resolutionCache.clear();
+                }
             }
 
             const hasInvalidatedResolution = resolutionCache.createHasInvalidatedResolution();
             if (isProgramUptoDate(program, rootFileNames, compilerOptions, getSourceVersion, fileExists, hasInvalidatedResolution, hasChangedAutomaticTypeDirectiveNames)) {
                 return;
             }
 
-            if (hasChangedCompilerOptions && changesAffectModuleResolution(program && program.getCompilerOptions(), compilerOptions)) {
-                resolutionCache.clear();
-            }
-            const needsUpdateInTypeRootWatch = hasChangedCompilerOptions || !program;
-            hasChangedCompilerOptions = false;
             beforeCompile(compilerOptions);
 
             // Compile the program
+            const needsUpdateInTypeRootWatch = hasChangedCompilerOptions || !program;
+            hasChangedCompilerOptions = false;
             resolutionCache.startCachingPerDirectoryResolution();
             compilerHost.hasInvalidatedResolution = hasInvalidatedResolution;
             compilerHost.hasChangedAutomaticTypeDirectiveNames = hasChangedAutomaticTypeDirectiveNames;
diff --git a/src/server/editorServices.ts b/src/server/editorServices.ts
@@ -842,6 +842,9 @@ namespace ts.server {
             this.logger.info(`remove project: ${project.getRootFiles().toString()}`);
 
             project.close();
+            if (Debug.shouldAssert(AssertionLevel.Normal)) {
+                this.filenameToScriptInfo.forEach(info => Debug.assert(!info.isAttached(project)));
+            }
             // Remove the project from pending project updates
             this.pendingProjectUpdates.delete(project.getProjectName());
 
diff --git a/src/server/project.ts b/src/server/project.ts
@@ -233,9 +233,9 @@ namespace ts.server {
                 this.realpath = path => host.realpath(path);
             }
 
-            this.languageService = createLanguageService(this, this.documentRegistry);
             // Use the current directory as resolution root only if the project created using current directory string
             this.resolutionCache = createResolutionCache(this, currentDirectory && this.currentDirectory);
+            this.languageService = createLanguageService(this, this.documentRegistry);
             if (!languageServiceEnabled) {
                 this.disableLanguageService();
             }
@@ -498,25 +498,23 @@ namespace ts.server {
 
         close() {
             if (this.program) {
-                // if we have a program - release all files that are enlisted in program
+                // if we have a program - release all files that are enlisted in program but arent root
+                // The releasing of the roots happens later
+                // The project could have pending update remaining and hence the info could be in the files but not in program graph
                 for (const f of this.program.getSourceFiles()) {
-                    const info = this.projectService.getScriptInfo(f.fileName);
-                    // We might not find the script info in case its not associated with the project any more
-                    // and project graph was not updated (eg delayed update graph in case of files changed/deleted on the disk)
-                    if (info) {
-                        info.detachFromProject(this);
-                    }
+                    this.detachScriptInfoIfNotRoot(f.fileName);
                 }
             }
-            if (!this.program || !this.languageServiceEnabled) {
-                // release all root files either if there is no program or language service is disabled.
-                // in the latter case set of root files can be larger than the set of files in program.
-                for (const root of this.rootFiles) {
-                    root.detachFromProject(this);
-                }
+            // Release external files
+            forEach(this.externalFiles, externalFile => this.detachScriptInfoIfNotRoot(externalFile));
+            // Always remove root files from the project
+            for (const root of this.rootFiles) {
+                root.detachFromProject(this);
             }
+
             this.rootFiles = undefined;
             this.rootFilesMap = undefined;
+            this.externalFiles = undefined;
             this.program = undefined;
             this.builder = undefined;
             this.resolutionCache.clear();
@@ -535,6 +533,15 @@ namespace ts.server {
             this.languageService = undefined;
         }
 
+        private detachScriptInfoIfNotRoot(uncheckedFilename: string) {
+            const info = this.projectService.getScriptInfo(uncheckedFilename);
+            // We might not find the script info in case its not associated with the project any more
+            // and project graph was not updated (eg delayed update graph in case of files changed/deleted on the disk)
+            if (info && !this.isRoot(info)) {
+                info.detachFromProject(this);
+            }
+        }
+
         isClosed() {
             return this.rootFiles === undefined;
         }
@@ -735,7 +742,6 @@ namespace ts.server {
          */
         updateGraph(): boolean {
             this.resolutionCache.startRecordingFilesWithChangedResolutions();
-            this.hasInvalidatedResolution = this.resolutionCache.createHasInvalidatedResolution();
 
             let hasChanges = this.updateGraphWorker();
 
@@ -795,9 +801,10 @@ namespace ts.server {
 
         private updateGraphWorker() {
             const oldProgram = this.program;
-
+            Debug.assert(!this.isClosed(), "Called update graph worker of closed project");
             this.writeLog(`Starting updateGraphWorker: Project: ${this.getProjectName()}`);
             const start = timestamp();
+            this.hasInvalidatedResolution = this.resolutionCache.createHasInvalidatedResolution();
             this.resolutionCache.startCachingPerDirectoryResolution();
             this.program = this.languageService.getProgram();
             this.resolutionCache.finishCachingPerDirectoryResolution();
@@ -1320,14 +1327,13 @@ namespace ts.server {
         }
 
         close() {
-            super.close();
-
             if (this.configFileWatcher) {
                 this.configFileWatcher.close();
                 this.configFileWatcher = undefined;
             }
 
             this.stopWatchingWildCards();
+            super.close();
         }
 
         addOpenRef() {
diff --git a/tests/baselines/reference/api/tsserverlibrary.d.ts b/tests/baselines/reference/api/tsserverlibrary.d.ts
@@ -7145,6 +7145,7 @@ declare namespace ts.server {
         getExternalFiles(): SortedReadonlyArray<string>;
         getSourceFile(path: Path): SourceFile;
         close(): void;
+        private detachScriptInfoIfNotRoot(uncheckedFilename);
         isClosed(): boolean;
         hasRoots(): boolean;
         getRootFiles(): NormalizedPath[];
