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

Asynchronous functions can mutably access the same memory simultaneously #1340

Closed
Xaeroxe opened this issue Oct 11, 2022 · 0 comments · Fixed by #1339
Closed

Asynchronous functions can mutably access the same memory simultaneously #1340

Xaeroxe opened this issue Oct 11, 2022 · 0 comments · Fixed by #1339

Comments

@Xaeroxe
Copy link
Contributor

Xaeroxe commented Oct 11, 2022

Example project: napi-mut-twice.zip

This is in a similar vein to #1338. To run the example

yarn run build

This runs correctly, nothing is wrong.

yarn run x

This mutably borrows data twice, something is wrong.

yarn run x2

Expected behavior: I'm not sure. More on this later.

Actual behavior: In yarn run x we can see the code behaving in a normal and intended way. In x2 however, the tokio runtime has two mutable borrows to the same data, which results in undefined and unsound behavior. This is not allowed per Chapter 4 section 2 of the Rust Book.

So what should we do about this? I have some ideas with varying trade offs.

  1. Forbid &mut _ in async #[napi] methods.

    Pros: JavaScript never has to think about Rust borrow rules and restrictions
    Cons: Rust users can't use &mut _ with async functions anymore.

  2. Use RefCell or something like it to dynamically check borrow rules.

    Pros: We can keep writing async functions with &mut _ arguments.
    Cons: JavaScript users are now exposed to the restrictions of Rust and all that entails in a likely surprising way.

My personal preference is for option 1.

Xaeroxe added a commit to Xaeroxe/napi-rs that referenced this issue Nov 11, 2022
Brooooooklyn pushed a commit to Xaeroxe/napi-rs that referenced this issue Nov 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant