Add Coco support (5.1 and JIT) about mlua HOT 4 CLOSED

I'm not sure that this is a good idea to add a new feature flag for a customised Lua, because of the complexity to manage them. Even now with 5 Lua versions supported (5.1, 5.2, 5.3, 5.4, jit) it's quite hard.

If Coco extension is really useful I might consider adding it to the vendored lua-5.1.5. Given that true coroutines are already supported by luajit maybe makes sense to add and test new functionality first, under #[cfg(all(feature = "luajit", feature = "vendored))]? And then consider extending it to the vendored lua-5.1.

from mlua.

I'm not sure that this is a good idea to add a new feature flag for a customised Lua, because of the complexity to manage them. Even now with 5 Lua versions supported (5.1, 5.2, 5.3, 5.4, jit) it's quite hard.

Currently, there is already a pull request open which adds the flag and alters (hopefully) all the code needed to be compatible with it. All that's left is to add Coco-specific functionality, which I might do as well. Keep in mind that since your enum is not #[non_exhaustive], adding a constant is a breaking change. I can't speak for the complexity of keeping this compatibility in the future, but adding basic support should be a couple clicks.

If Coco extension is really useful I might consider adding it to the vendored lua-5.1.5. Given that true coroutines are already supported by luajit maybe makes sense to add and test new functionality first, under #[cfg(all(feature = "luajit", feature = "vendored))]? And then consider extending it to the vendored lua-5.1.

It doesn't make sense for regular Lua 5.1 to suddenly gain Coco when it didn't have it before and none of the other "vanilla" versions have it. Adding a new version is the best way to introduce this functionality without surprising existing code. Coco is a popular and useful patch but it is not so ubiquitous that it deserves to be included unconditionally in the vanilla 5.1 distribution. Additionally including a specific version for Coco allows people to bring their own version and enable Coco-specific functionality once it is added.

As for Coco-specific functionality, there are only a few things that need to be done. Coco only adds a couple new pieces of functionality to the API.

from mlua.

Just tried to play with the coco extension using plain C and mlua with a new function try_yield().
My dirty patch to mlua uses local lua-src-rs crate with modifiedlua-5.1.5 sources for simplicity.
First attempt to test new functionality wasn't successful:

$ cargo test --test thread coroutine_yield --features lua51,vendored -- --nocapture
process didn't exit successfully: `.../mlua/target/debug/deps/thread-af2f8cb09bb556c2 coroutine_yield --nocapture` (signal: 11, SIGSEGV: invalid memory reference)

It failed on MacOS 10.15.7.

Nevertheless, I tested it using C and it also failed with Segmentation fault: 11:

#include <lua.h>
#include <lauxlib.h>
#include <lcoco.h>
#include <stdio.h>

int thread_fn(lua_State *L) {
    printf("before yield\n");
    lua_yield(L, 0);
    printf("after yield\n");
    return 0;
}

int main(int argc, char* argv[]) {
    lua_State *state = luaL_newstate();

    lua_State *thread = lua_newcthread(state, 8192);
    lua_pushcfunction(thread, thread_fn);
    lua_resume(thread, 0);
    printf("in yield\n");
    lua_resume(thread, 0);

    lua_close(state);
    return 0;
}

However, both examples works on linux.

I don't have much time to debug the extension but apparently it's deprecated and/or contain bugs. It also has removed from LuaJIT 2.1 (no lua_newcthread function) and the alternative way via coroutine.create() which I used, as expected, returns attempt to yield across C-call boundary error.

from mlua.

Just tried to play with the coco extension using plain C and mlua with a new function try_yield().
My dirty patch to mlua uses local lua-src-rs crate with modifiedlua-5.1.5 sources for simplicity.
First attempt to test new functionality wasn't successful:

$ cargo test --test thread coroutine_yield --features lua51,vendored -- --nocapture
process didn't exit successfully: `.../mlua/target/debug/deps/thread-af2f8cb09bb556c2 coroutine_yield --nocapture` (signal: 11, SIGSEGV: invalid memory reference)

It failed on MacOS 10.15.7.

Nevertheless, I tested it using C and it also failed with Segmentation fault: 11:

#include <lua.h>
#include <lauxlib.h>
#include <lcoco.h>
#include <stdio.h>

int thread_fn(lua_State *L) {
    printf("before yield\n");
    lua_yield(L, 0);
    printf("after yield\n");
    return 0;
}

int main(int argc, char* argv[]) {
    lua_State *state = luaL_newstate();

    lua_State *thread = lua_newcthread(state, 8192);
    lua_pushcfunction(thread, thread_fn);
    lua_resume(thread, 0);
    printf("in yield\n");
    lua_resume(thread, 0);

    lua_close(state);
    return 0;
}

However, both examples works on linux.

I don't have much time to debug the extension but apparently it's deprecated and/or contain bugs. It also has removed from LuaJIT 2.1 (no lua_newcthread function) and the alternative way via coroutine.create() which I used, as expected, returns attempt to yield across C-call boundary error.

That's... confusing, and concerning. I understand that Coco is a pretty major hack, but I didn't expect it to break just because you were running Catalina.

Well, if it doesn't work on the latest version of macOS, I suppose it's not acceptable into a Rust crate such as this one. I know it's really not your job to find out why a third-party, unsupported patch set isn't working, so I'll just go ahead and close this. Apologies if I wasted your time.

from mlua.