Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WasmRefCell's borrow check should not throw #2412

Closed
mikialex opened this issue Jan 4, 2021 · 2 comments
Closed

WasmRefCell's borrow check should not throw #2412

mikialex opened this issue Jan 4, 2021 · 2 comments
Labels

Comments

@mikialex
Copy link

mikialex commented Jan 4, 2021

Summary

Accroding to rust-lang/rust#58874, wasm's stack unwinding is abort. So, once panic occured, the Ref / RefMut that borrowed from WasmRefCell 's drop cant be executed. After panic happend, we still want (try) wasm instance continue working like before, but the borrow count inside WasmRefCell is "dirty" and not reset by Ref / RefMut's drop, which cause a borrow fail throw to js side and terminate execution.

fn borrow_fail() -> ! {

I think maybe log an error to js and reset the borrow count is a better idea than directly throw js error. It can notify borrow error to users and more importantly act as a "panic recovery" .

Additional Details

I'am working on a rendering system in wasm, that every data store in wasm memory, and communicate to js side by handle object generated by wasm-bindgen. I would try very hard to avoid panic happen, but if I missed something I wish the damage is minimized, not crash the all system. I have replaced all refcell to customized not panic implmentation and check all possiible drop.

@alexcrichton
Copy link
Contributor

I agree that there's issues here related to unwinding and destructors, and I think it would be great to fix, but I'm not entirely sure what the fix would look like. If you have a solution in mind could you send a PR?

@ppershing
Copy link

Doesn't refcell have similar reasons to keep the dirty state as mutex? Basically, if you panic, you don't have guarantee that the state behind refcell isn't half modified, which in turn means it is unsafe to access it. Rust therefore poisons the value

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants