From dcb989fd92d97b87476575f48115ad574f26dcb8 Mon Sep 17 00:00:00 2001 From: Filip Tibell Date: Sat, 19 Aug 2023 16:06:12 -0500 Subject: [PATCH] Implement basic abs path require, propagate async errors back to lua threads --- src/lune/globals/require/absolute.rs | 21 ++++++++++++++------- src/lune/globals/require/alias.rs | 9 ++++++--- src/lune/globals/require/builtin.rs | 9 ++++++--- src/lune/globals/require/context.rs | 20 +++++++++----------- src/lune/globals/require/mod.rs | 23 +++++++++++++---------- src/lune/globals/require/relative.rs | 9 ++++++--- src/lune/scheduler/impl_async.rs | 21 ++++++++++++++------- src/lune/scheduler/mod.rs | 14 ++++++++++++-- src/lune/scheduler/state.rs | 16 +++++++++++++++- 9 files changed, 95 insertions(+), 47 deletions(-) diff --git a/src/lune/globals/require/absolute.rs b/src/lune/globals/require/absolute.rs index 08f45ea..23b2a2c 100644 --- a/src/lune/globals/require/absolute.rs +++ b/src/lune/globals/require/absolute.rs @@ -2,12 +2,19 @@ use mlua::prelude::*; use super::context::*; -pub(super) async fn require<'lua>( - _lua: &'lua Lua, - _ctx: RequireContext, +pub(super) async fn require<'lua, 'ctx>( + lua: &'lua Lua, + ctx: &'ctx RequireContext, path: &str, -) -> LuaResult> { - Err(LuaError::runtime(format!( - "TODO: Support require for absolute paths (tried to require '{path}')" - ))) +) -> LuaResult> +where + 'lua: 'ctx, +{ + if ctx.is_cached(path)? { + ctx.get_from_cache(lua, path) + } else if ctx.is_pending(path)? { + ctx.wait_for_cache(lua, path).await + } else { + ctx.load(lua, path).await + } } diff --git a/src/lune/globals/require/alias.rs b/src/lune/globals/require/alias.rs index 0db3a07..e0d38eb 100644 --- a/src/lune/globals/require/alias.rs +++ b/src/lune/globals/require/alias.rs @@ -2,12 +2,15 @@ use mlua::prelude::*; use super::context::*; -pub(super) async fn require<'lua>( +pub(super) async fn require<'lua, 'ctx>( _lua: &'lua Lua, - _ctx: RequireContext, + _ctx: &'ctx RequireContext, alias: &str, name: &str, -) -> LuaResult> { +) -> LuaResult> +where + 'lua: 'ctx, +{ Err(LuaError::runtime(format!( "TODO: Support require for built-in libraries (tried to require '{name}' with alias '{alias}')" ))) diff --git a/src/lune/globals/require/builtin.rs b/src/lune/globals/require/builtin.rs index 377d947..e7a5171 100644 --- a/src/lune/globals/require/builtin.rs +++ b/src/lune/globals/require/builtin.rs @@ -2,11 +2,14 @@ use mlua::prelude::*; use super::context::*; -pub(super) async fn require<'lua>( +pub(super) async fn require<'lua, 'ctx>( _lua: &'lua Lua, - _ctx: RequireContext, + _ctx: &'ctx RequireContext, name: &str, -) -> LuaResult> { +) -> LuaResult> +where + 'lua: 'ctx, +{ Err(LuaError::runtime(format!( "TODO: Support require for built-in libraries (tried to require '{name}')" ))) diff --git a/src/lune/globals/require/context.rs b/src/lune/globals/require/context.rs index c2863ae..acf2c1c 100644 --- a/src/lune/globals/require/context.rs +++ b/src/lune/globals/require/context.rs @@ -23,17 +23,15 @@ impl RequireContext { context should be created per [`Lua`] struct, creating more than one context may lead to undefined require-behavior. */ - pub fn create(lua: &Lua) { - let this = Self { + pub fn new() -> Self { + Self { // TODO: Set to false by default, load some kind of config // or env var to check if we should be using absolute paths use_absolute_paths: true, working_directory: env::current_dir().expect("Failed to get current working directory"), cache_results: Arc::new(AsyncMutex::new(HashMap::new())), cache_pending: Arc::new(AsyncMutex::new(HashMap::new())), - }; - lua.set_named_registry_value(REGISTRY_KEY, this) - .expect("Failed to insert RequireContext into registry"); + } } /** @@ -102,9 +100,9 @@ impl RequireContext { path will first be transformed into an absolute path. */ pub fn get_from_cache<'lua>( - &'lua self, + &self, lua: &'lua Lua, - path: impl AsRef + 'lua, + path: impl AsRef, ) -> LuaResult> { let path = self.abs_path(path); @@ -136,9 +134,9 @@ impl RequireContext { path will first be transformed into an absolute path. */ pub async fn wait_for_cache<'lua>( - &'lua self, + &self, lua: &'lua Lua, - path: impl AsRef + 'lua, + path: impl AsRef, ) -> LuaResult> { let path = self.abs_path(path); let sched = lua @@ -166,9 +164,9 @@ impl RequireContext { path will first be transformed into an absolute path. */ pub async fn load<'lua>( - &'lua self, + &self, lua: &'lua Lua, - path: impl AsRef + 'lua, + path: impl AsRef, ) -> LuaResult> { let path = self.abs_path(path); let sched = lua diff --git a/src/lune/globals/require/mod.rs b/src/lune/globals/require/mod.rs index 7418a85..04e7b4a 100644 --- a/src/lune/globals/require/mod.rs +++ b/src/lune/globals/require/mod.rs @@ -11,31 +11,34 @@ mod builtin; mod relative; pub fn create(lua: &'static Lua) -> LuaResult> { - RequireContext::create(lua); - + lua.set_app_data(RequireContext::new()); lua.create_async_function(|lua, path: LuaString| async move { - let context = RequireContext::from(lua); - let path = path .to_str() .into_lua_err() .context("Failed to parse require path as string")? .to_string(); - if let Some(builtin_name) = path + let context = lua + .app_data_ref() + .expect("Failed to get RequireContext from app data"); + + let res = if let Some(builtin_name) = path .strip_prefix("@lune/") .map(|name| name.to_ascii_lowercase()) { - builtin::require(lua, context, &builtin_name).await + builtin::require(lua, &context, &builtin_name).await } else if let Some(aliased_path) = path.strip_prefix('@') { let (alias, name) = aliased_path.split_once('/').ok_or(LuaError::runtime( "Require with custom alias must contain '/' delimiter", ))?; - alias::require(lua, context, alias, name).await + alias::require(lua, &context, alias, name).await } else if context.use_absolute_paths() { - absolute::require(lua, context, &path).await + absolute::require(lua, &context, &path).await } else { - relative::require(lua, context, &path).await - } + relative::require(lua, &context, &path).await + }; + + res.clone() }) } diff --git a/src/lune/globals/require/relative.rs b/src/lune/globals/require/relative.rs index 08f45ea..def0c2f 100644 --- a/src/lune/globals/require/relative.rs +++ b/src/lune/globals/require/relative.rs @@ -2,11 +2,14 @@ use mlua::prelude::*; use super::context::*; -pub(super) async fn require<'lua>( +pub(super) async fn require<'lua, 'ctx>( _lua: &'lua Lua, - _ctx: RequireContext, + _ctx: &'ctx RequireContext, path: &str, -) -> LuaResult> { +) -> LuaResult> +where + 'lua: 'ctx, +{ Err(LuaError::runtime(format!( "TODO: Support require for absolute paths (tried to require '{path}')" ))) diff --git a/src/lune/scheduler/impl_async.rs b/src/lune/scheduler/impl_async.rs index fa3bf99..f57ec3b 100644 --- a/src/lune/scheduler/impl_async.rs +++ b/src/lune/scheduler/impl_async.rs @@ -37,13 +37,20 @@ where { let thread = thread.into_owned_lua_thread(self.lua)?; self.schedule_future(async move { - // TODO: Throw any error back to lua instead of panicking here - let rets = fut.await.expect("Failed to receive result"); - let rets = rets - .into_lua_multi(self.lua) - .expect("Failed to create return multi value"); - self.push_back(thread, rets) - .expect("Failed to schedule future thread"); + match fut.await.and_then(|rets| rets.into_lua_multi(self.lua)) { + Err(e) => { + self.state.set_lua_error(e); + // NOTE: We push the thread to the front of the scheduler + // to ensure that it runs first to be able to catch the + // stored error from within the scheduler lua interrupt + self.push_front(thread, ()) + .expect("Failed to schedule future thread"); + } + Ok(v) => { + self.push_back(thread, v) + .expect("Failed to schedule future thread"); + } + } }); Ok(()) diff --git a/src/lune/scheduler/mod.rs b/src/lune/scheduler/mod.rs index 398b1d5..520a323 100644 --- a/src/lune/scheduler/mod.rs +++ b/src/lune/scheduler/mod.rs @@ -44,13 +44,23 @@ pub(crate) struct Scheduler<'lua, 'fut> { impl<'lua, 'fut> Scheduler<'lua, 'fut> { pub fn new(lua: &'lua Lua) -> Self { - Self { + let this = Self { lua, state: Arc::new(SchedulerState::new()), threads: Arc::new(RefCell::new(VecDeque::new())), thread_senders: Arc::new(RefCell::new(HashMap::new())), futures: Arc::new(AsyncMutex::new(FuturesUnordered::new())), - } + }; + + // HACK: Propagate errors given to the scheduler back to their lua threads + // FUTURE: Do profiling and anything else we need inside of this interrupt + let state = this.state.clone(); + lua.set_interrupt(move |_| match state.get_lua_error() { + Some(e) => Err(e), + None => Ok(LuaVmState::Continue), + }); + + this } #[doc(hidden)] diff --git a/src/lune/scheduler/state.rs b/src/lune/scheduler/state.rs index 25df33b..3a0b12a 100644 --- a/src/lune/scheduler/state.rs +++ b/src/lune/scheduler/state.rs @@ -1,4 +1,9 @@ -use std::sync::atomic::{AtomicBool, AtomicU8, AtomicUsize, Ordering}; +use std::{ + cell::RefCell, + sync::atomic::{AtomicBool, AtomicU8, AtomicUsize, Ordering}, +}; + +use mlua::Error as LuaError; #[derive(Debug, Default)] pub struct SchedulerState { @@ -6,6 +11,7 @@ pub struct SchedulerState { exit_code: AtomicU8, num_resumptions: AtomicUsize, num_errors: AtomicUsize, + lua_error: RefCell>, } impl SchedulerState { @@ -41,4 +47,12 @@ impl SchedulerState { self.exit_state.store(true, Ordering::SeqCst); self.exit_code.store(code.into(), Ordering::SeqCst); } + + pub fn get_lua_error(&self) -> Option { + self.lua_error.take() + } + + pub fn set_lua_error(&self, e: LuaError) { + self.lua_error.replace(Some(e)); + } }