From f23c79f3ae1d4fa775a2f053c3577bc1241f7cdc Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Fri, 27 Feb 2015 13:39:57 -0800 Subject: [PATCH 1/5] Fix issue where source files could get corrupted. This could happen when you had multiple language services, and they were sharing some files. If a file got edited in one LS, it could get corrupted in the other. Now, the DocumentRegistry serves as the canonical source of 'good' source files. Language services always go to it to get the correct source file instead of trying to manually update their own source files when they are notified about changes from the host. --- src/services/services.ts | 67 ++++++++++--------- .../baselines/reference/APISample_compile.js | 16 ++--- .../reference/APISample_compile.types | 22 ++---- tests/baselines/reference/APISample_linter.js | 16 ++--- .../reference/APISample_linter.types | 22 ++---- .../reference/APISample_transform.js | 16 ++--- .../reference/APISample_transform.types | 22 ++---- .../baselines/reference/APISample_watcher.js | 16 ++--- .../reference/APISample_watcher.types | 22 ++---- 9 files changed, 73 insertions(+), 146 deletions(-) diff --git a/src/services/services.ts b/src/services/services.ts index c13ae22718b..734337a84cd 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -1270,31 +1270,21 @@ module ts { /** * Request an updated version of an already existing SourceFile with a given fileName - * and compilationSettings. The update will intern call updateLanguageServiceSourceFile + * and compilationSettings. The update will in-turn call updateLanguageServiceSourceFile * to get an updated SourceFile. * - * Note: It is not allowed to call update on a SourceFile that was not acquired from this - * registry originally. - * - * @param sourceFile The original sourceFile object to update * @param fileName The name of the file requested * @param compilationSettings Some compilation settings like target affects the * shape of a the resulting SourceFile. This allows the DocumentRegistry to store * multiple copies of the same file for different compilation settings. - * @parm scriptSnapshot Text of the file. Only used if the file was not found - * in the registry and a new one was created. - * @parm version Current version of the file. Only used if the file was not found - * in the registry and a new one was created. - * @parm textChangeRange Change ranges since the last snapshot. Only used if the file - * was not found in the registry and a new one was created. + * @param scriptSnapshot Text of the file. + * @param version Current version of the file. */ updateDocument( - sourceFile: SourceFile, fileName: string, compilationSettings: CompilerOptions, scriptSnapshot: IScriptSnapshot, - version: string, - textChangeRange: TextChangeRange): SourceFile; + version: string): SourceFile; /** * Informs the DocumentRegistry that a file is not needed any longer. @@ -1730,20 +1720,22 @@ module ts { } function updateDocument( - sourceFile: SourceFile, fileName: string, compilationSettings: CompilerOptions, scriptSnapshot: IScriptSnapshot, - version: string, - textChangeRange: TextChangeRange - ): SourceFile { + version: string): SourceFile { var bucket = getBucketForCompilationSettings(compilationSettings, /*createIfMissing*/ false); Debug.assert(bucket !== undefined); var entry = lookUp(bucket, fileName); Debug.assert(entry !== undefined); - entry.sourceFile = updateLanguageServiceSourceFile(entry.sourceFile, scriptSnapshot, version, textChangeRange); + // Check with the host if anything actually changed. + if (entry.sourceFile.version !== version) { + var textChangeRange = scriptSnapshot.getChangeRange(entry.sourceFile.scriptSnapshot); + entry.sourceFile = updateLanguageServiceSourceFile(entry.sourceFile, scriptSnapshot, version, textChangeRange); + } + return entry.sourceFile; } @@ -2162,19 +2154,34 @@ module ts { // it is safe to reuse the souceFiles; if not, then the shape of the AST can change, and the oldSourceFile // can not be reused. we have to dump all syntax trees and create new ones. if (!changesInCompilationSettingsAffectSyntax) { - // Check if the old program had this file already var oldSourceFile = program && program.getSourceFile(fileName); if (oldSourceFile) { - // This SourceFile is safe to reuse, return it - if (sourceFileUpToDate(oldSourceFile)) { - return oldSourceFile; - } - - // We have an older version of the sourceFile, incrementally parse the changes - var textChangeRange = hostFileInformation.scriptSnapshot.getChangeRange(oldSourceFile.scriptSnapshot); - return documentRegistry.updateDocument(oldSourceFile, fileName, newSettings, hostFileInformation.scriptSnapshot, hostFileInformation.version, textChangeRange); + // We already had a source file for this file name. Go to the registry to + // ensure that we get the right up to date version of it. We need this to + // address the following 'race'. Specifically, say we have the following: + // + // LS1 + // \ + // DocumentRegistry + // / + // LS2 + // + // Each LS has a reference to file 'foo.ts' at version 1. LS2 then updates + // it's version of 'foo.ts' to version 2. This will cause LS2 and the + // DocumentRegistry to have version 2 of the document. HOwever, LS1 will + // have version 1. And *importantly* this source file will be *corrupt*. + // The act of creating version 2 of the file irrevocably damages the version + // 1 file. + // + // So, later when we call into LS1, we need to make sure that it doesn't use + // it's source file any more, and instead defers to DocumentRegistry to get + // either version 1, version 2 (or some other version) depending on what the + // host says should be used. + return documentRegistry.updateDocument(fileName, newSettings, hostFileInformation.scriptSnapshot, hostFileInformation.version); } + + // We didn't already have the file. Fall through and acquire it from the registry. } // Could not find this file in the old program, create a new SourceFile for it. @@ -2228,8 +2235,8 @@ module ts { function dispose(): void { if (program) { - forEach(program.getSourceFiles(), - (f) => { documentRegistry.releaseDocument(f.fileName, program.getCompilerOptions()); }); + forEach(program.getSourceFiles(), f => + documentRegistry.releaseDocument(f.fileName, program.getCompilerOptions())); } } diff --git a/tests/baselines/reference/APISample_compile.js b/tests/baselines/reference/APISample_compile.js index 7f5bc29906a..b23774c680f 100644 --- a/tests/baselines/reference/APISample_compile.js +++ b/tests/baselines/reference/APISample_compile.js @@ -1872,25 +1872,17 @@ declare module "typescript" { acquireDocument(fileName: string, compilationSettings: CompilerOptions, scriptSnapshot: IScriptSnapshot, version: string): SourceFile; /** * Request an updated version of an already existing SourceFile with a given fileName - * and compilationSettings. The update will intern call updateLanguageServiceSourceFile + * and compilationSettings. The update will in-turn call updateLanguageServiceSourceFile * to get an updated SourceFile. * - * Note: It is not allowed to call update on a SourceFile that was not acquired from this - * registry originally. - * - * @param sourceFile The original sourceFile object to update * @param fileName The name of the file requested * @param compilationSettings Some compilation settings like target affects the * shape of a the resulting SourceFile. This allows the DocumentRegistry to store * multiple copies of the same file for different compilation settings. - * @parm scriptSnapshot Text of the file. Only used if the file was not found - * in the registry and a new one was created. - * @parm version Current version of the file. Only used if the file was not found - * in the registry and a new one was created. - * @parm textChangeRange Change ranges since the last snapshot. Only used if the file - * was not found in the registry and a new one was created. + * @param scriptSnapshot Text of the file. + * @param version Current version of the file. */ - updateDocument(sourceFile: SourceFile, fileName: string, compilationSettings: CompilerOptions, scriptSnapshot: IScriptSnapshot, version: string, textChangeRange: TextChangeRange): SourceFile; + updateDocument(fileName: string, compilationSettings: CompilerOptions, scriptSnapshot: IScriptSnapshot, version: string): SourceFile; /** * Informs the DocumentRegistry that a file is not needed any longer. * diff --git a/tests/baselines/reference/APISample_compile.types b/tests/baselines/reference/APISample_compile.types index d93a48b7852..0ec22a74523 100644 --- a/tests/baselines/reference/APISample_compile.types +++ b/tests/baselines/reference/APISample_compile.types @@ -5849,36 +5849,24 @@ declare module "typescript" { /** * Request an updated version of an already existing SourceFile with a given fileName - * and compilationSettings. The update will intern call updateLanguageServiceSourceFile + * and compilationSettings. The update will in-turn call updateLanguageServiceSourceFile * to get an updated SourceFile. * - * Note: It is not allowed to call update on a SourceFile that was not acquired from this - * registry originally. - * - * @param sourceFile The original sourceFile object to update * @param fileName The name of the file requested * @param compilationSettings Some compilation settings like target affects the * shape of a the resulting SourceFile. This allows the DocumentRegistry to store * multiple copies of the same file for different compilation settings. - * @parm scriptSnapshot Text of the file. Only used if the file was not found - * in the registry and a new one was created. - * @parm version Current version of the file. Only used if the file was not found - * in the registry and a new one was created. - * @parm textChangeRange Change ranges since the last snapshot. Only used if the file - * was not found in the registry and a new one was created. + * @param scriptSnapshot Text of the file. + * @param version Current version of the file. */ - updateDocument(sourceFile: SourceFile, fileName: string, compilationSettings: CompilerOptions, scriptSnapshot: IScriptSnapshot, version: string, textChangeRange: TextChangeRange): SourceFile; ->updateDocument : (sourceFile: SourceFile, fileName: string, compilationSettings: CompilerOptions, scriptSnapshot: IScriptSnapshot, version: string, textChangeRange: TextChangeRange) => SourceFile ->sourceFile : SourceFile ->SourceFile : SourceFile + updateDocument(fileName: string, compilationSettings: CompilerOptions, scriptSnapshot: IScriptSnapshot, version: string): SourceFile; +>updateDocument : (fileName: string, compilationSettings: CompilerOptions, scriptSnapshot: IScriptSnapshot, version: string) => SourceFile >fileName : string >compilationSettings : CompilerOptions >CompilerOptions : CompilerOptions >scriptSnapshot : IScriptSnapshot >IScriptSnapshot : IScriptSnapshot >version : string ->textChangeRange : TextChangeRange ->TextChangeRange : TextChangeRange >SourceFile : SourceFile /** diff --git a/tests/baselines/reference/APISample_linter.js b/tests/baselines/reference/APISample_linter.js index 36dd2421add..df516a19e5c 100644 --- a/tests/baselines/reference/APISample_linter.js +++ b/tests/baselines/reference/APISample_linter.js @@ -1903,25 +1903,17 @@ declare module "typescript" { acquireDocument(fileName: string, compilationSettings: CompilerOptions, scriptSnapshot: IScriptSnapshot, version: string): SourceFile; /** * Request an updated version of an already existing SourceFile with a given fileName - * and compilationSettings. The update will intern call updateLanguageServiceSourceFile + * and compilationSettings. The update will in-turn call updateLanguageServiceSourceFile * to get an updated SourceFile. * - * Note: It is not allowed to call update on a SourceFile that was not acquired from this - * registry originally. - * - * @param sourceFile The original sourceFile object to update * @param fileName The name of the file requested * @param compilationSettings Some compilation settings like target affects the * shape of a the resulting SourceFile. This allows the DocumentRegistry to store * multiple copies of the same file for different compilation settings. - * @parm scriptSnapshot Text of the file. Only used if the file was not found - * in the registry and a new one was created. - * @parm version Current version of the file. Only used if the file was not found - * in the registry and a new one was created. - * @parm textChangeRange Change ranges since the last snapshot. Only used if the file - * was not found in the registry and a new one was created. + * @param scriptSnapshot Text of the file. + * @param version Current version of the file. */ - updateDocument(sourceFile: SourceFile, fileName: string, compilationSettings: CompilerOptions, scriptSnapshot: IScriptSnapshot, version: string, textChangeRange: TextChangeRange): SourceFile; + updateDocument(fileName: string, compilationSettings: CompilerOptions, scriptSnapshot: IScriptSnapshot, version: string): SourceFile; /** * Informs the DocumentRegistry that a file is not needed any longer. * diff --git a/tests/baselines/reference/APISample_linter.types b/tests/baselines/reference/APISample_linter.types index 7a282b61f42..013bd1a84fd 100644 --- a/tests/baselines/reference/APISample_linter.types +++ b/tests/baselines/reference/APISample_linter.types @@ -5995,36 +5995,24 @@ declare module "typescript" { /** * Request an updated version of an already existing SourceFile with a given fileName - * and compilationSettings. The update will intern call updateLanguageServiceSourceFile + * and compilationSettings. The update will in-turn call updateLanguageServiceSourceFile * to get an updated SourceFile. * - * Note: It is not allowed to call update on a SourceFile that was not acquired from this - * registry originally. - * - * @param sourceFile The original sourceFile object to update * @param fileName The name of the file requested * @param compilationSettings Some compilation settings like target affects the * shape of a the resulting SourceFile. This allows the DocumentRegistry to store * multiple copies of the same file for different compilation settings. - * @parm scriptSnapshot Text of the file. Only used if the file was not found - * in the registry and a new one was created. - * @parm version Current version of the file. Only used if the file was not found - * in the registry and a new one was created. - * @parm textChangeRange Change ranges since the last snapshot. Only used if the file - * was not found in the registry and a new one was created. + * @param scriptSnapshot Text of the file. + * @param version Current version of the file. */ - updateDocument(sourceFile: SourceFile, fileName: string, compilationSettings: CompilerOptions, scriptSnapshot: IScriptSnapshot, version: string, textChangeRange: TextChangeRange): SourceFile; ->updateDocument : (sourceFile: SourceFile, fileName: string, compilationSettings: CompilerOptions, scriptSnapshot: IScriptSnapshot, version: string, textChangeRange: TextChangeRange) => SourceFile ->sourceFile : SourceFile ->SourceFile : SourceFile + updateDocument(fileName: string, compilationSettings: CompilerOptions, scriptSnapshot: IScriptSnapshot, version: string): SourceFile; +>updateDocument : (fileName: string, compilationSettings: CompilerOptions, scriptSnapshot: IScriptSnapshot, version: string) => SourceFile >fileName : string >compilationSettings : CompilerOptions >CompilerOptions : CompilerOptions >scriptSnapshot : IScriptSnapshot >IScriptSnapshot : IScriptSnapshot >version : string ->textChangeRange : TextChangeRange ->TextChangeRange : TextChangeRange >SourceFile : SourceFile /** diff --git a/tests/baselines/reference/APISample_transform.js b/tests/baselines/reference/APISample_transform.js index 2ead5140261..759a0fcc047 100644 --- a/tests/baselines/reference/APISample_transform.js +++ b/tests/baselines/reference/APISample_transform.js @@ -1904,25 +1904,17 @@ declare module "typescript" { acquireDocument(fileName: string, compilationSettings: CompilerOptions, scriptSnapshot: IScriptSnapshot, version: string): SourceFile; /** * Request an updated version of an already existing SourceFile with a given fileName - * and compilationSettings. The update will intern call updateLanguageServiceSourceFile + * and compilationSettings. The update will in-turn call updateLanguageServiceSourceFile * to get an updated SourceFile. * - * Note: It is not allowed to call update on a SourceFile that was not acquired from this - * registry originally. - * - * @param sourceFile The original sourceFile object to update * @param fileName The name of the file requested * @param compilationSettings Some compilation settings like target affects the * shape of a the resulting SourceFile. This allows the DocumentRegistry to store * multiple copies of the same file for different compilation settings. - * @parm scriptSnapshot Text of the file. Only used if the file was not found - * in the registry and a new one was created. - * @parm version Current version of the file. Only used if the file was not found - * in the registry and a new one was created. - * @parm textChangeRange Change ranges since the last snapshot. Only used if the file - * was not found in the registry and a new one was created. + * @param scriptSnapshot Text of the file. + * @param version Current version of the file. */ - updateDocument(sourceFile: SourceFile, fileName: string, compilationSettings: CompilerOptions, scriptSnapshot: IScriptSnapshot, version: string, textChangeRange: TextChangeRange): SourceFile; + updateDocument(fileName: string, compilationSettings: CompilerOptions, scriptSnapshot: IScriptSnapshot, version: string): SourceFile; /** * Informs the DocumentRegistry that a file is not needed any longer. * diff --git a/tests/baselines/reference/APISample_transform.types b/tests/baselines/reference/APISample_transform.types index 7a9d0523653..92a2a5885ff 100644 --- a/tests/baselines/reference/APISample_transform.types +++ b/tests/baselines/reference/APISample_transform.types @@ -5945,36 +5945,24 @@ declare module "typescript" { /** * Request an updated version of an already existing SourceFile with a given fileName - * and compilationSettings. The update will intern call updateLanguageServiceSourceFile + * and compilationSettings. The update will in-turn call updateLanguageServiceSourceFile * to get an updated SourceFile. * - * Note: It is not allowed to call update on a SourceFile that was not acquired from this - * registry originally. - * - * @param sourceFile The original sourceFile object to update * @param fileName The name of the file requested * @param compilationSettings Some compilation settings like target affects the * shape of a the resulting SourceFile. This allows the DocumentRegistry to store * multiple copies of the same file for different compilation settings. - * @parm scriptSnapshot Text of the file. Only used if the file was not found - * in the registry and a new one was created. - * @parm version Current version of the file. Only used if the file was not found - * in the registry and a new one was created. - * @parm textChangeRange Change ranges since the last snapshot. Only used if the file - * was not found in the registry and a new one was created. + * @param scriptSnapshot Text of the file. + * @param version Current version of the file. */ - updateDocument(sourceFile: SourceFile, fileName: string, compilationSettings: CompilerOptions, scriptSnapshot: IScriptSnapshot, version: string, textChangeRange: TextChangeRange): SourceFile; ->updateDocument : (sourceFile: SourceFile, fileName: string, compilationSettings: CompilerOptions, scriptSnapshot: IScriptSnapshot, version: string, textChangeRange: TextChangeRange) => SourceFile ->sourceFile : SourceFile ->SourceFile : SourceFile + updateDocument(fileName: string, compilationSettings: CompilerOptions, scriptSnapshot: IScriptSnapshot, version: string): SourceFile; +>updateDocument : (fileName: string, compilationSettings: CompilerOptions, scriptSnapshot: IScriptSnapshot, version: string) => SourceFile >fileName : string >compilationSettings : CompilerOptions >CompilerOptions : CompilerOptions >scriptSnapshot : IScriptSnapshot >IScriptSnapshot : IScriptSnapshot >version : string ->textChangeRange : TextChangeRange ->TextChangeRange : TextChangeRange >SourceFile : SourceFile /** diff --git a/tests/baselines/reference/APISample_watcher.js b/tests/baselines/reference/APISample_watcher.js index 1d1f9f03d68..80149676d8f 100644 --- a/tests/baselines/reference/APISample_watcher.js +++ b/tests/baselines/reference/APISample_watcher.js @@ -1941,25 +1941,17 @@ declare module "typescript" { acquireDocument(fileName: string, compilationSettings: CompilerOptions, scriptSnapshot: IScriptSnapshot, version: string): SourceFile; /** * Request an updated version of an already existing SourceFile with a given fileName - * and compilationSettings. The update will intern call updateLanguageServiceSourceFile + * and compilationSettings. The update will in-turn call updateLanguageServiceSourceFile * to get an updated SourceFile. * - * Note: It is not allowed to call update on a SourceFile that was not acquired from this - * registry originally. - * - * @param sourceFile The original sourceFile object to update * @param fileName The name of the file requested * @param compilationSettings Some compilation settings like target affects the * shape of a the resulting SourceFile. This allows the DocumentRegistry to store * multiple copies of the same file for different compilation settings. - * @parm scriptSnapshot Text of the file. Only used if the file was not found - * in the registry and a new one was created. - * @parm version Current version of the file. Only used if the file was not found - * in the registry and a new one was created. - * @parm textChangeRange Change ranges since the last snapshot. Only used if the file - * was not found in the registry and a new one was created. + * @param scriptSnapshot Text of the file. + * @param version Current version of the file. */ - updateDocument(sourceFile: SourceFile, fileName: string, compilationSettings: CompilerOptions, scriptSnapshot: IScriptSnapshot, version: string, textChangeRange: TextChangeRange): SourceFile; + updateDocument(fileName: string, compilationSettings: CompilerOptions, scriptSnapshot: IScriptSnapshot, version: string): SourceFile; /** * Informs the DocumentRegistry that a file is not needed any longer. * diff --git a/tests/baselines/reference/APISample_watcher.types b/tests/baselines/reference/APISample_watcher.types index 55c48d1354b..7eb789aca9f 100644 --- a/tests/baselines/reference/APISample_watcher.types +++ b/tests/baselines/reference/APISample_watcher.types @@ -6118,36 +6118,24 @@ declare module "typescript" { /** * Request an updated version of an already existing SourceFile with a given fileName - * and compilationSettings. The update will intern call updateLanguageServiceSourceFile + * and compilationSettings. The update will in-turn call updateLanguageServiceSourceFile * to get an updated SourceFile. * - * Note: It is not allowed to call update on a SourceFile that was not acquired from this - * registry originally. - * - * @param sourceFile The original sourceFile object to update * @param fileName The name of the file requested * @param compilationSettings Some compilation settings like target affects the * shape of a the resulting SourceFile. This allows the DocumentRegistry to store * multiple copies of the same file for different compilation settings. - * @parm scriptSnapshot Text of the file. Only used if the file was not found - * in the registry and a new one was created. - * @parm version Current version of the file. Only used if the file was not found - * in the registry and a new one was created. - * @parm textChangeRange Change ranges since the last snapshot. Only used if the file - * was not found in the registry and a new one was created. + * @param scriptSnapshot Text of the file. + * @param version Current version of the file. */ - updateDocument(sourceFile: SourceFile, fileName: string, compilationSettings: CompilerOptions, scriptSnapshot: IScriptSnapshot, version: string, textChangeRange: TextChangeRange): SourceFile; ->updateDocument : (sourceFile: SourceFile, fileName: string, compilationSettings: CompilerOptions, scriptSnapshot: IScriptSnapshot, version: string, textChangeRange: TextChangeRange) => SourceFile ->sourceFile : SourceFile ->SourceFile : SourceFile + updateDocument(fileName: string, compilationSettings: CompilerOptions, scriptSnapshot: IScriptSnapshot, version: string): SourceFile; +>updateDocument : (fileName: string, compilationSettings: CompilerOptions, scriptSnapshot: IScriptSnapshot, version: string) => SourceFile >fileName : string >compilationSettings : CompilerOptions >CompilerOptions : CompilerOptions >scriptSnapshot : IScriptSnapshot >IScriptSnapshot : IScriptSnapshot >version : string ->textChangeRange : TextChangeRange ->TextChangeRange : TextChangeRange >SourceFile : SourceFile /** From 505c1f258f6a611efccd4b4b56655ee65b53385a Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Fri, 27 Feb 2015 13:45:29 -0800 Subject: [PATCH 2/5] Update comment. --- src/services/services.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/services/services.ts b/src/services/services.ts index 734337a84cd..e295bb0c912 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -1732,6 +1732,8 @@ module ts { // Check with the host if anything actually changed. if (entry.sourceFile.version !== version) { + // If so, ask the host for what changed between these two versions and then do the + // actual incremental parsing. var textChangeRange = scriptSnapshot.getChangeRange(entry.sourceFile.scriptSnapshot); entry.sourceFile = updateLanguageServiceSourceFile(entry.sourceFile, scriptSnapshot, version, textChangeRange); } From 604c37eee2239c942e0ea17842f79fb9461c288e Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Fri, 27 Feb 2015 15:29:31 -0800 Subject: [PATCH 3/5] Whenever a document is acquired, make sure it returns a source that corresponds to the version requested. --- src/services/services.ts | 71 ++++++++++++++++++++++++---------------- 1 file changed, 42 insertions(+), 29 deletions(-) diff --git a/src/services/services.ts b/src/services/services.ts index e295bb0c912..ececc1082ca 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -1432,7 +1432,11 @@ module ts { interface DocumentRegistryEntry { sourceFile: SourceFile; - refCount: number; + + // The number of language services that this source file is referenced in. When no more + // language services are referencing the file, then the file can be removed from the + // registry. + languageServiceRefCount: number; owners: string[]; } @@ -1661,6 +1665,8 @@ module ts { } export function createDocumentRegistry(): DocumentRegistry { + // Maps from compiler setting target (ES3, ES5, etc.) to all the cached documents we have + // for those settings. var buckets: Map> = {}; function getKeyFromCompilationSettings(settings: CompilerOptions): string { @@ -1684,7 +1690,7 @@ module ts { var entry = entries[i]; sourceFiles.push({ name: i, - refCount: entry.refCount, + refCount: entry.languageServiceRefCount, references: entry.owners.slice(0) }); } @@ -1697,45 +1703,52 @@ module ts { return JSON.stringify(bucketInfoArray, null, 2); } - function acquireDocument( + function acquireDocument(fileName: string, compilationSettings: CompilerOptions, scriptSnapshot: IScriptSnapshot, version: string): SourceFile { + return acquireOrUpdateDocument(fileName, compilationSettings, scriptSnapshot, version, /*acquiring:*/ true); + } + + function updateDocument(fileName: string, compilationSettings: CompilerOptions, scriptSnapshot: IScriptSnapshot, version: string): SourceFile { + return acquireOrUpdateDocument(fileName, compilationSettings, scriptSnapshot, version, /*acquiring:*/ false); + } + + function acquireOrUpdateDocument( fileName: string, compilationSettings: CompilerOptions, scriptSnapshot: IScriptSnapshot, - version: string): SourceFile { + version: string, + acquiring: boolean): SourceFile { var bucket = getBucketForCompilationSettings(compilationSettings, /*createIfMissing*/ true); var entry = lookUp(bucket, fileName); if (!entry) { + Debug.assert(acquiring, "How could we be trying to update a document that the registry doesn't have?"); + + // Have never seen this file with these settings. Create a new source file for it. var sourceFile = createLanguageServiceSourceFile(fileName, scriptSnapshot, compilationSettings.target, version, /*setNodeParents:*/ false); bucket[fileName] = entry = { sourceFile: sourceFile, - refCount: 0, + languageServiceRefCount: 0, owners: [] }; } - entry.refCount++; + else { + // We have an entry for this file. However, it may be for a different version of + // the script snapshot. If so, update it appropriately. Otherwise, we can just + // return it as is. + if (entry.sourceFile.version !== version) { + entry.sourceFile = updateLanguageServiceSourceFile(entry.sourceFile, scriptSnapshot, version, + scriptSnapshot.getChangeRange(entry.sourceFile.scriptSnapshot)); + } + } - return entry.sourceFile; - } - - function updateDocument( - fileName: string, - compilationSettings: CompilerOptions, - scriptSnapshot: IScriptSnapshot, - version: string): SourceFile { - - var bucket = getBucketForCompilationSettings(compilationSettings, /*createIfMissing*/ false); - Debug.assert(bucket !== undefined); - var entry = lookUp(bucket, fileName); - Debug.assert(entry !== undefined); - - // Check with the host if anything actually changed. - if (entry.sourceFile.version !== version) { - // If so, ask the host for what changed between these two versions and then do the - // actual incremental parsing. - var textChangeRange = scriptSnapshot.getChangeRange(entry.sourceFile.scriptSnapshot); - entry.sourceFile = updateLanguageServiceSourceFile(entry.sourceFile, scriptSnapshot, version, textChangeRange); + // If we're acquiring, then this is the first time this LS is asking for this document. + // Increase our ref count so we know there's another LS using the document. If we're + // not acquiring, then that means the LS is 'updating' the file instead, and that means + // it has already acquired the document previously. As such, we do not need to increase + // the ref count. + if (acquiring) { + entry.languageServiceRefCount++; } return entry.sourceFile; @@ -1746,10 +1759,10 @@ module ts { Debug.assert(bucket !== undefined); var entry = lookUp(bucket, fileName); - entry.refCount--; + entry.languageServiceRefCount--; - Debug.assert(entry.refCount >= 0); - if (entry.refCount === 0) { + Debug.assert(entry.languageServiceRefCount >= 0); + if (entry.languageServiceRefCount === 0) { delete bucket[fileName]; } } From 545fa20efdfa6d8372052324abc70fce6686ed5a Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Fri, 27 Feb 2015 16:25:44 -0800 Subject: [PATCH 4/5] Add registry tests. --- .../unittests/services/documentRegistry.ts | 36 +++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/tests/cases/unittests/services/documentRegistry.ts b/tests/cases/unittests/services/documentRegistry.ts index 339c8866329..34dda543c1c 100644 --- a/tests/cases/unittests/services/documentRegistry.ts +++ b/tests/cases/unittests/services/documentRegistry.ts @@ -35,4 +35,40 @@ describe("DocumentRegistry", () => { assert(f3 === f4, "Changed module: Expected to have the same instance of the document"); }); + + it("Acquiring document gets correct version 1", () => { + var documentRegistry = ts.createDocumentRegistry(); + var defaultCompilerOptions = ts.getDefaultCompilerOptions(); + + // Simulate one LS getting the document. + var f1 = documentRegistry.acquireDocument("file1.ts", defaultCompilerOptions, ts.ScriptSnapshot.fromString("var x = 1;"), /* version */ "1"); + + // Simulate another LS getting the document at another version. + var f2 = documentRegistry.acquireDocument("file1.ts", defaultCompilerOptions, ts.ScriptSnapshot.fromString("var x = 1;"), /* version */ "2"); + + assert(f2.version === "2"); + }); + + it("Acquiring document gets correct version 2", () => { + debugger; + var documentRegistry = ts.createDocumentRegistry(); + var defaultCompilerOptions = ts.getDefaultCompilerOptions(); + + var contents = "var x = 1;" + var snapshot = ts.ScriptSnapshot.fromString(contents); + + // Simulate one LS getting the document. + var f1 = documentRegistry.acquireDocument("file1.ts", defaultCompilerOptions, snapshot, /* version */ "1"); + + // Simulate another LS getting that document. + var f2 = documentRegistry.acquireDocument("file1.ts", defaultCompilerOptions, snapshot, /* version */ "1"); + + // Now LS1 updates their document. + var f3 = documentRegistry.updateDocument(f1, "file1.ts", defaultCompilerOptions, snapshot, /* version */ "2", + ts.createTextChangeRange(ts.createTextSpan(0, contents.length), contents.length)); + + // Now LS2 tries to update their document. + var f4 = documentRegistry.updateDocument(f1, "file1.ts", defaultCompilerOptions, snapshot, /* version */ "3", + ts.createTextChangeRange(ts.createTextSpan(0, contents.length), contents.length)); + }); }); \ No newline at end of file From 3c78a0522bb56bd96b298321b7458ae924a0a5fd Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Fri, 27 Feb 2015 16:29:12 -0800 Subject: [PATCH 5/5] Add tests. --- tests/cases/unittests/services/documentRegistry.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/cases/unittests/services/documentRegistry.ts b/tests/cases/unittests/services/documentRegistry.ts index 34dda543c1c..8fc466b857f 100644 --- a/tests/cases/unittests/services/documentRegistry.ts +++ b/tests/cases/unittests/services/documentRegistry.ts @@ -50,13 +50,15 @@ describe("DocumentRegistry", () => { }); it("Acquiring document gets correct version 2", () => { - debugger; var documentRegistry = ts.createDocumentRegistry(); var defaultCompilerOptions = ts.getDefaultCompilerOptions(); var contents = "var x = 1;" var snapshot = ts.ScriptSnapshot.fromString(contents); + // Always treat any change as a full change. + snapshot.getChangeRange = old => ts.createTextChangeRange(ts.createTextSpan(0, contents.length), contents.length); + // Simulate one LS getting the document. var f1 = documentRegistry.acquireDocument("file1.ts", defaultCompilerOptions, snapshot, /* version */ "1"); @@ -64,11 +66,9 @@ describe("DocumentRegistry", () => { var f2 = documentRegistry.acquireDocument("file1.ts", defaultCompilerOptions, snapshot, /* version */ "1"); // Now LS1 updates their document. - var f3 = documentRegistry.updateDocument(f1, "file1.ts", defaultCompilerOptions, snapshot, /* version */ "2", - ts.createTextChangeRange(ts.createTextSpan(0, contents.length), contents.length)); + var f3 = documentRegistry.updateDocument("file1.ts", defaultCompilerOptions, snapshot, /* version */ "2"); // Now LS2 tries to update their document. - var f4 = documentRegistry.updateDocument(f1, "file1.ts", defaultCompilerOptions, snapshot, /* version */ "3", - ts.createTextChangeRange(ts.createTextSpan(0, contents.length), contents.length)); + var f4 = documentRegistry.updateDocument("file1.ts", defaultCompilerOptions, snapshot, /* version */ "3"); }); }); \ No newline at end of file