Skip to content

Commit 5ea7a9c

Browse files
committed
wasm: avoid using a crashed Process
1 parent 0621f1e commit 5ea7a9c

File tree

2 files changed

+35
-5
lines changed

2 files changed

+35
-5
lines changed

wasm/quick-lint-js.js

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -389,22 +389,26 @@ class ProcessFactory {
389389
}
390390
}
391391

392+
let nextProcessIDForDebugging = 1;
393+
392394
// A WebAssembly instance.
393395
//
394396
// If a Process crashes, every DocumentForVSCode or DocumentForWebDemo
395397
// associated with its creating Process is tainted and should not be used
396398
// anymore.
397399
class Process {
398400
constructor(wasmInstance) {
401+
this._idForDebugging = nextProcessIDForDebugging++;
399402
this._wasmInstance = wasmInstance;
400403

404+
let process = this;
401405
function wrap(name) {
402406
if (!Object.prototype.hasOwnProperty.call(wasmInstance.exports, name)) {
403407
throw new TypeError(`WASM does not export function: ${name}`);
404408
}
405409
let func = wasmInstance.exports[name];
406410
return (...args) => {
407-
exports.maybeInjectFault(name);
411+
exports.maybeInjectFault(process, name);
408412
try {
409413
return func(...args);
410414
} catch (e) {
@@ -427,6 +431,10 @@ class Process {
427431
this._webDemoSetText = wrap("qljs_web_demo_set_text");
428432
}
429433

434+
toString() {
435+
return `Process(id=${this._idForDebugging})`;
436+
}
437+
430438
async createDocumentForVSCodeAsync() {
431439
return new DocumentForVSCode(this);
432440
}
@@ -454,8 +462,14 @@ class DocumentForVSCode {
454462
utf8ReplacementText.pointer,
455463
utf8ReplacementText.byteSize
456464
);
457-
} finally {
458465
utf8ReplacementText.dispose();
466+
} catch (e) {
467+
if (e instanceof ProcessCrashed) {
468+
// Don't touch the Process.
469+
} else {
470+
utf8ReplacementText.dispose();
471+
}
472+
throw e;
459473
}
460474
}
461475

@@ -656,7 +670,7 @@ function assertEqual(actual, expected) {
656670
//
657671
// Replace this function with a function which throws an exception to simulate
658672
// errors such as segfaults and OOMs.
659-
exports.maybeInjectFault = () => {};
673+
exports.maybeInjectFault = (_process, _functionName) => {};
660674

661675
// quick-lint-js finds bugs in JavaScript programs.
662676
// Copyright (C) 2020 Matthew "strager" Glazar

wasm/test-document.js

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -201,7 +201,12 @@ describe("DocumentLinter", () => {
201201

202202
let coinFlips;
203203
let rng = new ExhaustiveRNG();
204-
qljs.maybeInjectFault = (functionName) => {
204+
let crashedProcesses = new Set();
205+
qljs.maybeInjectFault = (process, functionName) => {
206+
assert.ok(
207+
!crashedProcesses.has(process),
208+
"Should not use previously-crashed process"
209+
);
205210
// TODO(strager): Figure out why qljs_vscode_create_document failures
206211
// cause this test to fail.
207212
// TODO(strager): Fix problems when qljs_vscode_destroy_document fails.
@@ -212,6 +217,7 @@ describe("DocumentLinter", () => {
212217
let shouldCrash = rng.nextCoinFlip();
213218
coinFlips.push(shouldCrash);
214219
if (shouldCrash) {
220+
crashedProcesses.add(process);
215221
throw new ProcessCrashed("(injected fault)");
216222
}
217223
}
@@ -268,6 +274,8 @@ describe("DocumentLinter", () => {
268274

269275
console.log(`coinFlips: ${coinFlips}`);
270276
rng.lap();
277+
278+
crashedProcesses.clear(); // Avoid out-of-memory errors.
271279
}
272280

273281
async function didLintingCrashAsync(callback) {
@@ -287,12 +295,18 @@ describe("DocumentLinter", () => {
287295
it("concurrent edits are applied in order of calls, with exhaustive fault injection", async () => {
288296
let coinFlips;
289297
let rng = new ExhaustiveRNG();
290-
function maybeInjectFaultWithExhaustiveRNG(functionName) {
298+
let crashedProcesses = new Set();
299+
function maybeInjectFaultWithExhaustiveRNG(process, functionName) {
300+
assert.ok(
301+
!crashedProcesses.has(process),
302+
"Should not use previously-crashed process"
303+
);
291304
// TODO(strager): Fix problems when qljs_vscode_destroy_document fails.
292305
if (functionName != "qljs_vscode_destroy_document") {
293306
let shouldCrash = rng.nextCoinFlip();
294307
coinFlips.push(shouldCrash);
295308
if (shouldCrash) {
309+
crashedProcesses.add(process);
296310
throw new qljs.ProcessCrashed("(injected fault)");
297311
}
298312
}
@@ -362,6 +376,8 @@ describe("DocumentLinter", () => {
362376
console.log(`coinFlips: ${coinFlips}`);
363377
rng.lap();
364378
qljs.maybeInjectFault = originalMaybeInjectFault;
379+
380+
crashedProcesses.clear(); // Avoid out-of-memory errors.
365381
}
366382
});
367383
});

0 commit comments

Comments
 (0)