Merge pull request #16494 from Microsoft/dontCreateScriptInfosWithFileExistsAndReadFile

Fixes the memory leak because of project and its corresponding script info even after project is removed
This commit is contained in:
Sheetal Nandi
2017-06-14 15:17:25 -07:00
committed by GitHub
5 changed files with 87 additions and 32 deletions

View File

@@ -2204,7 +2204,8 @@ namespace ts.projectSystem {
projectService.closeClientFile(f1.path);
projectService.checkNumberOfProjects({});
for (const f of [f2, f3]) {
for (const f of [f1, 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}'`);
}

View File

@@ -565,10 +565,17 @@ namespace ts.server {
}
else {
if (info && (!info.isScriptOpen())) {
// file has been changed which might affect the set of referenced files in projects that include
// this file and set of inferred projects
info.reloadFromFile();
this.updateProjectGraphs(info.containingProjects);
if (info.containingProjects.length === 0) {
// Orphan script info, remove it as we can always reload it on next open
info.stopWatcher();
this.filenameToScriptInfo.remove(info.path);
}
else {
// file has been changed which might affect the set of referenced files in projects that include
// this file and set of inferred projects
info.reloadFromFile();
this.updateProjectGraphs(info.containingProjects);
}
}
}
}
@@ -829,10 +836,29 @@ namespace ts.server {
this.assignScriptInfoToInferredProjectIfNecessary(f, /*addToListOfOpenFiles*/ false);
}
}
// Cleanup script infos that arent part of any project is postponed to
// next file open so that if file from same project is opened we wont end up creating same script infos
}
if (info.containingProjects.length === 0) {
// if there are not projects that include this script info - delete it
this.filenameToScriptInfo.remove(info.path);
// If the current info is being just closed - add the watcher file to track changes
// But if file was deleted, handle that part
if (this.host.fileExists(info.fileName)) {
this.watchClosedScriptInfo(info);
}
else {
this.handleDeletedFile(info);
}
}
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);
}
}
}
@@ -1303,6 +1329,14 @@ namespace ts.server {
return this.getScriptInfoForNormalizedPath(toNormalizedPath(uncheckedFileName));
}
watchClosedScriptInfo(info: ScriptInfo) {
// do not watch files with mixed content - server doesn't know how to interpret it
if (!info.hasMixedContent) {
const { fileName } = info;
info.setWatcher(this.host.watchFile(fileName, _ => this.onSourceFileChanged(fileName)));
}
}
getOrCreateScriptInfoForNormalizedPath(fileName: NormalizedPath, openedByClient: boolean, fileContent?: string, scriptKind?: ScriptKind, hasMixedContent?: boolean) {
let info = this.getScriptInfoForNormalizedPath(fileName);
if (!info) {
@@ -1318,15 +1352,13 @@ namespace ts.server {
}
}
else {
// do not watch files with mixed content - server doesn't know how to interpret it
if (!hasMixedContent) {
info.setWatcher(this.host.watchFile(fileName, _ => this.onSourceFileChanged(fileName)));
}
this.watchClosedScriptInfo(info);
}
}
}
if (info) {
if (openedByClient && !info.isScriptOpen()) {
info.stopWatcher();
info.open(fileContent);
if (hasMixedContent) {
info.registerFileUpdate();
@@ -1421,6 +1453,7 @@ namespace ts.server {
for (const p of this.inferredProjects) {
p.updateGraph();
}
this.printProjects();
}
@@ -1454,6 +1487,11 @@ namespace ts.server {
// at this point if file is the part of some configured/external project then this project should be created
const info = this.getOrCreateScriptInfoForNormalizedPath(fileName, /*openedByClient*/ true, fileContent, scriptKind, hasMixedContent);
this.assignScriptInfoToInferredProjectIfNecessary(info, /*addToListOfOpenFiles*/ true);
// Delete the orphan files here because there might be orphan script infos (which are not part of project)
// when some file/s were closed which resulted in project removal.
// It was then postponed to cleanup these script infos so that they can be reused if
// the file from that old project is reopened because of opening file from here.
this.deleteOrphanScriptInfoNotInAnyProject();
this.printProjects();
return { configFileName, configFileErrors };
}

View File

@@ -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;
}
}
}
}

View File

@@ -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;
}
}
}
}

View File

@@ -822,7 +822,7 @@ namespace ts {
private _compilationSettings: CompilerOptions;
private currentDirectory: string;
constructor(private host: LanguageServiceHost, private getCanonicalFileName: (fileName: string) => string) {
constructor(private host: LanguageServiceHost, getCanonicalFileName: (fileName: string) => string) {
// script id => script index
this.currentDirectory = host.getCurrentDirectory();
this.fileNameToEntry = createFileMap<HostFileInformation>();
@@ -857,22 +857,17 @@ namespace ts {
return entry;
}
private getEntry(path: Path): HostFileInformation {
public getEntryByPath(path: Path): HostFileInformation {
return this.fileNameToEntry.get(path);
}
private contains(path: Path): boolean {
public containsEntryByPath(path: Path): boolean {
return this.fileNameToEntry.contains(path);
}
public getOrCreateEntry(fileName: string): HostFileInformation {
const path = toPath(fileName, this.currentDirectory, this.getCanonicalFileName);
return this.getOrCreateEntryByPath(fileName, path);
}
public getOrCreateEntryByPath(fileName: string, path: Path): HostFileInformation {
return this.contains(path)
? this.getEntry(path)
return this.containsEntryByPath(path)
? this.getEntryByPath(path)
: this.createEntry(fileName, path);
}
@@ -889,12 +884,12 @@ namespace ts {
}
public getVersion(path: Path): string {
const file = this.getEntry(path);
const file = this.getEntryByPath(path);
return file && file.version;
}
public getScriptSnapshot(path: Path): IScriptSnapshot {
const file = this.getEntry(path);
const file = this.getEntryByPath(path);
return file && file.scriptSnapshot;
}
}
@@ -1159,12 +1154,19 @@ namespace ts {
getCurrentDirectory: () => currentDirectory,
fileExists: (fileName): boolean => {
// stub missing host functionality
return hostCache.getOrCreateEntry(fileName) !== undefined;
const path = toPath(fileName, currentDirectory, getCanonicalFileName);
return hostCache.containsEntryByPath(path) ?
!!hostCache.getEntryByPath(path) :
(host.fileExists && host.fileExists(fileName));
},
readFile: (fileName): string => {
// stub missing host functionality
const entry = hostCache.getOrCreateEntry(fileName);
return entry && entry.scriptSnapshot.getText(0, entry.scriptSnapshot.getLength());
const path = toPath(fileName, currentDirectory, getCanonicalFileName);
if (hostCache.containsEntryByPath(path)) {
const entry = hostCache.getEntryByPath(path);
return entry && entry.scriptSnapshot.getText(0, entry.scriptSnapshot.getLength());
}
return host.readFile && host.readFile(fileName);
},
directoryExists: directoryName => {
return directoryProbablyExists(directoryName, host);
@@ -1316,7 +1318,9 @@ namespace ts {
if (program) {
forEach(program.getSourceFiles(), f =>
documentRegistry.releaseDocument(f.fileName, program.getCompilerOptions()));
program = undefined;
}
host = undefined;
}
/// Diagnostics