Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add resume_error to Thread for luau #513

Closed
wants to merge 5 commits into from
Closed

Conversation

cheesycod
Copy link

@cheesycod cheesycod commented Jan 20, 2025

This adds support for the Luau specific lua_resumeerror to mlua dev branch as Thread::resume_error which allows directly resuming a thread with a bubbled up error which is at the top of the threads stack. This can be useful for many situations including resuming coroutine yields but as a bubbled up error with working pcall etc.

This C API is increasingly used by luau runtimes for error handling in threads which is how I found out about it:

void luauv_scheduler_spawnerror(lua_State* thread, lua_State* from, const char* fmt, ...) {
    va_list args;
    va_start(args, fmt);
    lua_pushvfstring(thread, fmt, args);
    va_end(args);

    int status = lua_resumeerror(thread, from);
    handlestatus(thread, status);
}

Hence why I made a pr for mlua to add it @khvzak

The API in question takes in a single argument of type IntoLua which is pushed to top of thread stack. Resume error is called and gettop is used to get the results and then IntoLuaMulti is used to convert to desired value.

For sample usage of this API:

                let fut = async move {
                    let res = fut.await;

                    match res {
                        Ok(res) => {
                            *taskmgr.inner.pending_asyncs.borrow_mut() -= 1;

                            #[cfg(not(feature = "fast"))]
                            let result = taskmgr.resume_thread_fast(th.clone(), res).await;
                            #[cfg(feature = "fast")]
                            let result = taskmgr.resume_thread_fast(&th, res);

                            taskmgr.inner.feedback.on_response(
                                "AsyncThread",
                                &taskmgr,
                                &th,
                                result,
                            );
                        }
                        Err(err) => {
                            *taskmgr.inner.pending_asyncs.borrow_mut() -= 1;

                            #[cfg(not(feature = "fast"))]
                            let result = taskmgr.resume_thread_fast(th.clone(), result).await;
                            #[cfg(feature = "fast")]
                            let result = th.resume_error::<LuaMultiValue>(err.to_string());

                            taskmgr.inner.feedback.on_response(
                                "AsyncThread.Resume",
                                &taskmgr,
                                &th,
                                Some(result),
                            );
                        }
                    }
                };

                #[cfg(not(feature = "send"))]
                async_executor.spawn_local(fut);
                #[cfg(feature = "send")]
                async_executor.spawn(fut);

                Ok(())
            },
        )?)?;

Without lua_resumeerror, doing the above requires monkeypatching coroutine.yield to get errors to correctly bubble which then breaks pcall. With lua_resumeerror, the above just works with fully working error propagation and pcall fully working as desired (and is also more performant).

Also, merging this PR would solve the original issue of #500 regarding erroring yielded threads. It turns out lua_resumeerror is the Luau solution for this.

@cheesycod
Copy link
Author

@khvzak This PR is now ready for review. Have moved the resume error test to the tests/thread.rs file like the other thread tests

khvzak added a commit that referenced this pull request Jan 28, 2025
This method uses Luau-specific C API extension `lua_resumerror`
that allow to throw an error immediately when resuming a thead.
Closes #500 #513
@cheesycod
Copy link
Author

Oh, its already merged to mlua main, am closing this PR

@cheesycod cheesycod closed this Jan 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant