mirror of
https://github.com/denoland/deno.git
synced 2024-10-29 08:58:01 -04:00
fix(ops): disallow auto-borrowing OpState across potential await point (#16952)
Fixes https://github.com/denoland/deno/issues/16934 Example compiler error: ``` error: mutable opstate is not supported in async ops --> core/ops_builtin.rs:122:1 | 122 | #[op] | ^^^^^ | = note: this error originates in the attribute macro `op` (in Nightly builds, run with -Z macro-backtrace for more info) ```
This commit is contained in:
parent
715f35d635
commit
55595ca1b7
10 changed files with 234 additions and 5 deletions
|
@ -1829,9 +1829,10 @@ impl Future for CallbackInfo {
|
|||
|
||||
#[op]
|
||||
fn op_ffi_unsafe_callback_ref(
|
||||
state: &mut deno_core::OpState,
|
||||
state: Rc<RefCell<deno_core::OpState>>,
|
||||
rid: ResourceId,
|
||||
) -> Result<impl Future<Output = Result<(), AnyError>>, AnyError> {
|
||||
let state = state.borrow();
|
||||
let callback_resource =
|
||||
state.resource_table.get::<UnsafeCallbackResource>(rid)?;
|
||||
|
||||
|
|
|
@ -1290,10 +1290,11 @@ where
|
|||
|
||||
#[op]
|
||||
fn op_flash_wait_for_listening(
|
||||
state: &mut OpState,
|
||||
state: Rc<RefCell<OpState>>,
|
||||
server_id: u32,
|
||||
) -> Result<impl Future<Output = Result<u16, AnyError>> + 'static, AnyError> {
|
||||
let mut listening_rx = {
|
||||
let mut state = state.borrow_mut();
|
||||
let flash_ctx = state.borrow_mut::<FlashContext>();
|
||||
let server_ctx = flash_ctx
|
||||
.servers
|
||||
|
@ -1312,10 +1313,11 @@ fn op_flash_wait_for_listening(
|
|||
|
||||
#[op]
|
||||
fn op_flash_drive_server(
|
||||
state: &mut OpState,
|
||||
state: Rc<RefCell<OpState>>,
|
||||
server_id: u32,
|
||||
) -> Result<impl Future<Output = Result<(), AnyError>> + 'static, AnyError> {
|
||||
let join_handle = {
|
||||
let mut state = state.borrow_mut();
|
||||
let flash_ctx = state.borrow_mut::<FlashContext>();
|
||||
flash_ctx
|
||||
.join_handles
|
||||
|
|
13
ops/lib.rs
13
ops/lib.rs
|
@ -193,7 +193,8 @@ fn codegen_v8_async(
|
|||
.inputs
|
||||
.iter()
|
||||
.map_while(|a| {
|
||||
(if is_v8 { scope_arg(a) } else { None }).or_else(|| opstate_arg(a))
|
||||
(if is_v8 { scope_arg(a) } else { None })
|
||||
.or_else(|| rc_refcell_opstate_arg(a))
|
||||
})
|
||||
.collect::<Vec<_>>();
|
||||
let rust_i0 = special_args.len();
|
||||
|
@ -291,6 +292,16 @@ fn opstate_arg(arg: &FnArg) -> Option<TokenStream2> {
|
|||
}
|
||||
}
|
||||
|
||||
fn rc_refcell_opstate_arg(arg: &FnArg) -> Option<TokenStream2> {
|
||||
match arg {
|
||||
arg if is_rc_refcell_opstate(arg) => Some(quote! { ctx.state.clone(), }),
|
||||
arg if is_mut_ref_opstate(arg) => Some(
|
||||
quote! { compile_error!("mutable opstate is not supported in async ops"), },
|
||||
),
|
||||
_ => None,
|
||||
}
|
||||
}
|
||||
|
||||
/// Generate the body of a v8 func for a sync op
|
||||
fn codegen_v8_sync(
|
||||
core: &TokenStream2,
|
||||
|
|
|
@ -650,7 +650,9 @@ impl Optimizer {
|
|||
let segment = single_segment(segments)?;
|
||||
match segment {
|
||||
// Is `T` a OpState?
|
||||
PathSegment { ident, .. } if ident == "OpState" => {
|
||||
PathSegment { ident, .. }
|
||||
if ident == "OpState" && !self.is_async =>
|
||||
{
|
||||
self.has_ref_opstate = true;
|
||||
}
|
||||
// Is `T` a str?
|
||||
|
|
11
ops/optimizer_tests/issue16934.expected
Normal file
11
ops/optimizer_tests/issue16934.expected
Normal file
|
@ -0,0 +1,11 @@
|
|||
=== Optimizer Dump ===
|
||||
returns_result: false
|
||||
has_ref_opstate: false
|
||||
has_rc_opstate: false
|
||||
has_fast_callback_option: false
|
||||
needs_fast_callback_option: false
|
||||
fast_result: None
|
||||
fast_parameters: []
|
||||
transforms: {}
|
||||
is_async: false
|
||||
fast_compatible: false
|
92
ops/optimizer_tests/issue16934.out
Normal file
92
ops/optimizer_tests/issue16934.out
Normal file
|
@ -0,0 +1,92 @@
|
|||
#[allow(non_camel_case_types)]
|
||||
///Auto-generated by `deno_ops`, i.e: `#[op]`
|
||||
///
|
||||
///Use `send_stdin::decl()` to get an op-declaration
|
||||
///you can include in a `deno_core::Extension`.
|
||||
pub struct send_stdin;
|
||||
#[doc(hidden)]
|
||||
impl send_stdin {
|
||||
pub fn name() -> &'static str {
|
||||
stringify!(send_stdin)
|
||||
}
|
||||
pub fn v8_fn_ptr<'scope>() -> deno_core::v8::FunctionCallback {
|
||||
use deno_core::v8::MapFnTo;
|
||||
Self::v8_func.map_fn_to()
|
||||
}
|
||||
pub fn decl<'scope>() -> deno_core::OpDecl {
|
||||
deno_core::OpDecl {
|
||||
name: Self::name(),
|
||||
v8_fn_ptr: Self::v8_fn_ptr(),
|
||||
enabled: true,
|
||||
fast_fn: None,
|
||||
is_async: true,
|
||||
is_unstable: false,
|
||||
is_v8: false,
|
||||
argc: 1usize,
|
||||
}
|
||||
}
|
||||
#[inline]
|
||||
#[allow(clippy::too_many_arguments)]
|
||||
async fn call(state: &mut OpState, cmd: String) -> Result<(), anyhow::Error> {
|
||||
let instance = state.borrow::<MinecraftInstance>().clone();
|
||||
instance.send_command(&cmd, CausedBy::Unknown).await?;
|
||||
Ok(())
|
||||
}
|
||||
pub fn v8_func<'scope>(
|
||||
scope: &mut deno_core::v8::HandleScope<'scope>,
|
||||
args: deno_core::v8::FunctionCallbackArguments,
|
||||
mut rv: deno_core::v8::ReturnValue,
|
||||
) {
|
||||
use deno_core::futures::FutureExt;
|
||||
let ctx = unsafe {
|
||||
&*(deno_core::v8::Local::<deno_core::v8::External>::cast(args.data()).value()
|
||||
as *const deno_core::_ops::OpCtx)
|
||||
};
|
||||
let op_id = ctx.id;
|
||||
let promise_id = args.get(0);
|
||||
let promise_id = deno_core::v8::Local::<
|
||||
deno_core::v8::Integer,
|
||||
>::try_from(promise_id)
|
||||
.map(|l| l.value() as deno_core::PromiseId)
|
||||
.map_err(deno_core::anyhow::Error::from);
|
||||
let promise_id: deno_core::PromiseId = match promise_id {
|
||||
Ok(promise_id) => promise_id,
|
||||
Err(err) => {
|
||||
deno_core::_ops::throw_type_error(
|
||||
scope,
|
||||
format!("invalid promise id: {}", err),
|
||||
);
|
||||
return;
|
||||
}
|
||||
};
|
||||
let arg_0 = match deno_core::v8::Local::<
|
||||
deno_core::v8::String,
|
||||
>::try_from(args.get(1usize as i32)) {
|
||||
Ok(v8_string) => deno_core::serde_v8::to_utf8(v8_string, scope),
|
||||
Err(_) => {
|
||||
return deno_core::_ops::throw_type_error(
|
||||
scope,
|
||||
format!("Expected string at position {}", 1usize),
|
||||
);
|
||||
}
|
||||
};
|
||||
let get_class = {
|
||||
let state = ::std::cell::RefCell::borrow(&ctx.state);
|
||||
state.tracker.track_async(op_id);
|
||||
state.get_error_class_fn
|
||||
};
|
||||
deno_core::_ops::queue_async_op(
|
||||
ctx,
|
||||
scope,
|
||||
false,
|
||||
async move {
|
||||
let result = Self::call(
|
||||
compile_error!("mutable opstate is not supported in async ops"),
|
||||
arg_0,
|
||||
)
|
||||
.await;
|
||||
(promise_id, op_id, deno_core::_ops::to_op_result(get_class, result))
|
||||
},
|
||||
);
|
||||
}
|
||||
}
|
11
ops/optimizer_tests/issue16934.rs
Normal file
11
ops/optimizer_tests/issue16934.rs
Normal file
|
@ -0,0 +1,11 @@
|
|||
async fn send_stdin(
|
||||
state: &mut OpState,
|
||||
cmd: String,
|
||||
) -> Result<(), anyhow::Error> {
|
||||
// https://github.com/denoland/deno/issues/16934
|
||||
//
|
||||
// OpState borrowed across await point is not allowed, as it will likely panic at runtime.
|
||||
let instance = state.borrow::<MinecraftInstance>().clone();
|
||||
instance.send_command(&cmd, CausedBy::Unknown).await?;
|
||||
Ok(())
|
||||
}
|
1
ops/optimizer_tests/issue16934_fast.expected
Normal file
1
ops/optimizer_tests/issue16934_fast.expected
Normal file
|
@ -0,0 +1 @@
|
|||
FastUnsupportedParamType
|
90
ops/optimizer_tests/issue16934_fast.out
Normal file
90
ops/optimizer_tests/issue16934_fast.out
Normal file
|
@ -0,0 +1,90 @@
|
|||
#[allow(non_camel_case_types)]
|
||||
///Auto-generated by `deno_ops`, i.e: `#[op]`
|
||||
///
|
||||
///Use `send_stdin::decl()` to get an op-declaration
|
||||
///you can include in a `deno_core::Extension`.
|
||||
pub struct send_stdin;
|
||||
#[doc(hidden)]
|
||||
impl send_stdin {
|
||||
pub fn name() -> &'static str {
|
||||
stringify!(send_stdin)
|
||||
}
|
||||
pub fn v8_fn_ptr<'scope>() -> deno_core::v8::FunctionCallback {
|
||||
use deno_core::v8::MapFnTo;
|
||||
Self::v8_func.map_fn_to()
|
||||
}
|
||||
pub fn decl<'scope>() -> deno_core::OpDecl {
|
||||
deno_core::OpDecl {
|
||||
name: Self::name(),
|
||||
v8_fn_ptr: Self::v8_fn_ptr(),
|
||||
enabled: true,
|
||||
fast_fn: None,
|
||||
is_async: true,
|
||||
is_unstable: false,
|
||||
is_v8: false,
|
||||
argc: 1usize,
|
||||
}
|
||||
}
|
||||
#[inline]
|
||||
#[allow(clippy::too_many_arguments)]
|
||||
async fn call(state: &mut OpState, v: i32) -> Result<(), anyhow::Error> {
|
||||
Ok(())
|
||||
}
|
||||
pub fn v8_func<'scope>(
|
||||
scope: &mut deno_core::v8::HandleScope<'scope>,
|
||||
args: deno_core::v8::FunctionCallbackArguments,
|
||||
mut rv: deno_core::v8::ReturnValue,
|
||||
) {
|
||||
use deno_core::futures::FutureExt;
|
||||
let ctx = unsafe {
|
||||
&*(deno_core::v8::Local::<deno_core::v8::External>::cast(args.data()).value()
|
||||
as *const deno_core::_ops::OpCtx)
|
||||
};
|
||||
let op_id = ctx.id;
|
||||
let promise_id = args.get(0);
|
||||
let promise_id = deno_core::v8::Local::<
|
||||
deno_core::v8::Integer,
|
||||
>::try_from(promise_id)
|
||||
.map(|l| l.value() as deno_core::PromiseId)
|
||||
.map_err(deno_core::anyhow::Error::from);
|
||||
let promise_id: deno_core::PromiseId = match promise_id {
|
||||
Ok(promise_id) => promise_id,
|
||||
Err(err) => {
|
||||
deno_core::_ops::throw_type_error(
|
||||
scope,
|
||||
format!("invalid promise id: {}", err),
|
||||
);
|
||||
return;
|
||||
}
|
||||
};
|
||||
let arg_0 = args.get(1usize as i32);
|
||||
let arg_0 = match deno_core::serde_v8::from_v8(scope, arg_0) {
|
||||
Ok(v) => v,
|
||||
Err(err) => {
|
||||
let msg = format!(
|
||||
"Error parsing args at position {}: {}", 1usize,
|
||||
deno_core::anyhow::Error::from(err)
|
||||
);
|
||||
return deno_core::_ops::throw_type_error(scope, msg);
|
||||
}
|
||||
};
|
||||
let get_class = {
|
||||
let state = ::std::cell::RefCell::borrow(&ctx.state);
|
||||
state.tracker.track_async(op_id);
|
||||
state.get_error_class_fn
|
||||
};
|
||||
deno_core::_ops::queue_async_op(
|
||||
ctx,
|
||||
scope,
|
||||
false,
|
||||
async move {
|
||||
let result = Self::call(
|
||||
compile_error!("mutable opstate is not supported in async ops"),
|
||||
arg_0,
|
||||
)
|
||||
.await;
|
||||
(promise_id, op_id, deno_core::_ops::to_op_result(get_class, result))
|
||||
},
|
||||
);
|
||||
}
|
||||
}
|
8
ops/optimizer_tests/issue16934_fast.rs
Normal file
8
ops/optimizer_tests/issue16934_fast.rs
Normal file
|
@ -0,0 +1,8 @@
|
|||
async fn send_stdin(state: &mut OpState, v: i32) -> Result<(), anyhow::Error> {
|
||||
// @test-attr:fast
|
||||
//
|
||||
// https://github.com/denoland/deno/issues/16934
|
||||
//
|
||||
// OpState borrowed across await point is not allowed, as it will likely panic at runtime.
|
||||
Ok(())
|
||||
}
|
Loading…
Reference in a new issue