Skip to content

Commit d16e07a

Browse files
Stop GC while having a sandwitched JS frame
This makes nested eval GC safe
1 parent 92b9ff8 commit d16e07a

File tree

8 files changed

+118
-4
lines changed

8 files changed

+118
-4
lines changed

ext/witapi/bindgen/rb-abi-guest.c

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -205,3 +205,13 @@ __attribute__((export_name("rb-vm-bugreport: func() -> ()")))
205205
void __wasm_export_rb_abi_guest_rb_vm_bugreport(void) {
206206
rb_abi_guest_rb_vm_bugreport();
207207
}
208+
__attribute__((export_name("rb-gc-enable: func() -> bool")))
209+
int32_t __wasm_export_rb_abi_guest_rb_gc_enable(void) {
210+
bool ret = rb_abi_guest_rb_gc_enable();
211+
return ret;
212+
}
213+
__attribute__((export_name("rb-gc-disable: func() -> bool")))
214+
int32_t __wasm_export_rb_abi_guest_rb_gc_disable(void) {
215+
bool ret = rb_abi_guest_rb_gc_disable();
216+
return ret;
217+
}

ext/witapi/bindgen/rb-abi-guest.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,8 @@ extern "C"
6868
void rb_abi_guest_rb_clear_errinfo(void);
6969
void rb_abi_guest_rstring_ptr(rb_abi_guest_rb_abi_value_t value, rb_abi_guest_string_t *ret0);
7070
void rb_abi_guest_rb_vm_bugreport(void);
71+
bool rb_abi_guest_rb_gc_enable(void);
72+
bool rb_abi_guest_rb_gc_disable(void);
7173
#ifdef __cplusplus
7274
}
7375
#endif

ext/witapi/bindgen/rb-abi-guest.wit

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,3 +18,6 @@ rb-clear-errinfo: func()
1818
rstring-ptr: func(value: rb-abi-value) -> string
1919

2020
rb-vm-bugreport: func()
21+
22+
rb-gc-enable: func() -> bool
23+
rb-gc-disable: func() -> bool

ext/witapi/witapi-core.c

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -303,4 +303,11 @@ void rb_vm_bugreport(const void *);
303303

304304
void rb_abi_guest_rb_vm_bugreport(void) { rb_vm_bugreport(NULL); }
305305

306+
bool rb_abi_guest_rb_gc_enable(void) {
307+
return rb_gc_enable() == Qtrue;
308+
}
309+
bool rb_abi_guest_rb_gc_disable(void) {
310+
return rb_gc_disable() == Qtrue;
311+
}
312+
306313
void Init_witapi(void) {}

packages/npm-packages/ruby-wasm-wasi/src/bindgen/rb-abi-guest.d.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,8 @@ export class RbAbiGuest {
8080
rbClearErrinfo(): void;
8181
rstringPtr(value: RbAbiValue): string;
8282
rbVmBugreport(): void;
83+
rbGcEnable(): boolean;
84+
rbGcDisable(): boolean;
8385
}
8486

