Drop `Lua::async_scope` as it's unsound

This commit is contained in:
Alex Orlenko 2023-02-12 22:25:10 +00:00
parent f52abf919e
commit b66bff9155
No known key found for this signature in database
GPG Key ID: 4C150C250863B96D
3 changed files with 5 additions and 246 deletions

View File

@ -56,7 +56,7 @@ use crate::{chunk::Compiler, types::VmState};
use {
crate::types::{AsyncCallback, AsyncCallbackUpvalue, AsyncPollUpvalue},
futures_core::{
future::{Future, LocalBoxFuture},
future::Future,
task::{Context, Poll, Waker},
},
futures_task::noop_waker,
@ -1887,27 +1887,6 @@ impl Lua {
f(&Scope::new(self))
}
/// An asynchronous version of [`scope`] that allows to create scoped async functions and
/// execute them.
///
/// Requires `feature = "async"`
///
/// [`scope`]: #method.scope
#[cfg(feature = "async")]
#[cfg_attr(docsrs, doc(cfg(feature = "async")))]
pub fn async_scope<'lua, 'scope, R, F, FR>(
&'lua self,
f: F,
) -> LocalBoxFuture<'scope, Result<R>>
where
'lua: 'scope,
R: 'static,
F: FnOnce(Scope<'lua, 'scope>) -> FR,
FR: 'scope + Future<Output = Result<R>>,
{
Box::pin(f(Scope::new(self)))
}
/// Attempts to coerce a Lua value into a String in a manner consistent with Lua's internal
/// behavior.
///

View File

@ -25,11 +25,7 @@ use crate::value::{FromLua, FromLuaMulti, IntoLua, IntoLuaMulti, MultiValue, Val
use crate::userdata::USER_VALUE_MAXSLOT;
#[cfg(feature = "async")]
use {
crate::types::{AsyncCallback, AsyncCallbackUpvalue, AsyncPollUpvalue},
futures_core::future::Future,
futures_util::future::{self, TryFutureExt},
};
use futures_core::future::Future;
/// Constructed by the [`Lua::scope`] method, allows temporarily creating Lua userdata and
/// callbacks that are not required to be Send or 'static.
@ -108,41 +104,6 @@ impl<'lua, 'scope> Scope<'lua, 'scope> {
})
}
/// Wraps a Rust async function or closure, creating a callable Lua function handle to it.
///
/// This is a version of [`Lua::create_async_function`] that creates a callback which expires on
/// scope drop. See [`Lua::scope`] and [`Lua::async_scope`] for more details.
///
/// Requires `feature = "async"`
///
/// [`Lua::create_async_function`]: crate::Lua::create_async_function
/// [`Lua::scope`]: crate::Lua::scope
/// [`Lua::async_scope`]: crate::Lua::async_scope
#[cfg(feature = "async")]
#[cfg_attr(docsrs, doc(cfg(feature = "async")))]
pub fn create_async_function<'callback, A, R, F, FR>(
&'callback self,
func: F,
) -> Result<Function<'lua>>
where
A: FromLuaMulti<'callback>,
R: IntoLuaMulti<'callback>,
F: 'scope + Fn(&'callback Lua, A) -> FR,
FR: 'callback + Future<Output = Result<R>>,
{
unsafe {
self.create_async_callback(Box::new(move |lua, args| {
let args = match A::from_lua_multi(args, lua) {
Ok(args) => args,
Err(e) => return Box::pin(future::err(e)),
};
Box::pin(
func(lua, args).and_then(move |ret| future::ready(ret.into_lua_multi(lua))),
)
}))
}
}
/// Creates a Lua userdata object from a custom userdata type.
///
/// This is a version of [`Lua::create_userdata`] that creates a userdata which expires on
@ -579,64 +540,6 @@ impl<'lua, 'scope> Scope<'lua, 'scope> {
Ok(f)
}
#[cfg(feature = "async")]
unsafe fn create_async_callback<'callback>(
&self,
f: AsyncCallback<'callback, 'scope>,
) -> Result<Function<'lua>> {
let f = mem::transmute::<AsyncCallback<'callback, 'scope>, AsyncCallback<'lua, 'static>>(f);
let f = self.lua.create_async_callback(f)?;
// We need to pre-allocate strings to avoid failures in destructor.
let get_poll_str = self.lua.create_string("get_poll")?;
let poll_str = self.lua.create_string("poll")?;
let destructor: DestructorCallback = Box::new(move |f| {
let state = f.lua.state();
let _sg = StackGuard::new(state);
assert_stack(state, 5);
f.lua.push_ref(&f);
// We know the destructor has not run yet because we hold a reference to the callback.
// First, get the environment table
#[cfg(any(feature = "lua54", feature = "lua53", feature = "lua52"))]
ffi::lua_getupvalue(state, -1, 1);
#[cfg(any(feature = "lua51", feature = "luajit", feature = "luau"))]
ffi::lua_getfenv(state, -1);
// Second, get the `get_poll()` closure using the corresponding key
f.lua.push_ref(&get_poll_str.0);
ffi::lua_rawget(state, -2);
// Destroy all upvalues
ffi::lua_getupvalue(state, -1, 1);
let upvalue1 = take_userdata::<AsyncCallbackUpvalue>(state);
ffi::lua_pushnil(state);
ffi::lua_setupvalue(state, -2, 1);
ffi::lua_pop(state, 1);
let mut data: Vec<Box<dyn Any>> = vec![Box::new(upvalue1)];
// Finally, get polled future and destroy it
f.lua.push_ref(&poll_str.0);
if ffi::lua_rawget(state, -2) == ffi::LUA_TFUNCTION {
ffi::lua_getupvalue(state, -1, 1);
let upvalue2 = take_userdata::<AsyncPollUpvalue>(state);
ffi::lua_pushnil(state);
ffi::lua_setupvalue(state, -2, 1);
data.push(Box::new(upvalue2));
}
data
});
self.destructors
.borrow_mut()
.push((f.0.clone(), destructor));
Ok(f)
}
}
impl<'lua, 'scope> Drop for Scope<'lua, 'scope> {

View File

@ -1,19 +1,14 @@
#![cfg(feature = "async")]
use std::cell::Cell;
use std::rc::Rc;
use std::sync::{
atomic::{AtomicI64, AtomicU64, Ordering},
Arc,
};
use std::sync::atomic::{AtomicU64, Ordering};
use std::sync::Arc;
use std::time::Duration;
use futures_timer::Delay;
use futures_util::stream::TryStreamExt;
use mlua::{
Error, Function, Lua, LuaOptions, Result, StdLib, Table, TableExt, Thread, UserData,
UserDataMethods, Value,
Error, Function, Lua, LuaOptions, Result, StdLib, Table, TableExt, UserData, UserDataMethods,
};
#[tokio::test]
@ -463,121 +458,3 @@ async fn test_async_thread_error() -> Result<()> {
Ok(())
}
#[tokio::test]
async fn test_async_scope() -> Result<()> {
let ref lua = Lua::new();
let ref rc = Rc::new(Cell::new(0));
let fut = lua.async_scope(|scope| async move {
let f = scope.create_async_function(move |_, n: u64| {
let rc2 = rc.clone();
async move {
rc2.set(42);
Delay::new(Duration::from_millis(n)).await;
assert_eq!(Rc::strong_count(&rc2), 2);
Ok(())
}
})?;
lua.globals().set("f", f.clone())?;
assert_eq!(Rc::strong_count(rc), 1);
let _ = f.call_async::<u64, ()>(10).await?;
assert_eq!(Rc::strong_count(rc), 1);
// Create future in partialy polled state (Poll::Pending)
let g = lua.create_thread(f)?;
g.resume::<u64, ()>(10)?;
lua.globals().set("g", g)?;
assert_eq!(Rc::strong_count(rc), 2);
Ok(())
});
assert_eq!(Rc::strong_count(rc), 1);
let _ = fut.await?;
assert_eq!(Rc::strong_count(rc), 1);
match lua
.globals()
.get::<_, Function>("f")?
.call_async::<_, ()>(10)
.await
{
Err(Error::CallbackError { ref cause, .. }) => match cause.as_ref() {
Error::CallbackDestructed => {}
e => panic!("expected `CallbackDestructed` error cause, got {:?}", e),
},
r => panic!("improper return for destructed function: {:?}", r),
};
match lua.globals().get::<_, Thread>("g")?.resume::<_, Value>(()) {
Err(Error::CallbackError { ref cause, .. }) => match cause.as_ref() {
Error::CallbackDestructed => {}
e => panic!("expected `CallbackDestructed` error cause, got {:?}", e),
},
r => panic!("improper return for destructed function: {:?}", r),
};
Ok(())
}
#[tokio::test]
async fn test_async_scope_userdata() -> Result<()> {
#[derive(Clone)]
struct MyUserData(Arc<AtomicI64>);
impl UserData for MyUserData {
fn add_methods<'lua, M: UserDataMethods<'lua, Self>>(methods: &mut M) {
methods.add_async_method("get_value", |_, data, ()| async move {
Delay::new(Duration::from_millis(10)).await;
Ok(data.0.load(Ordering::Relaxed))
});
methods.add_async_method("set_value", |_, data, n| async move {
Delay::new(Duration::from_millis(10)).await;
data.0.store(n, Ordering::Relaxed);
Ok(())
});
methods.add_async_function("sleep", |_, n| async move {
Delay::new(Duration::from_millis(n)).await;
Ok(format!("elapsed:{}ms", n))
});
}
}
let ref lua = Lua::new();
let ref arc = Arc::new(AtomicI64::new(11));
lua.async_scope(|scope| async move {
let ud = scope.create_userdata(MyUserData(arc.clone()))?;
lua.globals().set("userdata", ud)?;
lua.load(
r#"
assert(userdata:get_value() == 11)
userdata:set_value(12)
assert(userdata.sleep(5) == "elapsed:5ms")
assert(userdata:get_value() == 12)
"#,
)
.exec_async()
.await
})
.await?;
assert_eq!(Arc::strong_count(arc), 1);
match lua.load("userdata:get_value()").exec_async().await {
Err(Error::CallbackError { ref cause, .. }) => match cause.as_ref() {
Error::CallbackDestructed => {}
e => panic!("expected `CallbackDestructed` error cause, got {:?}", e),
},
r => panic!("improper return for destructed userdata: {:?}", r),
};
Ok(())
}