Skip to content

Commit ed45c6c

Browse files
committed
wasm: recover when process crashes for another document
If multiple DocumentLinter-s are open with a single Process, and the Process crashes for one DocumentLinter, the other DocumentLinter continues to use the Process. This is a problem because the Process might have a corrupted heap or some other problem. If a Process crashes, we want to stop using it for any linting. Before interacting with a Process, have DocumentLinter check if that process has ever crashed for any DocumentLinter. If so, lint with a new Process. This commit should not affect the VS Code extension which creates a new DocumentProcessManager per DocumentLinter.
1 parent bc858d0 commit ed45c6c

File tree

2 files changed

+272
-28
lines changed

2 files changed

+272
-28
lines changed

wasm/quick-lint-js.js

Lines changed: 58 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -69,13 +69,17 @@ class DocumentLinter {
6969
assertEqual(this._state, DocumentLinterState.NO_PARSER);
7070
this._state = DocumentLinterState.CREATING_PARSER;
7171
this._parserPromise = (async () => {
72+
let process;
7273
let parser;
7374
try {
74-
let process =
75-
await this._documentProcessManager.getOrCreateProcessAsync();
76-
parser = await process.createDocumentForVSCodeAsync();
75+
process = await this._documentProcessManager.getOrCreateProcessAsync();
76+
// BEGIN CRITICAL SECTION (no awaiting below)
77+
this._documentProcessManager.checkIfProcessCrashed(process);
78+
parser = process.createDocumentForVSCode();
79+
// END CRITICAL SECTION (no awaiting above)
7780
} catch (e) {
7881
if (e instanceof ProcessCrashed) {
82+
this._documentProcessManager.processCrashed(process);
7983
throw new LintingCrashed(e);
8084
} else {
8185
throw e;
@@ -117,9 +121,15 @@ class DocumentLinter {
117121
}
118122
if (this._parser !== null) {
119123
try {
124+
// BEGIN CRITICAL SECTION (no awaiting below)
125+
this._documentProcessManager.checkIfProcessCrashed(
126+
this._parser.process
127+
);
120128
this._parser.dispose();
129+
// END CRITICAL SECTION (no awaiting above)
121130
} catch (e) {
122131
if (e instanceof ProcessCrashed) {
132+
this._documentProcessManager.processCrashed(this._parser.process);
123133
// Ignore.
124134
} else {
125135
throw e;
@@ -196,17 +206,26 @@ class DocumentLinter {
196206
case DocumentLinterState.PARSER_LOADED:
197207
this._pendingChanges.push(...changes);
198208
try {
209+
this._documentProcessManager.checkIfProcessCrashed(
210+
this._parser.process
211+
);
199212
for (let change of this._pendingChanges) {
200213
this._parser.replaceText(change.range, change.text);
201214
}
202215
this._pendingChanges.length = 0;
203216
// END CRITICAL SECTION (no awaiting above)
204217

218+
// BEGIN CRITICAL SECTION (no awaiting below)
219+
this._documentProcessManager.checkIfProcessCrashed(
220+
this._parser.process
221+
);
205222
let diags = this._parser.lint();
223+
// END CRITICAL SECTION (no awaiting above)
206224
this._document.setDiagnostics(diags);
207225
} catch (e) {
208226
// END CRITICAL SECTION (no awaiting above)
209227
if (e instanceof ProcessCrashed) {
228+
this._documentProcessManager.processCrashed(this._parser.process);
210229
await this._recoverFromCrashAsync(e);
211230
} else {
212231
throw e;
@@ -235,6 +254,7 @@ class DocumentLinter {
235254
// BEGIN CRITICAL SECTION (no awaiting below)
236255
assertEqual(this._state, DocumentLinterState.PARSER_UNINITIALIZED);
237256
try {
257+
this._documentProcessManager.checkIfProcessCrashed(this._parser.process);
238258
this._parser.replaceText(
239259
{
240260
start: { line: 0, character: 0 },
@@ -246,10 +266,14 @@ class DocumentLinter {
246266
this._state = DocumentLinterState.PARSER_LOADED;
247267
// END CRITICAL SECTION (no awaiting above)
248268

269+
// BEGIN CRITICAL SECTION (no awaiting below)
270+
this._documentProcessManager.checkIfProcessCrashed(this._parser.process);
249271
let diags = this._parser.lint();
272+
// END CRITICAL SECTION (no awaiting above)
250273
this._document.setDiagnostics(diags);
251274
} catch (e) {
252275
if (e instanceof ProcessCrashed) {
276+
this._documentProcessManager.processCrashed(this._parser.process);
253277
await this._recoverFromCrashAsync(e);
254278
} else {
255279
throw e;
@@ -266,15 +290,17 @@ class DocumentLinter {
266290
this._state = DocumentLinterState.RECOVERING;
267291
this._recoveryPromise = (async () => {
268292
let diags;
293+
let process;
269294
try {
270-
let process =
271-
await this._documentProcessManager.getOrCreateDifferentProcessAsync(
272-
this._parser.process
273-
);
274-
let parser = await process.createDocumentForVSCodeAsync();
295+
process = await this._documentProcessManager.getOrCreateProcessAsync();
296+
// BEGIN CRITICAL SECTION (no awaiting below)
297+
this._documentProcessManager.checkIfProcessCrashed(process);
298+
let parser = process.createDocumentForVSCode();
299+
// END CRITICAL SECTION (no awaiting above)
275300

276301
// BEGIN CRITICAL SECTION (no awaiting below)
277302
assertEqual(this._state, DocumentLinterState.RECOVERING);
303+
this._documentProcessManager.checkIfProcessCrashed(process);
278304
parser.replaceText(
279305
{
280306
start: { line: 0, character: 0 },
@@ -287,12 +313,16 @@ class DocumentLinter {
287313
this._state = DocumentLinterState.PARSER_LOADED;
288314
// END CRITICAL SECTION (no awaiting above)
289315

316+
// BEGIN CRITICAL SECTION (no awaiting below)
317+
this._documentProcessManager.checkIfProcessCrashed(process);
290318
diags = parser.lint();
319+
// END CRITICAL SECTION (no awaiting above)
291320
} catch (e) {
292321
this._parser = null;
293322
this._parserPromise = null;
294323
this._state = DocumentLinterState.NO_PARSER;
295324
if (e instanceof ProcessCrashed) {
325+
this._documentProcessManager.processCrashed(process);
296326
throw new LintingCrashed(e);
297327
} else {
298328
throw e;
@@ -315,6 +345,25 @@ class DocumentProcessManager {
315345
this._process = null;
316346
}
317347

348+
processCrashed(process) {
349+
if (!(process instanceof Process)) {
350+
throw new TypeError(`Expected Process, but got ${process}`);
351+
}
352+
if (this._process === process) {
353+
this._process = null;
354+
this._processPromise = null;
355+
}
356+
}
357+
358+
checkIfProcessCrashed(process) {
359+
if (!(process instanceof Process)) {
360+
throw new TypeError(`Expected Process, but got ${process}`);
361+
}
362+
if (this._process !== process) {
363+
throw new ProcessCrashed();
364+
}
365+
}
366+
318367
async createNewProcessAsync() {
319368
let processFactory = await this._processFactoryPromise;
320369
let process = await processFactory.createProcessAsync();
@@ -332,17 +381,6 @@ class DocumentProcessManager {
332381
return await this._processPromise;
333382
}
334383

335-
async getOrCreateDifferentProcessAsync(existingProcess) {
336-
if (!(existingProcess instanceof Process)) {
337-
throw new TypeError(`Expected Process, but got ${existingProcess}`);
338-
}
339-
if (this._process !== existingProcess) {
340-
return this._process;
341-
}
342-
this._forgetCurrentProcess();
343-
return await this.getOrCreateProcessAsync();
344-
}
345-
346384
_forgetCurrentProcess() {
347385
this._process = null;
348386
this._processPromise = null;
@@ -493,7 +531,7 @@ class Process {
493531
return `Process(id=${this._idForDebugging})`;
494532
}
495533

496-
async createDocumentForVSCodeAsync() {
534+
createDocumentForVSCode() {
497535
return new DocumentForVSCode(this);
498536
}
499537

0 commit comments

Comments
 (0)