Skip to content

Commit

Permalink
Handle WebAssembly.RuntimeError by closing communication channel (#467
Browse files Browse the repository at this point in the history
)

* Reject channel writes if channel has been closed

* Report error and close on WebAssembly.RuntimeError
  • Loading branch information
georgestagg authored Aug 5, 2024
1 parent 5db0762 commit 7451e60
Show file tree
Hide file tree
Showing 6 changed files with 54 additions and 4 deletions.
5 changes: 5 additions & 0 deletions src/tests/webR/webr-main.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1034,6 +1034,11 @@ test('Close webR communication channel', async () => {
// Close the channel
tempR.close();
await expect(closedPromise).resolves.toEqual(true);

// Writing messages after closing the channel is an error
expect(() => tempR.writeConsole('foo <- 123')).toThrow(
"The webR communication channel has been closed"
);
});

test('Default and user provided REnv properties are merged', async () => {
Expand Down
16 changes: 14 additions & 2 deletions src/webR/chan/channel-postmessage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,10 @@ export class PostMessageChannelMain extends ChannelMain {
const initWorker = (worker: Worker) => {
this.#worker = worker;
this.#handleEventsFromWorker(worker);
this.close = () => worker.terminate();
this.close = () => {
worker.terminate();
this.putClosedMessage();
};
const msg = {
type: 'init',
data: { config, channelType: ChannelType.PostMessage },
Expand Down Expand Up @@ -263,7 +266,16 @@ export class PostMessageChannelWorker {
this.#dispatch(msg);
}
} catch (e) {
// Don't break the REPL loop on any other Wasm issues
// Close on unrecoverable error
if (e instanceof WebAssembly.RuntimeError) {
this.writeSystem({ type: 'console.error', data: e.message });
this.writeSystem({
type: 'console.error',
data: "An unrecoverable WebAssembly error has occurred, the webR worker will be closed.",
});
this.writeSystem({ type: 'close' });
}
// Don't break the REPL loop on other Wasm `Exception` errors
if (!(e instanceof (WebAssembly as any).Exception)) {
throw e;
}
Expand Down
14 changes: 13 additions & 1 deletion src/webR/chan/channel-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -297,7 +297,19 @@ export class ServiceWorkerChannelWorker implements ChannelWorker {
}

run(args: string[]) {
Module.callMain(args);
try{
Module.callMain(args);
} catch (e) {
if (e instanceof WebAssembly.RuntimeError) {
this.writeSystem({ type: 'console.error', data: e.message });
this.writeSystem({
type: 'console.error',
data: "An unrecoverable WebAssembly error has occurred, the webR worker will be closed.",
});
this.writeSystem({ type: 'close' });
}
throw e;
}
}

setInterrupt(interrupt: () => void) {
Expand Down
14 changes: 13 additions & 1 deletion src/webR/chan/channel-shared.ts
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,19 @@ export class SharedBufferChannelWorker implements ChannelWorker {
}

run(args: string[]) {
Module.callMain(args);
try{
Module.callMain(args);
} catch (e) {
if (e instanceof WebAssembly.RuntimeError) {
this.writeSystem({ type: 'console.error', data: e.message });
this.writeSystem({
type: 'console.error',
data: "An unrecoverable WebAssembly error has occurred, the webR worker will be closed.",
});
this.writeSystem({ type: 'close' });
}
throw e;
}
}

setInterrupt(interrupt: () => void) {
Expand Down
6 changes: 6 additions & 0 deletions src/webR/chan/channel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { promiseHandles, ResolveFn, RejectFn } from '../utils';
import { AsyncQueue } from './queue';
import { Message, newRequest, Response } from './message';
import { WebRPayload, WebRPayloadWorker, webRPayloadAsError } from '../payload';
import { WebRChannelError } from '../error';

// The channel structure is asymmetric:
//
Expand Down Expand Up @@ -34,6 +35,7 @@ export abstract class ChannelMain {
systemQueue = new AsyncQueue<Message>();

#parked = new Map<string, { resolve: ResolveFn; reject: RejectFn }>();
#closed = false;

abstract initialised: Promise<unknown>;
abstract close(): void;
Expand All @@ -56,6 +58,9 @@ export abstract class ChannelMain {
}

write(msg: Message): void {
if (this.#closed) {
throw new WebRChannelError("The webR communication channel has been closed.");
}
this.inputQueue.put(msg);
}

Expand All @@ -70,6 +75,7 @@ export abstract class ChannelMain {
}

protected putClosedMessage(): void {
this.#closed = true;
this.outputQueue.put({ type: 'closed' });
}

Expand Down
3 changes: 3 additions & 0 deletions src/webR/webr-main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -310,6 +310,9 @@ export class WebR {
case 'console.error':
console.error(msg.data);
break;
case 'close':
this.#chan.close();
break;
default:
throw new WebRError('Unknown system message type `' + msg.type + '`');
}
Expand Down

0 comments on commit 7451e60

Please sign in to comment.