8587
export class RbIseq {

packages/npm-packages/ruby-wasm-wasi/src/bindgen/rb-abi-guest.js

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { data_view, to_uint32, UTF8_DECODER, utf8_encode, UTF8_ENCODED_LEN, Slab } from './intrinsics.js';
1+
import { data_view, to_uint32, UTF8_DECODER, utf8_encode, UTF8_ENCODED_LEN, Slab, throw_invalid_bool } from './intrinsics.js';
22
export class RbAbiGuest {
33
constructor() {
44
this._resource0_slab = new Slab();
@@ -161,6 +161,16 @@ export class RbAbiGuest {
161161
rbVmBugreport() {
162162
this._exports['rb-vm-bugreport: func() -> ()']();
163163
}
164+
rbGcEnable() {
165+
const ret = this._exports['rb-gc-enable: func() -> bool']();
166+
const bool0 = ret;
167+
return bool0 == 0 ? false : (bool0 == 1 ? true : throw_invalid_bool());
168+
}
169+
rbGcDisable() {
170+
const ret = this._exports['rb-gc-disable: func() -> bool']();
171+
const bool0 = ret;
172+
return bool0 == 0 ? false : (bool0 == 1 ? true : throw_invalid_bool());
173+
}
164174
}
165175

166176
export class RbIseq {

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

Lines changed: 64 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import * as RbAbi from "./bindgen/rb-abi-guest";
22
import {
3+
RbJsAbiHost,
34
addRbJsAbiHostToImports,
45
JsAbiResult,
56
JsAbiValue,
@@ -29,9 +30,41 @@ export class RubyVM {
2930
private instance: WebAssembly.Instance | null = null;
3031
private transport: JsValueTransport;
3132
private exceptionFormatter: RbExceptionFormatter;
33+
private interfaceState: RbAbiInterfaceState = {
34+
hasJSFrameAfterRbFrame: false,
35+
}
3236

3337
constructor() {
34-
this.guest = new RbAbi.RbAbiGuest();
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.
41+
const proxyExports = (exports: RbAbi.RbAbiGuest) => {
42+
const excludedMethods: (keyof RbAbi.RbAbiGuest)[] = ["addToImports", "instantiate", "rbGcEnable", "rbGcDisable"];
43+
const excluded = ["constructor"].concat(excludedMethods);
44+
// wrap all methods in RbAbi.RbAbiGuest class
45+
for (const key of Object.getOwnPropertyNames(RbAbi.RbAbiGuest.prototype)) {
46+
if (excluded.includes(key)) {
47+
continue;
48+
}
49+
const value = exports[key];
50+
if (typeof value === "function") {
51+
exports[key] = (...args: any[]) => {
52+
const shouldStopGC = this.interfaceState.hasJSFrameAfterRbFrame;
53+
let isAlreadyDisabled = false;
54+
if (shouldStopGC) {
55+
isAlreadyDisabled = exports.rbGcDisable();
56+
}
57+
const result = Reflect.apply(value, exports, args)
58+
if (shouldStopGC && !isAlreadyDisabled) {
59+
exports.rbGcEnable();
60+
}
61+
return result;
62+
}
63+
}
64+
}
65+
return exports;
66+
}
67+
this.guest = proxyExports(new RbAbi.RbAbiGuest());
3568
this.transport = new JsValueTransport();
3669
this.exceptionFormatter = new RbExceptionFormatter();
3770
}
@@ -80,9 +113,28 @@ export class RubyVM {
80113
}
81114
};
82115
}
116+
// NOTE: The imported functions must disable Ruby GC if they call
117+
// Ruby API that may trigger GC. Otherwise, the GC may collect
118+
// objects that are still referenced by Wasm locals because Asyncify
119+
// cannot scan the Wasm stack above the JS frame.
120+
const proxyImports = (imports: RbJsAbiHost) => {
121+
for (const [key, value] of Object.entries(imports)) {
122+
if (typeof value === "function") {
123+
imports[key] = (...args: any[]) => {
124+
const oldValue = this.interfaceState.hasJSFrameAfterRbFrame;
125+
this.interfaceState.hasJSFrameAfterRbFrame = true;
126+
const result = Reflect.apply(value, imports, args);
127+
this.interfaceState.hasJSFrameAfterRbFrame = oldValue;
128+
return result;
129+
}
130+
}
131+
}
132+
return imports;
133+
};
134+
83135
addRbJsAbiHostToImports(
84136
imports,
85-
{
137+
proxyImports({
86138
evalJs: wrapTry((code) => {
87139
return Function(code)();
88140
}),
@@ -201,7 +253,7 @@ export class RubyVM {
201253
reflectSetPrototypeOf: function (target, prototype): boolean {
202254
throw new Error("Function not implemented.");
203255
},
204-
},
256+
}),
205257
(name) => {
206258
return this.instance.exports[name];
207259
}
@@ -294,6 +346,15 @@ export class RubyVM {
294346
}
295347
}
296348

349+
type RbAbiInterfaceState = {
350+
/**
351+
* Track if the last JS frame that was created by a Ruby frame
352+
* to determine if we need to disable Ruby GC during nested
353+
* JS->Ruby->JS->Ruby calls
354+
**/
355+
hasJSFrameAfterRbFrame: boolean;
356+
}
357+
297358
/**
298359
* Export a JS value held by the Ruby VM to the JS environment.
299360
* This is implemented in a dirty way since wit cannot reference resources

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

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,4 +82,23 @@ describe("GC integration", () => {
8282
expect(o1.call("hash").toString()).toBe(o2.call("hash").toString());
8383
expect(o2.call("hash").toString()).toBe(o3.call("hash").toString());
8484
});
85+
86+
test("stop GC while having a sandwitched JS frame", async () => {
87+
const vm = await initRubyVM();
88+
const o = vm.eval(`
89+
require "js"
90+
91+
JS.eval(<<~JS)
92+
return {
93+
takeVM(vm) {
94+
return vm.eval("GC.disable").toJS();
95+
}
96+
}
97+
JS
98+
`);
99+
const wasDisabled = o.call("takeVM", vm.wrap(vm));
100+
expect(wasDisabled.toJS()).toBe(true);
101+
const isNotEnabledBack = vm.eval("GC.enable");
102+
expect(isNotEnabledBack.toJS()).toBe(false);
103+
});
85104
});

0 commit comments

Comments
 (0)