Skip to content

Commit 05f11d6

Browse files
Disable GC during sandwitched frame again
1 parent 51fe35c commit 05f11d6

File tree

8 files changed

+69
-12
lines changed

8 files changed

+69
-12
lines changed

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -205,6 +205,16 @@ __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+
}
208218
__attribute__((export_name("rb-set-should-prohibit-rewind: func(new-value: bool) -> bool")))
209219
int32_t __wasm_export_rb_abi_guest_rb_set_should_prohibit_rewind(int32_t arg) {
210220
bool ret = rb_abi_guest_rb_set_should_prohibit_rewind(arg);

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
bool rb_abi_guest_rb_set_should_prohibit_rewind(bool new_value);
7274
#ifdef __cplusplus
7375
}

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,4 +19,7 @@ rstring-ptr: func(value: rb-abi-value) -> string
1919

2020
rb-vm-bugreport: func()
2121

22+
rb-gc-enable: func() -> bool
23+
rb-gc-disable: func() -> bool
24+
2225
rb-set-should-prohibit-rewind: func(new-value: bool) -> bool

ext/witapi/witapi-core.c

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -329,6 +329,15 @@ void rb_vm_bugreport(const void *);
329329

330330
void rb_abi_guest_rb_vm_bugreport(void) { rb_vm_bugreport(NULL); }
331331

332+
bool rb_abi_guest_rb_gc_enable(void) {
333+
return rb_gc_enable() == Qtrue;
334+
}
335+
336+
VALUE rb_gc_disable_no_rest(void);
337+
bool rb_abi_guest_rb_gc_disable(void) {
338+
return rb_gc_disable_no_rest() == Qtrue;
339+
}
340+
332341
bool rb_abi_guest_rb_set_should_prohibit_rewind(bool value) {
333342
bool old = rb_should_prohibit_rewind;
334343
rb_should_prohibit_rewind = value;

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
rbSetShouldProhibitRewind(newValue: boolean): boolean;
8486
}
8587

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -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
rbSetShouldProhibitRewind(arg0) {
165175
const ret = this._exports['rb-set-should-prohibit-rewind: func(new-value: bool) -> bool'](arg0 ? 1 : 0);
166176
const bool0 = ret;

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

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ export class RubyVM {
3838
// Wrap exported functions from Ruby VM to prohibit nested VM operation
3939
// if the call stack has sandwitched JS frames like JS -> Ruby -> JS -> Ruby.
4040
const proxyExports = (exports: RbAbi.RbAbiGuest) => {
41-
const excludedMethods: (keyof RbAbi.RbAbiGuest)[] = ["addToImports", "instantiate", "rbSetShouldProhibitRewind"];
41+
const excludedMethods: (keyof RbAbi.RbAbiGuest)[] = ["addToImports", "instantiate", "rbSetShouldProhibitRewind", "rbGcDisable", "rbGcEnable"];
4242
const excluded = ["constructor"].concat(excludedMethods);
4343
// wrap all methods in RbAbi.RbAbiGuest class
4444
for (const key of Object.getOwnPropertyNames(RbAbi.RbAbiGuest.prototype)) {
@@ -49,15 +49,18 @@ export class RubyVM {
4949
if (typeof value === "function") {
5050
exports[key] = (...args: any[]) => {
5151
const isNestedVMCall = this.interfaceState.hasJSFrameAfterRbFrame;
52-
let oldValue = false;
5352
if (isNestedVMCall) {
54-
oldValue = this.guest.rbSetShouldProhibitRewind(true)
53+
const oldShouldProhibitRewind = this.guest.rbSetShouldProhibitRewind(true)
54+
const oldIsDisabledGc = this.guest.rbGcDisable()
55+
const result = Reflect.apply(value, exports, args)
56+
this.guest.rbSetShouldProhibitRewind(oldShouldProhibitRewind)
57+
if (!oldIsDisabledGc) {
58+
this.guest.rbGcEnable()
59+
}
60+
return result;
61+
} else {
62+
return Reflect.apply(value, exports, args)
5563
}
56-
const result = Reflect.apply(value, exports, args)
57-
if (isNestedVMCall) {
58-
this.guest.rbSetShouldProhibitRewind(oldValue)
59-
}
60-
return result;
6164
}
6265
}
6366
}

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

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

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 () => {
86+
test("stop GC while having a sandwitched JS frame", async () => {
9087
const vm = await initRubyVM();
9188
const o = vm.eval(`
9289
require "js"
@@ -101,7 +98,28 @@ describe("GC integration", () => {
10198
`);
10299
const wasDisabled = o.call("takeVM", vm.wrap(vm));
103100
expect(wasDisabled.toJS()).toBe(true);
101+
// Ensure that GC is enabled back
104102
const isNotEnabledBack = vm.eval("GC.enable");
105103
expect(isNotEnabledBack.toJS()).toBe(false);
104+
105+
// Re-start pending GC (including the incremental one)
106+
vm.eval("GC.start");
107+
108+
// Disable GC before the nested call
109+
vm.eval("GC.disable")
110+
const o2 = vm.eval(`
111+
JS.eval(<<~JS)
112+
return {
113+
takeVM(vm) {
114+
return vm.eval("GC.disable").toJS();
115+
}
116+
}
117+
JS
118+
`);
119+
const wasDisabled2 = o2.call("takeVM", vm.wrap(vm));
120+
expect(wasDisabled2.toJS()).toBe(true);
121+
// Ensure that GC is still disabled because it was disabled before the nested call
122+
const isNotEnabledBack2 = vm.eval("GC.enable");
123+
expect(isNotEnabledBack2.toJS()).toBe(true);
106124
});
107125
});

0 commit comments

Comments
 (0)