Skip to content

Commit 0fd7c12

Browse files
Prohibit nested VM operations
1 parent d16e07a commit 0fd7c12

File tree

3 files changed

+36
-13
lines changed

3 files changed

+36
-13
lines changed

packages/npm-packages/ruby-wasm-wasi/src/index.ts

Lines changed: 6 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -35,9 +35,8 @@ export class RubyVM {
3535
}
3636

3737
constructor() {
38-
// Wrap exported functions from Ruby VM to stop GC during the
39-
// function call if the call stack has sandwitched JS frames
40-
// like JS -> Ruby -> JS -> Ruby.
38+
// Wrap exported functions from Ruby VM to prohibit nested VM operation
39+
// if the call stack has sandwitched JS frames like JS -> Ruby -> JS -> Ruby.
4140
const proxyExports = (exports: RbAbi.RbAbiGuest) => {
4241
const excludedMethods: (keyof RbAbi.RbAbiGuest)[] = ["addToImports", "instantiate", "rbGcEnable", "rbGcDisable"];
4342
const excluded = ["constructor"].concat(excludedMethods);
@@ -49,16 +48,11 @@ export class RubyVM {
4948
const value = exports[key];
5049
if (typeof value === "function") {
5150
exports[key] = (...args: any[]) => {
52-
const shouldStopGC = this.interfaceState.hasJSFrameAfterRbFrame;
53-
let isAlreadyDisabled = false;
54-
if (shouldStopGC) {
55-
isAlreadyDisabled = exports.rbGcDisable();
51+
const isNestedVMCall = this.interfaceState.hasJSFrameAfterRbFrame;
52+
if (isNestedVMCall) {
53+
throw new Error("Nested VM operation is prohibited. If you are trying to call RubyVM API environment through Ruby, please call it from JS environment directly.");
5654
}
57-
const result = Reflect.apply(value, exports, args)
58-
if (shouldStopGC && !isAlreadyDisabled) {
59-
exports.rbGcEnable();
60-
}
61-
return result;
55+
return Reflect.apply(value, exports, args)
6256
}
6357
}
6458
}

packages/npm-packages/ruby-wasm-wasi/test/gc.test.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,10 @@ describe("GC integration", () => {
8383
expect(o2.call("hash").toString()).toBe(o3.call("hash").toString());
8484
});
8585

86-
test("stop GC while having a sandwitched JS frame", async () => {
86+
// TODO: Stopping GC while having a sandwitched JS frame is one of the options
87+
// to make nested eval safe. But there is still a problem with Fiber switching,
88+
// so this test is skipped for now.
89+
test.skip("stop GC while having a sandwitched JS frame", async () => {
8790
const vm = await initRubyVM();
8891
const o = vm.eval(`
8992
require "js"

packages/npm-packages/ruby-wasm-wasi/test/vm.test.ts

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,4 +119,30 @@ eval:11:in \`<main>'`);
119119
expect(vm.eval(`"hello".encoding.name`).toString()).toBe("UTF-8");
120120
expect(vm.eval(`__ENCODING__.name`).toString()).toBe("UTF-8");
121121
});
122+
123+
// FIXME: The below two tests are now failing because we cannot make it safe
124+
// to call eval within eval.
125+
// See https://github.com/ruby/ruby.wasm/issues/219#issuecomment-1551758711
126+
test.failing("Nested Fiber switch", async () => {
127+
const vm = await initRubyVM();
128+
const setVM = vm.eval(`proc { |vm| JS::RubyVM = vm }`)
129+
setVM.call("call", vm.wrap(vm))
130+
vm.eval(`
131+
$f0 = Fiber.new {
132+
JS::RubyVM.eval("$f1.resume")
133+
}
134+
$f1 = Fiber.new {}
135+
$f0.resume
136+
`)
137+
vm.eval(`puts $q.inspect`)
138+
})
139+
140+
test.failing("raise in nested eval", async () => {
141+
const vm = await initRubyVM();
142+
const setVM = vm.eval(`proc { |vm| JS::RubyVM = vm }`)
143+
setVM.call("call", vm.wrap(vm))
144+
expect(() => {
145+
vm.eval(`JS::RubyVM.eval("raise 'Exception from nested eval'")`)
146+
}).toThrowError("Exception from nested eval")
147+
})
122148
});

0 commit comments

Comments
 (0)