Skip to content

Commit

Permalink
Refactor ThreadStatus:
Browse files Browse the repository at this point in the history 
- Add `ThreadStatus::Running`
- Replace `ThreadStatus::Unresumable` with `ThreadStatus::Finished`
Change `Error::CoroutineInactive` to `Error::CoroutineUnresumable`
  • Loading branch information
@khvzak
khvzak committed Jul 31, 2024
1 parent 5acf9d7 commit b7d170a
Show file tree
Hide file tree
Showing 4 changed files with 43 additions and 45 deletions.
10 changes: 5 additions & 5 deletions src/error.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -98,17 +98,17 @@ pub enum Error {
/// A string containing more detailed error information.
message: Option<StdString>,
},
/// [`Thread::resume`] was called on an inactive coroutine.
/// [`Thread::resume`] was called on an unresumable coroutine.
///
/// A coroutine is inactive if its main function has returned or if an error has occurred inside
/// the coroutine. Already running coroutines are also marked as inactive (unresumable).
/// A coroutine is unresumable if its main function has returned or if an error has occurred inside
/// the coroutine. Already running coroutines are also marked as unresumable.
///
/// [`Thread::status`] can be used to check if the coroutine can be resumed without causing this
/// error.
///
/// [`Thread::resume`]: crate::Thread::resume
/// [`Thread::status`]: crate::Thread::status
CoroutineInactive,
CoroutineUnresumable,
/// An [`AnyUserData`] is not the expected type in a borrow.
///
/// This error can only happen when manually using [`AnyUserData`], or when implementing
Expand Down Expand Up @@ -259,7 +259,7 @@ impl fmt::Display for Error {
Some(ref message) => write!(fmt, " ({message})"),
}
}
Error::CoroutineInactive => write!(fmt, "cannot resume inactive coroutine"),
Error::CoroutineUnresumable => write!(fmt, "coroutine is non-resumable"),
Error::UserDataTypeMismatch => write!(fmt, "userdata is not expected type"),
Error::UserDataDestructed => write!(fmt, "userdata has been destructed"),
Error::UserDataBorrowError => write!(fmt, "error borrowing userdata"),
Expand Down
60 changes: 29 additions & 31 deletions src/thread.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,14 +30,14 @@ use {
/// Status of a Lua thread (coroutine).
#[derive(Debug, Copy, Clone, Eq, PartialEq)]
pub enum ThreadStatus {
/// The thread was just created, or is suspended because it has called `coroutine.yield`.
/// The thread was just created or is suspended (yielded).
///
/// If a thread is in this state, it can be resumed by calling [`Thread::resume`].
///
/// [`Thread::resume`]: crate::Thread::resume
Resumable,
/// Either the thread has finished executing, or the thread is currently running.
Unresumable,
/// The thread is currently running.
Running,
/// The thread has finished executing.
Finished,
/// The thread has raised a Lua error during execution.
Error,
}
Expand Down Expand Up @@ -108,7 +108,7 @@ impl Thread {
///
/// // The coroutine has now returned, so `resume` will fail
/// match thread.resume::<_, u32>(()) {
/// Err(Error::CoroutineInactive) => {},
/// Err(Error::CoroutineUnresumable) => {},
/// unexpected => panic!("unexpected result {:?}", unexpected),
/// }
/// # Ok(())
Expand All @@ -120,9 +120,8 @@ impl Thread {
R: FromLuaMulti,
{
let lua = self.0.lua.lock();

if unsafe { self.status_unprotected() } != ThreadStatus::Resumable {
return Err(Error::CoroutineInactive);
if self.status_inner(&lua) != ThreadStatus::Resumable {
return Err(Error::CoroutineUnresumable);
}

let state = lua.state();
Expand Down Expand Up @@ -170,25 +169,23 @@ impl Thread {

/// Gets the status of the thread.
pub fn status(&self) -> ThreadStatus {
let _guard = self.0.lua.lock();
unsafe { self.status_unprotected() }
self.status_inner(&self.0.lua.lock())
}

/// Gets the status of the thread without locking the Lua state.
pub(crate) unsafe fn status_unprotected(&self) -> ThreadStatus {
/// Gets the status of the thread (internal implementation).
pub(crate) fn status_inner(&self, lua: &RawLua) -> ThreadStatus {
let thread_state = self.state();
// FIXME: skip double lock
if thread_state == self.0.lua.lock().state() {
// The coroutine is currently running
return ThreadStatus::Unresumable;
if thread_state == lua.state() {
// The thread is currently running
return ThreadStatus::Running;
}
let status = ffi::lua_status(thread_state);
let status = unsafe { ffi::lua_status(thread_state) };
if status != ffi::LUA_OK && status != ffi::LUA_YIELD {
ThreadStatus::Error
} else if status == ffi::LUA_YIELD || ffi::lua_gettop(thread_state) > 0 {
} else if status == ffi::LUA_YIELD || unsafe { ffi::lua_gettop(thread_state) > 0 } {
ThreadStatus::Resumable
} else {
ThreadStatus::Unresumable
ThreadStatus::Finished
}
}

Expand Down Expand Up @@ -226,10 +223,11 @@ impl Thread {
#[cfg_attr(docsrs, doc(cfg(any(feature = "lua54", feature = "luau"))))]
pub fn reset(&self, func: crate::function::Function) -> Result<()> {
let lua = self.0.lua.lock();
let thread_state = self.state();
if thread_state == lua.state() {
if self.status_inner(&lua) == ThreadStatus::Running {
return Err(Error::runtime("cannot reset a running thread"));
}

let thread_state = self.state();
unsafe {
#[cfg(all(feature = "lua54", not(feature = "vendored")))]
let status = ffi::lua_resetthread(thread_state);
Expand Down Expand Up @@ -398,7 +396,7 @@ impl<R> Drop for AsyncThread<R> {
// For Lua 5.4 this also closes all pending to-be-closed variables
if !lua.recycle_thread(&mut self.thread) {
#[cfg(feature = "lua54")]
if self.thread.status_unprotected() == ThreadStatus::Error {
if self.thread.status_inner(&lua) == ThreadStatus::Error {
#[cfg(not(feature = "vendored"))]
ffi::lua_resetthread(self.thread.state());
#[cfg(feature = "vendored")]
Expand All @@ -416,13 +414,13 @@ impl<R: FromLuaMulti> Stream for AsyncThread<R> {

fn poll_next(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<Option<Self::Item>> {
let lua = self.thread.0.lua.lock();
if self.thread.status_inner(&lua) != ThreadStatus::Resumable {
return Poll::Ready(None);
}

let state = lua.state();
let thread_state = self.thread.state();
unsafe {
if self.thread.status_unprotected() != ThreadStatus::Resumable {
return Poll::Ready(None);
}

let _sg = StackGuard::new(state);
let _thread_sg = StackGuard::with_top(thread_state, 0);
let _wg = WakerGuard::new(&lua, cx.waker());
Expand Down Expand Up @@ -454,13 +452,13 @@ impl<R: FromLuaMulti> Future for AsyncThread<R> {

fn poll(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<Self::Output> {
let lua = self.thread.0.lua.lock();
if self.thread.status_inner(&lua) != ThreadStatus::Resumable {
return Poll::Ready(Err(Error::CoroutineUnresumable));
}

let state = lua.state();
let thread_state = self.thread.state();
unsafe {
if self.thread.status_unprotected() != ThreadStatus::Resumable {
return Poll::Ready(Err(Error::CoroutineInactive));
}

let _sg = StackGuard::new(state);
let _thread_sg = StackGuard::with_top(thread_state, 0);
let _wg = WakerGuard::new(&lua, cx.waker());
Expand Down
2 changes: 1 addition & 1 deletion tests/luau.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -362,7 +362,7 @@ fn test_interrupts() -> Result<()> {
let result: i32 = co.resume(())?;
assert_eq!(result, 6);
assert_eq!(yield_count.load(Ordering::Relaxed), 7);
assert_eq!(co.status(), ThreadStatus::Unresumable);
assert_eq!(co.status(), ThreadStatus::Finished);

//
// Test errors in interrupts
Expand Down
16 changes: 8 additions & 8 deletions tests/thread.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ fn test_thread() -> Result<()> {
assert_eq!(thread.resume::<_, i64>(3)?, 6);
assert_eq!(thread.status(), ThreadStatus::Resumable);
assert_eq!(thread.resume::<_, i64>(4)?, 10);
assert_eq!(thread.status(), ThreadStatus::Unresumable);
assert_eq!(thread.status(), ThreadStatus::Finished);

let accumulate = lua.create_thread(
lua.load(
Expand Down Expand Up @@ -85,17 +85,17 @@ fn test_thread() -> Result<()> {
assert_eq!(thread.resume::<_, u32>(43)?, 987);

match thread.resume::<_, u32>(()) {
Err(Error::CoroutineInactive) => {}
Err(Error::CoroutineUnresumable) => {}
Err(_) => panic!("resuming dead coroutine error is not CoroutineInactive kind"),
_ => panic!("resuming dead coroutine did not return error"),
}

// Already running thread must be unresumable
let thread = lua.create_thread(lua.create_function(|lua, ()| {
assert_eq!(lua.current_thread().status(), ThreadStatus::Unresumable);
assert_eq!(lua.current_thread().status(), ThreadStatus::Running);
let result = lua.current_thread().resume::<_, ()>(());
assert!(
matches!(result, Err(Error::CoroutineInactive)),
matches!(result, Err(Error::CoroutineUnresumable)),
"unexpected result: {result:?}",
);
Ok(())
Expand Down Expand Up @@ -128,7 +128,7 @@ fn test_thread_reset() -> Result<()> {
assert_eq!(thread.status(), ThreadStatus::Resumable);
assert_eq!(Arc::strong_count(&arc), 2);
thread.resume::<_, ()>(())?;
assert_eq!(thread.status(), ThreadStatus::Unresumable);
assert_eq!(thread.status(), ThreadStatus::Finished);
thread.reset(func.clone())?;
lua.gc_collect()?;
assert_eq!(Arc::strong_count(&arc), 1);
Expand All @@ -147,11 +147,11 @@ fn test_thread_reset() -> Result<()> {
// It's became possible to force reset thread by popping error object
assert!(matches!(
thread.status(),
ThreadStatus::Unresumable | ThreadStatus::Error
ThreadStatus::Finished | ThreadStatus::Error
));
// Would pass in 5.4.4
// assert!(thread.reset(func.clone()).is_ok());
// assert_eq!(thread.status(), ThreadStatus::Resumable);
assert!(thread.reset(func.clone()).is_ok());
assert_eq!(thread.status(), ThreadStatus::Resumable);
}
#[cfg(any(feature = "lua54", feature = "luau"))]
{
Expand Down

0 comments on commit b7d170a

Please sign in to comment.