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

Allow erroring a Thread #500

Open
cheesycod opened this issue Dec 9, 2024 · 13 comments
Open

Allow erroring a Thread #500

cheesycod opened this issue Dec 9, 2024 · 13 comments

Comments

@cheesycod
Copy link

Can an API be added to allow erroring a yielded Thread etc. For use case, I need this for my scheduler to override the builtin behaviour of create_async_function

@mlua-rs mlua-rs deleted a comment Dec 9, 2024
@khvzak
Copy link
Member

khvzak commented Dec 9, 2024

I'm not sure I understand the question.
What is "erroring a yielded Thread"?

@cheesycod
Copy link
Author

cheesycod commented Dec 9, 2024

I'm not sure I understand the question. What is "erroring a yielded Thread"?

Actually never mind on this specific issue. I do have one other issue though. In order for my scheduler to correctly work, it needs to preload arguments to the thread (see https://github.com/Scythe-Technology/Zune/blob/199510ea089412e776d1c25d0374f86d6a58fadc/src/core/standard/task.zig#L110 for a zig example, it does a xpush). Can such an API be exposed in mlua if possible to allow preparing arguments directly in thread without resuming it.

If this isnt possible, could thread state be exposed somehow so xpush etc can be done manually

@khvzak
Copy link
Member

khvzak commented Dec 10, 2024

Have you tried Thread::into_async ?
Alternatively you can create a WrappedThread struct that store arguments together with thread.

OR to manipulate the thread stack directly you can always use raw api, through Lua::exec_raw

lua.exec_raw::<()>((&thread, arg1, arg2), |state| {
    mlua::ffi::lua_xmove(state, ffi::lua_tothread(state, -3), 2);
})?;

@cheesycod
Copy link
Author

cheesycod commented Dec 10, 2024

Have you tried Thread::into_async ? Alternatively you can create a WrappedThread struct that store arguments together with thread.

OR to manipulate the thread stack directly you can always use raw api, through Lua::exec_raw

lua.exec_raw::<()>((&thread, arg1, arg2), |state| {
    mlua::ffi::lua_xmove(state, ffi::lua_tothread(state, -3), 2);
})?;

In order to do the second one, I need access to pop_error, check_stack, StackGuard and protect_lua! in mlua to do manual thread resumption. I also may need access to mlua::state::RawLua as well (all of these are private)

@cheesycod
Copy link
Author

cheesycod commented Dec 10, 2024

@khvzak one more issue with thread resumption is that from_stack_multi takes in a RawLua but thats not actually exposed(?) publicly anywhere???

I've made a PR that solves all of my problems by simply exposing RawLua: #501

@khvzak
Copy link
Member

khvzak commented Dec 10, 2024

In order to do the second one, I need access to pop_error, check_stack, StackGuard and protect_lua! in mlua to do manual thread resumption. I also may need access to mlua::state::RawLua as well (all of these are private).

You should not need any of the internal functions. Lua::exec_raw provides interface to execute any raw Lua API functions including those that can trigger longjmp.

from_stack_multi takes in a RawLua but thats not actually exposed(?) publicly anywhere???

This is right, it's a hidden private api for internal use.

--
Also it's still not clear what problem are you trying to solve that require such low level interface.

@cheesycod
Copy link
Author

trigger

I am trying to preload arguments to thread stack and then resume the preloaded/prepared thread in my scheduler to better match Roblox's behaviour in their own task scheduler.

For example:

local task = task or require("@lune/task")
local thread = task.delay(1, function(foo)
    print(foo)
    print(coroutine.yield())
    print("c")
    print(coroutine.yield())
    print("e")
end, "b")

task.spawn(thread, "a")

task.delay(2, function()
    coroutine.resume(thread, "d")
end)

This should error due to delay adding/xpushing the "b" to thread stack and then returning the created thread. Then on resume, it should just resume the thread without any further stack moves

@khvzak
Copy link
Member

khvzak commented Dec 10, 2024

I'm not familiar with roblox interface, however this looks quite weird.

Are you sure that you can task.spawn thread what was delayed? What this will do?

@cheesycod
Copy link
Author

I'm not familiar with roblox interface, however this looks quite weird.

Are you sure that you can task.spawn thread what was delayed? What this will do?

The point here is to match the exact behaviour that pretty much every other (Roblox/Zune) scheduler does which is to error out here bc the argument "b" is first pushed, and then doing spawn here causes an error. Right now it continues but weirdly emits out a multivalue

@khvzak
Copy link
Member

khvzak commented Dec 10, 2024

The exact behaviour of official Roblox studio is panic with error:

task.spawn should not be called on a thread that is already 'delayed' in the task library 
cannot resume non-suspended coroutine 

Trying instead of task.spawn calling coroutine.resume(thread) fails with similar error:

cannot resume non-suspended coroutine

Which means it does not push anything to the thread.

This delay behaviour can be replicated roughly with

let delay = lua.create_function(|lua, (secs, f, args): (f32, Function, MultiValue)| {
    let t = unsafe {
        lua.exec_raw::<Thread>((), |state| {
            mlua::ffi::lua_newthread(state); // dead coroutine
        })?
    };
    let t2 = t.clone();
    tokio::spawn(async move {
        time::sleep(Duration::from_secs_f32(secs)).await;
        t2.reset(f).unwrap();
        t2.resume::<()>(args).unwrap();
    });
    Ok(t)
})?;

@cheesycod
Copy link
Author

The exact behaviour of official Roblox studio is panic with error:

task.spawn should not be called on a thread that is already 'delayed' in the task library 
cannot resume non-suspended coroutine 

Trying instead of task.spawn calling coroutine.resume(thread) fails with similar error:

cannot resume non-suspended coroutine

Which means it does not push anything to the thread.

This delay behaviour can be replicated roughly with

let delay = lua.create_function(|lua, (secs, f, args): (f32, Function, MultiValue)| {
    let t = unsafe {
        lua.exec_raw::<Thread>((), |state| {
            mlua::ffi::lua_newthread(state); // dead coroutine
        })?
    };
    let t2 = t.clone();
    tokio::spawn(async move {
        time::sleep(Duration::from_secs_f32(secs)).await;
        t2.reset(f).unwrap();
        t2.resume::<()>(args).unwrap();
    });
    Ok(t)
})?;

Oh thanks a lot for the help!

@cheesycod
Copy link
Author

cheesycod commented Dec 10, 2024

The exact behaviour of official Roblox studio is panic with error:

task.spawn should not be called on a thread that is already 'delayed' in the task library 
cannot resume non-suspended coroutine 

Trying instead of task.spawn calling coroutine.resume(thread) fails with similar error:

cannot resume non-suspended coroutine

Which means it does not push anything to the thread.

This delay behaviour can be replicated roughly with

let delay = lua.create_function(|lua, (secs, f, args): (f32, Function, MultiValue)| {
    let t = unsafe {
        lua.exec_raw::<Thread>((), |state| {
            mlua::ffi::lua_newthread(state); // dead coroutine
        })?
    };
    let t2 = t.clone();
    tokio::spawn(async move {
        time::sleep(Duration::from_secs_f32(secs)).await;
        t2.reset(f).unwrap();
        t2.resume::<()>(args).unwrap();
    });
    Ok(t)
})?;

BTW, someone tested it on that sample and they got 2 errors in Roblox studio involving trying to call a string argument as if it was the last pushed argument.

the first error is from the immediate resume (when using task.spawn or coroutine.resume), "cannot call string"
the second error is from the task scheduler, trying to continue the "delay", but since the thread already is in an error state, "cannot resume non-suspended coroutine" is correct
its not yielded, and its also not in an okay state

@khvzak
Copy link
Member

khvzak commented Dec 11, 2024

My bad.
When trying coroutine.resume(thread) the console prints only cannot resume non-suspended coroutine and nothing else.
After more detailed check I realised that the error in fact was from task.delay and it indeed pushes everything to the thread stack.

I personally think that roblox task interface is bad and counter-intuitive. I'm not sure yet how to replicate this behaviour.

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

No branches or pull requests

2 participants