Skip to content

Commit 8433c75

Browse files
committed
wasm: simplify using DocumentProcessManager
DocumentProcessManager needs to be told when a Process crashes. Simplify this interface by instead making DocumentProcessManager ask a Process if it has crashed. This commit should not change behavior.
1 parent 79fdfee commit 8433c75

File tree

2 files changed

+43
-23
lines changed

2 files changed

+43
-23
lines changed

wasm/quick-lint-js.js

Lines changed: 14 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,6 @@ class DocumentLinter {
7676
parser = process.createDocumentForVSCode();
7777
} catch (e) {
7878
if (e instanceof ProcessCrashed) {
79-
this._documentProcessManager.processCrashed(process);
8079
throw new LintingCrashed(e);
8180
} else {
8281
throw e;
@@ -121,7 +120,6 @@ class DocumentLinter {
121120
this._parser.dispose();
122121
} catch (e) {
123122
if (e instanceof ProcessCrashed) {
124-
this._documentProcessManager.processCrashed(this._parser.process);
125123
// Ignore.
126124
} else {
127125
throw e;
@@ -209,7 +207,6 @@ class DocumentLinter {
209207
} catch (e) {
210208
// END CRITICAL SECTION (no awaiting above)
211209
if (e instanceof ProcessCrashed) {
212-
this._documentProcessManager.processCrashed(this._parser.process);
213210
await this._recoverFromCrashAsync(e);
214211
} else {
215212
throw e;
@@ -253,7 +250,6 @@ class DocumentLinter {
253250
this._document.setDiagnostics(diags);
254251
} catch (e) {
255252
if (e instanceof ProcessCrashed) {
256-
this._documentProcessManager.processCrashed(this._parser.process);
257253
await this._recoverFromCrashAsync(e);
258254
} else {
259255
throw e;
@@ -295,7 +291,6 @@ class DocumentLinter {
295291
this._parserPromise = null;
296292
this._state = DocumentLinterState.NO_PARSER;
297293
if (e instanceof ProcessCrashed) {
298-
this._documentProcessManager.processCrashed(process);
299294
throw new LintingCrashed(e);
300295
} else {
301296
throw e;
@@ -315,24 +310,12 @@ class DocumentProcessManager {
315310
this._processFactoryPromise = createProcessFactoryAsync();
316311
this._processesCreated = 0;
317312
this._processPromise = null;
318-
this._process = null;
319-
}
320-
321-
processCrashed(process) {
322-
if (!(process instanceof Process)) {
323-
throw new TypeError(`Expected Process, but got ${process}`);
324-
}
325-
if (this._process === process) {
326-
this._process = null;
327-
this._processPromise = null;
328-
}
329313
}
330314

331315
async createNewProcessAsync() {
332316
let processFactory = await this._processFactoryPromise;
333317
let process = await processFactory.createProcessAsync();
334318
this._processesCreated += 1;
335-
this._process = process;
336319
return process;
337320
}
338321

@@ -341,13 +324,17 @@ class DocumentProcessManager {
341324
if (this._processPromise === null) {
342325
this._processPromise = this.createNewProcessAsync();
343326
// END CRITICAL SECTION (no awaiting above)
327+
} else {
328+
// END CRITICAL SECTION (no awaiting above)
344329
}
345-
return await this._processPromise;
346-
}
347-
348-
_forgetCurrentProcess() {
349-
this._process = null;
350-
this._processPromise = null;
330+
let process = await this._processPromise;
331+
// BEGIN CRITICAL SECTION (no awaiting below)
332+
while (process.isTainted()) {
333+
this._processPromise = this.createNewProcessAsync();
334+
// END CRITICAL SECTION (no awaiting above)
335+
process = await this._processPromise;
336+
}
337+
return process;
351338
}
352339

353340
// For testing only.
@@ -501,6 +488,10 @@ class Process {
501488
this._webDemoSetText = wrap("qljs_web_demo_set_text");
502489
}
503490

491+
isTainted() {
492+
return this._crashedException !== null;
493+
}
494+
504495
toString() {
505496
return `Process(id=${this._idForDebugging})`;
506497
}

wasm/test-document.js

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -626,6 +626,35 @@ describe("DocumentLinter", () => {
626626

627627
assert.strictEqual(documentProcessManager.numberOfProcessesEverCreated, 1);
628628
});
629+
630+
it("crashing document does not prevent other document from linting successfully", async () => {
631+
let documentProcessManager = new DocumentProcessManager();
632+
633+
let document1 = new MockDocument("let x; let x;");
634+
let linter1 = disposeAfterTest(
635+
new DocumentLinter(document1, documentProcessManager)
636+
);
637+
638+
let document2 = new MockDocument("let y; let y;");
639+
let linter2 = disposeAfterTest(
640+
new DocumentLinter(document2, documentProcessManager)
641+
);
642+
643+
qljs.maybeInjectFault = (_process, functionName) => {
644+
if (functionName === "qljs_vscode_lint") {
645+
throw new ProcessCrashed("(injected fault)");
646+
}
647+
};
648+
await assert.rejects(async () => {
649+
await linter1.editorChangedVisibilityAsync();
650+
});
651+
652+
qljs.maybeInjectFault = originalMaybeInjectFault;
653+
await linter2.editorChangedVisibilityAsync();
654+
assert.deepStrictEqual(document2.getDiagnosticMessages(), [
655+
"redeclaration of variable: y",
656+
]);
657+
});
629658
});
630659

631660
describe("ExhaustiveRNG", () => {

0 commit comments

Comments
 (0)