-
Notifications
You must be signed in to change notification settings - Fork 100
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
Allow #[cppgc] &mut T
in sync ops
#787
Comments
Copying over my comments from #793: I am not sure we can guarantee any safety at all with this: struct Wrap(usize);
#[op2(fast)]
async fn op_read(#[cppgc] wrap: &Wrap) {
println!("foo start: {}", wrap.0);
tokio::time::sleep(Duration::from_secs(1));
println!("foo end: {}", wrap.0);
}
#[op2(fast)]
fn op_write(#[cppgc] wrap: &mut Wrap) {
wrap.0 += 1;
println!("bar: {}", wrap.0);
} const wrap = op_make_wrap();
const promise = op_read(wrap); // foo start: 0
op_write(wrap); // bar: 1
await promise; // foo end: 1 Here we are explicitly violating one of the Rust memory safety axioms: a user must not be able to modify a value, while someone has an immutable reference ( I don't think this is fixable by making local changes to what are valid
And further, To then get access to cppgc values in async ops, one would not get a reference, but something like a That also does not seem viable, because we are now adding the overhead of So I have a final proposal that I think solves both problems: we have two types of These invariants can be enforced using the compiler (except for the "no mutable and immutable borrow for the same type in a single sync op", and the requirement that the op must not be reentrant. And following on from that: if you do need a mutable resource in an async op, you can use an immutable resource containing a RefCell. |
This will greatly simplify Zlib and net.Blocklist in Deno as we have to wrap them in RefCell for mutability. Should be pretty easy to implement
The text was updated successfully, but these errors were encountered: