Skip to content

Commit 419d379

Browse files
Merge pull request #2167 from Microsoft/documentRegistry
Fix issue where source files could get corrupted.
2 parents dc917d9 + 3c78a05 commit 419d379

File tree

10 files changed

+146
-168
lines changed

10 files changed

+146
-168
lines changed

src/services/services.ts

Lines changed: 74 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -1270,31 +1270,21 @@ module ts {
12701270

12711271
/**
12721272
* Request an updated version of an already existing SourceFile with a given fileName
1273-
* and compilationSettings. The update will intern call updateLanguageServiceSourceFile
1273+
* and compilationSettings. The update will in-turn call updateLanguageServiceSourceFile
12741274
* to get an updated SourceFile.
12751275
*
1276-
* Note: It is not allowed to call update on a SourceFile that was not acquired from this
1277-
* registry originally.
1278-
*
1279-
* @param sourceFile The original sourceFile object to update
12801276
* @param fileName The name of the file requested
12811277
* @param compilationSettings Some compilation settings like target affects the
12821278
* shape of a the resulting SourceFile. This allows the DocumentRegistry to store
12831279
* multiple copies of the same file for different compilation settings.
1284-
* @parm scriptSnapshot Text of the file. Only used if the file was not found
1285-
* in the registry and a new one was created.
1286-
* @parm version Current version of the file. Only used if the file was not found
1287-
* in the registry and a new one was created.
1288-
* @parm textChangeRange Change ranges since the last snapshot. Only used if the file
1289-
* was not found in the registry and a new one was created.
1280+
* @param scriptSnapshot Text of the file.
1281+
* @param version Current version of the file.
12901282
*/
12911283
updateDocument(
1292-
sourceFile: SourceFile,
12931284
fileName: string,
12941285
compilationSettings: CompilerOptions,
12951286
scriptSnapshot: IScriptSnapshot,
1296-
version: string,
1297-
textChangeRange: TextChangeRange): SourceFile;
1287+
version: string): SourceFile;
12981288

