Refactor ThreadStatus:

- Add `ThreadStatus::Running`
- Replace `ThreadStatus::Unresumable` with `ThreadStatus::Finished`
Change `Error::CoroutineInactive` to `Error::CoroutineUnresumable`

Author: khvzak

src/error.rs

@@ -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
@@ -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"),

src/thread.rs

@@ -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,
 }
@@ -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(())
@@ -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();
@@ -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
         }
     }
 
@@ -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);
@@ -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")]
@@ -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());
@@ -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());

tests/luau.rs

@@ -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

tests/thread.rs

@@ -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(
@@ -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(())
@@ -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);
@@ -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"))]
     {