From f23c79f3ae1d4fa775a2f053c3577bc1241f7cdc Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Fri, 27 Feb 2015 13:39:57 -0800 Subject: [PATCH] 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 /**