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

Question about WeakRef basics #2201

Closed
whatisaphone opened this issue Jun 13, 2020 · 5 comments
Closed

Question about WeakRef basics #2201

whatisaphone opened this issue Jun 13, 2020 · 5 comments
Labels

Comments

@whatisaphone
Copy link

Let's say I have a JS function, not under my control. It takes a callback, and maybe calls it sometime in the future. Example:

function mushyJSFunction(callback) {
    if (Math.random() < 0.5) {
        setTimeout(callback, Math.random() * 10000000);
    }
}

As I understand it, using stable features today, calling this function from Rust with a Closure necessitates a memory leak on the Rust side. This is because you need to keep the Closure alive for it to be called, and there's no way to know when JS is finished with the closure.

Does the new WeakRef functionality let you fix that memory leak? (of course subject to GC nondeterminism)

Or to rephrase, can you pass "ownership" of a Rust value to the JS garbage collector, including eventually being dropped/deallocated properly across the FFI barrier?

@alexcrichton
Copy link
Contributor

There's a number of ways to manage memory here. For example Closure::once will automatically deallocate memory after it's invoked, so a leak isn't required. That being said, yes, with WeakRef and various support there the need to ever call an explicit free function from JS will go away. You'll still be able to do that for performance if desired, but otherwise we'll be able to enusre that memory is reclaimed even if an explicit free isn't required.

Until the WeakRef proposal is shipping in all browsers though we won't be able to do this. The only way to pass ownership to the JS GC is via weak refs.

@whatisaphone
Copy link
Author

Closure::once will automatically deallocate memory after it's invoked, so a leak isn't required

What about the case where the closure is never actually invoked (such as my example when Math.random() < 0.5)?

Until the WeakRef proposal is shipping in all browsers though we won't be able to do this. The only way to pass ownership to the JS GC is via weak refs.

Does today's WASM_BINDGEN_WEAKREF-gated feature aleady support this (in any browser)? If so, is there a code example showing how it's meant to work? I wasn't able to find one. The thing I got hung up on is that we have impl IntoWasmAbi for &Closure but we don't have impl IntoWasmAbi for Closure (without &), and if you only pass a reference it won't be callable after the function returns unless you Box::leak it.

Untested code, may not compile, but hopefully this shows what I mean:

#[wasm_bindgen]
extern {
    #[wasm_bindgen]
    fn mushyJSFunction(effect: &Closure<dyn Fn()>);
}

fn foo() {
    let closure = Closure::new(|| {});
    mushyJSFunction(&closure);
    // Here, `closure` is dropped and can no longer be called.
    // How do you persist it without `Box::leak`?
}

@alexcrichton
Copy link
Contributor

Yes Closure::once will leak memory if the closure isn't invoked.

The current support for WASM_BINDGEN_WEAKREF is relatively, well, weak. I haven't checked on it in a long time to see if it's still working in the browser. I'm pretty certain it's not well integrated with Closure, although that's just from a lack of work being done on it rather than somethign fundamental. At this time I think even with that feature enabled you'll still leak memory with closures.

@whatisaphone
Copy link
Author

relatively, well, weak

🙂

Thanks for the response! Sounds like I might be jumping the gun a bit. I'll wait until things have stabilized a little more

@alexcrichton
Copy link
Contributor

FWIW I updated this in #2248 and I think that closures should be handled now. Closures are still explicitly deallocated on Drop, but you can Closure::forget them to relinquish them to the JS GC basically.

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

2 participants