12991289
/**
13001290
* Informs the DocumentRegistry that a file is not needed any longer.
@@ -1442,7 +1432,11 @@ module ts {
14421432

14431433
interface DocumentRegistryEntry {
14441434
sourceFile: SourceFile;
1445-
refCount: number;
1435+
1436+
// The number of language services that this source file is referenced in. When no more
1437+
// language services are referencing the file, then the file can be removed from the
1438+
// registry.
1439+
languageServiceRefCount: number;
14461440
owners: string[];
14471441
}
14481442

@@ -1671,6 +1665,8 @@ module ts {
16711665
}
16721666

16731667
export function createDocumentRegistry(): DocumentRegistry {
1668+
// Maps from compiler setting target (ES3, ES5, etc.) to all the cached documents we have
1669+
// for those settings.
16741670
var buckets: Map<Map<DocumentRegistryEntry>> = {};
16751671

16761672
function getKeyFromCompilationSettings(settings: CompilerOptions): string {
@@ -1694,7 +1690,7 @@ module ts {
16941690
var entry = entries[i];
16951691
sourceFiles.push({
16961692
name: i,
1697-
refCount: entry.refCount,
1693+
refCount: entry.languageServiceRefCount,
16981694
references: entry.owners.slice(0)
16991695
});
17001696
}
@@ -1707,43 +1703,54 @@ module ts {
17071703
return JSON.stringify(bucketInfoArray, null, 2);
17081704
}
17091705

1710-
function acquireDocument(
1706+
function acquireDocument(fileName: string, compilationSettings: CompilerOptions, scriptSnapshot: IScriptSnapshot, version: string): SourceFile {
1707+
return acquireOrUpdateDocument(fileName, compilationSettings, scriptSnapshot, version, /*acquiring:*/ true);
1708+
}
1709+
1710+
function updateDocument(fileName: string, compilationSettings: CompilerOptions, scriptSnapshot: IScriptSnapshot, version: string): SourceFile {
1711+
return acquireOrUpdateDocument(fileName, compilationSettings, scriptSnapshot, version, /*acquiring:*/ false);
1712+
}
1713+
1714+
function acquireOrUpdateDocument(
17111715
fileName: string,
17121716
compilationSettings: CompilerOptions,
17131717
scriptSnapshot: IScriptSnapshot,
1714-
version: string): SourceFile {
1718+
version: string,
1719+
acquiring: boolean): SourceFile {
17151720

17161721
var bucket = getBucketForCompilationSettings(compilationSettings, /*createIfMissing*/ true);
17171722
var entry = lookUp(bucket, fileName);
17181723
if (!entry) {
1724+
Debug.assert(acquiring, "How could we be trying to update a document that the registry doesn't have?");
1725+
1726+
// Have never seen this file with these settings. Create a new source file for it.
17191727
var sourceFile = createLanguageServiceSourceFile(fileName, scriptSnapshot, compilationSettings.target, version, /*setNodeParents:*/ false);
17201728

17211729
bucket[fileName] = entry = {
17221730
sourceFile: sourceFile,
1723-
refCount: 0,
1731+
languageServiceRefCount: 0,
17241732
owners: []
17251733
};
17261734
}
1727-
entry.refCount++;
1728-
1729-
return entry.sourceFile;
1730-
}
1731-
1732-
function updateDocument(
1733-
sourceFile: SourceFile,
1734-
fileName: string,
1735-
compilationSettings: CompilerOptions,
1736-
scriptSnapshot: IScriptSnapshot,
1737-
version: string,
1738-
textChangeRange: TextChangeRange
1739-
): SourceFile {
1735+
else {
1736+
// We have an entry for this file. However, it may be for a different version of
1737+
// the script snapshot. If so, update it appropriately. Otherwise, we can just
1738+
// return it as is.
1739+
if (entry.sourceFile.version !== version) {
1740+
entry.sourceFile = updateLanguageServiceSourceFile(entry.sourceFile, scriptSnapshot, version,
1741+
scriptSnapshot.getChangeRange(entry.sourceFile.scriptSnapshot));
1742+
}
1743+
}
17401744

1741-
var bucket = getBucketForCompilationSettings(compilationSettings, /*createIfMissing*/ false);
1742-
Debug.assert(bucket !== undefined);
1743-
var entry = lookUp(bucket, fileName);
1744-
Debug.assert(entry !== undefined);
1745+
// If we're acquiring, then this is the first time this LS is asking for this document.
1746+
// Increase our ref count so we know there's another LS using the document. If we're
1747+
// not acquiring, then that means the LS is 'updating' the file instead, and that means
1748+
// it has already acquired the document previously. As such, we do not need to increase
1749+
// the ref count.
1750+
if (acquiring) {
1751+
entry.languageServiceRefCount++;
1752+
}
17451753

1746-
entry.sourceFile = updateLanguageServiceSourceFile(entry.sourceFile, scriptSnapshot, version, textChangeRange);
17471754
return entry.sourceFile;
17481755
}
17491756

