Skip to content

Commit

Permalink
Merge pull request #1993 from Microsoft/incrementalCorruption
Browse files Browse the repository at this point in the history
Fix issue with cancellation causing corruption with source files.
  • Loading branch information
CyrusNajmabadi committed Feb 10, 2015
2 parents b277695 + b86ef44 commit 91dd9b6
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 6 deletions.
13 changes: 12 additions & 1 deletion src/compiler/parser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -736,6 +736,16 @@ module ts {
return parseSourceFile(sourceFile.fileName, newText, sourceFile.languageVersion, /*syntaxCursor*/ undefined, /*setNodeParents*/ true)
}

// Make sure we're not trying to incrementally update a source file more than once. Once
// we do an update the original source file is considered unusbale from that point onwards.
//
// This is because we do incremental parsing in-place. i.e. we take nodes from the old
// tree and give them new positions and parents. From that point on, trusting the old
// tree at all is not possible as far too much of it may violate invariants.
var incrementalSourceFile = <IncrementalNode><Node>sourceFile;
Debug.assert(!incrementalSourceFile.hasBeenIncrementallyParsed);
incrementalSourceFile.hasBeenIncrementallyParsed = true;

var oldText = sourceFile.text;
var syntaxCursor = createSyntaxCursor(sourceFile);

Expand Down Expand Up @@ -774,7 +784,7 @@ module ts {
//
// Also, mark any syntax elements that intersect the changed span. We know, up front,
// that we cannot reuse these elements.
updateTokenPositionsAndMarkElements(<IncrementalNode><Node>sourceFile,
updateTokenPositionsAndMarkElements(incrementalSourceFile,
changeRange.span.start, textSpanEnd(changeRange.span), textSpanEnd(textChangeRangeNewSpan(changeRange)), delta, oldText, newText, aggressiveChecks);

// Now that we've set up our internal incremental state just proceed and parse the
Expand Down Expand Up @@ -815,6 +825,7 @@ module ts {
}

interface IncrementalNode extends Node, IncrementalElement {
hasBeenIncrementallyParsed: boolean
}

interface IncrementalNodeArray extends NodeArray<IncrementalNode>, IncrementalElement {
Expand Down
11 changes: 6 additions & 5 deletions src/services/services.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2022,6 +2022,12 @@ module ts {
return;
}

// IMPORTANT - It is critical from this moment onward that we do not check
// cancellation tokens. We are about to mutate source files from a previous program
// instance. If we cancel midway through, we may end up in an inconsistent state where
// the program points to old source files that have been invalidated because of
// incremental parsing.

var oldSettings = program && program.getCompilerOptions();
var newSettings = hostCache.compilationSettings();
var changesInCompilationSettingsAffectSyntax = oldSettings && oldSettings.target !== newSettings.target;
Expand Down Expand Up @@ -2056,8 +2062,6 @@ module ts {
return;

function getOrCreateSourceFile(fileName: string): SourceFile {
cancellationToken.throwIfCancellationRequested();

// The program is asking for this file, check first if the host can locate it.
// If the host can not locate the file, then it does not exist. return undefined
// to the program to allow reporting of errors for missing files.
Expand Down Expand Up @@ -5363,9 +5367,6 @@ module ts {
cancellationToken.throwIfCancellationRequested();

var fileContents = sourceFile.text;

cancellationToken.throwIfCancellationRequested();

var result: TodoComment[] = [];

if (descriptors.length > 0) {
Expand Down

0 comments on commit 91dd9b6

Please sign in to comment.