@@ -1752,10 +1759,10 @@ module ts {
17521759
Debug.assert(bucket !== undefined);
17531760

17541761
var entry = lookUp(bucket, fileName);
1755-
entry.refCount--;
1762+
entry.languageServiceRefCount--;
17561763

1757-
Debug.assert(entry.refCount >= 0);
1758-
if (entry.refCount === 0) {
1764+
Debug.assert(entry.languageServiceRefCount >= 0);
1765+
if (entry.languageServiceRefCount === 0) {
17591766
delete bucket[fileName];
17601767
}
17611768
}
@@ -2162,19 +2169,34 @@ module ts {
21622169
// it is safe to reuse the souceFiles; if not, then the shape of the AST can change, and the oldSourceFile
21632170
// can not be reused. we have to dump all syntax trees and create new ones.
21642171
if (!changesInCompilationSettingsAffectSyntax) {
2165-
21662172
// Check if the old program had this file already
21672173
var oldSourceFile = program && program.getSourceFile(fileName);
21682174
if (oldSourceFile) {
2169-
// This SourceFile is safe to reuse, return it
2170-
if (sourceFileUpToDate(oldSourceFile)) {
2171-
return oldSourceFile;
2172-
}
2173-
2174-
// We have an older version of the sourceFile, incrementally parse the changes
2175-
var textChangeRange = hostFileInformation.scriptSnapshot.getChangeRange(oldSourceFile.scriptSnapshot);
2176-
return documentRegistry.updateDocument(oldSourceFile, fileName, newSettings, hostFileInformation.scriptSnapshot, hostFileInformation.version, textChangeRange);
2177-
}
2175+
// We already had a source file for this file name. Go to the registry to
2176+
// ensure that we get the right up to date version of it. We need this to
2177+
// address the following 'race'. Specifically, say we have the following:
2178+
//
2179+
// LS1
2180+
// \
2181+
// DocumentRegistry
2182+
// /
2183+
// LS2
2184+
//
2185+
// Each LS has a reference to file 'foo.ts' at version 1. LS2 then updates
2186+
// it's version of 'foo.ts' to version 2. This will cause LS2 and the
2187+
// DocumentRegistry to have version 2 of the document. HOwever, LS1 will
2188+
// have version 1. And *importantly* this source file will be *corrupt*.
2189+
// The act of creating version 2 of the file irrevocably damages the version
2190+
// 1 file.
2191+
//
2192+
// So, later when we call into LS1, we need to make sure that it doesn't use
2193+
// it's source file any more, and instead defers to DocumentRegistry to get
2194+
// either version 1, version 2 (or some other version) depending on what the
2195+
// host says should be used.
2196+
return documentRegistry.updateDocument(fileName, newSettings, hostFileInformation.scriptSnapshot, hostFileInformation.version);
2197+
}
2198+
2199+
// We didn't already have the file. Fall through and acquire it from the registry.
21782200
}
21792201

21802202
// Could not find this file in the old program, create a new SourceFile for it.
@@ -2228,8 +2250,8 @@ module ts {
22282250

22292251
function dispose(): void {
22302252
if (program) {
2231-
forEach(program.getSourceFiles(),
2232-
(f) => { documentRegistry.releaseDocument(f.fileName, program.getCompilerOptions()); });
2253+
forEach(program.getSourceFiles(), f =>
2254+
documentRegistry.releaseDocument(f.fileName, program.getCompilerOptions()));
22332255
}
22342256
}
22352257

tests/baselines/reference/APISample_compile.js

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1872,25 +1872,17 @@ declare module "typescript" {
18721872
acquireDocument(fileName: string, compilationSettings: CompilerOptions, scriptSnapshot: IScriptSnapshot, version: string): SourceFile;
18731873
/**
18741874
* Request an updated version of an already existing SourceFile with a given fileName
1875-
* and compilationSettings. The update will intern call updateLanguageServiceSourceFile
1875+
* and compilationSettings. The update will in-turn call updateLanguageServiceSourceFile
18761876
* to get an updated SourceFile.
18771877
*
1878-
* Note: It is not allowed to call update on a SourceFile that was not acquired from this
1879-
* registry originally.
1880-
*
1881-
* @param sourceFile The original sourceFile object to update
18821878
* @param fileName The name of the file requested
18831879
* @param compilationSettings Some compilation settings like target affects the
18841880
* shape of a the resulting SourceFile. This allows the DocumentRegistry to store
18851881
* multiple copies of the same file for different compilation settings.
1886-
* @parm scriptSnapshot Text of the file. Only used if the file was not found
1887-
* in the registry and a new one was created.
1888-
* @parm version Current version of the file. Only used if the file was not found
1889-
* in the registry and a new one was created.
1890-
* @parm textChangeRange Change ranges since the last snapshot. Only used if the file
1891-
* was not found in the registry and a new one was created.
1882+
* @param scriptSnapshot Text of the file.
1883+
* @param version Current version of the file.
18921884
*/
1893-
updateDocument(sourceFile: SourceFile, fileName: string, compilationSettings: CompilerOptions, scriptSnapshot: IScriptSnapshot, version: string, textChangeRange: TextChangeRange): SourceFile;
1885+
updateDocument(fileName: string, compilationSettings: CompilerOptions, scriptSnapshot: IScriptSnapshot, version: string): SourceFile;
18941886
/**
18951887
* Informs the DocumentRegistry that a file is not needed any longer.
18961888
*

tests/baselines/reference/APISample_compile.types

Lines changed: 5 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -5849,36 +5849,24 @@ declare module "typescript" {
58495849

58505850
/**
58515851
* Request an updated version of an already existing SourceFile with a given fileName
5852-
* and compilationSettings. The update will intern call updateLanguageServiceSourceFile
5852+
* and compilationSettings. The update will in-turn call updateLanguageServiceSourceFile
58535853
* to get an updated SourceFile.
58545854
*
5855-
* Note: It is not allowed to call update on a SourceFile that was not acquired from this
5856-
* registry originally.
5857-
*
5858-
* @param sourceFile The original sourceFile object to update
58595855
* @param fileName The name of the file requested
58605856
* @param compilationSettings Some compilation settings like target affects the
58615857
* shape of a the resulting SourceFile. This allows the DocumentRegistry to store
58625858
* multiple copies of the same file for different compilation settings.
5863-
* @parm scriptSnapshot Text of the file. Only used if the file was not found
5864-
* in the registry and a new one was created.
5865-
* @parm version Current version of the file. Only used if the file was not found
5866-
* in the registry and a new one was created.
5867-
* @parm textChangeRange Change ranges since the last snapshot. Only used if the file
5868-
* was not found in the registry and a new one was created.
5859+
* @param scriptSnapshot Text of the file.
5860+
* @param version Current version of the file.
58695861
*/
5870-
updateDocument(sourceFile: SourceFile, fileName: string, compilationSettings: CompilerOptions, scriptSnapshot: IScriptSnapshot, version: string, textChangeRange: TextChangeRange): SourceFile;
5871-
>updateDocument : (sourceFile: SourceFile, fileName: string, compilationSettings: CompilerOptions, scriptSnapshot: IScriptSnapshot, version: string, textChangeRange: TextChangeRange) => SourceFile
5872-
>sourceFile : SourceFile
5873-
>SourceFile : SourceFile
5862+
updateDocument(fileName: string, compilationSettings: CompilerOptions, scriptSnapshot: IScriptSnapshot, version: string): SourceFile;
5863+
>updateDocument : (fileName: string, compilationSettings: CompilerOptions, scriptSnapshot: IScriptSnapshot, version: string) => SourceFile
58745864
>fileName : string
58755865
>compilationSettings : CompilerOptions
58765866
>CompilerOptions : CompilerOptions
58775867
>scriptSnapshot : IScriptSnapshot
58785868
>IScriptSnapshot : IScriptSnapshot
58795869
>version : string
5880-
>textChangeRange : TextChangeRange
5881-
>TextChangeRange : TextChangeRange
58825870
>SourceFile : SourceFile
58835871

58845872
/**

tests/baselines/reference/APISample_linter.js

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1903,25 +1903,17 @@ declare module "typescript" {
19031903
acquireDocument(fileName: string, compilationSettings: CompilerOptions, scriptSnapshot: IScriptSnapshot, version: string): SourceFile;
19041904
/**
19051905
* Request an updated version of an already existing SourceFile with a given fileName
1906-
* and compilationSettings. The update will intern call updateLanguageServiceSourceFile
1906+
* and compilationSettings. The update will in-turn call updateLanguageServiceSourceFile
19071907
* to get an updated SourceFile.
19081908
*
1909-
* Note: It is not allowed to call update on a SourceFile that was not acquired from this
1910-
* registry originally.
1911-
*
1912-
* @param sourceFile The original sourceFile object to update
19131909
* @param fileName The name of the file requested
19141910
* @param compilationSettings Some compilation settings like target affects the
19151911
* shape of a the resulting SourceFile. This allows the DocumentRegistry to store
19161912
* multiple copies of the same file for different compilation settings.
1917-
* @parm scriptSnapshot Text of the file. Only used if the file was not found
1918-
* in the registry and a new one was created.
1919-
* @parm version Current version of the file. Only used if the file was not found
1920-
* in the registry and a new one was created.
1921-
* @parm textChangeRange Change ranges since the last snapshot. Only used if the file
1922-
* was not found in the registry and a new one was created.
1913+
* @param scriptSnapshot Text of the file.
1914+
* @param version Current version of the file.
19231915
*/
1924-
updateDocument(sourceFile: SourceFile, fileName: string, compilationSettings: CompilerOptions, scriptSnapshot: IScriptSnapshot, version: string, textChangeRange: TextChangeRange): SourceFile;
1916+
updateDocument(fileName: string, compilationSettings: CompilerOptions, scriptSnapshot: IScriptSnapshot, version: string): SourceFile;
19251917
/**
19261918
* Informs the DocumentRegistry that a file is not needed any longer.
19271919
*

0 commit comments

Comments
 (0)