LuaLanes / lanes

Lanes is a lightweight, native, lazy evaluating multithreading library for Lua 5.1 to 5.4.
Other
437 stars 94 forks source link

Lanes crash on require #221

Closed jjvbsag closed 3 months ago

jjvbsag commented 5 months ago

Working on Ubuntu 20.04.6 LTS Manually compiled lanes master from github (cd src && make), then copied to /usr/local/...

Running with installed lua works

lua -e'print(_VERSION)require"lanes"'
Lua 5.1

Running with installed luajit fails

luajit -e'print(jit.version)require"lanes"'
LuaJIT 2.1.0-beta3
luajit: bad light userdata pointer
stack traceback:
    [C]: at 0x7fbb74d8fc40
    [C]: in function 'require'
    /usr/local/share/lua/5.1/lanes.lua:38: in main chunk
    [C]: in function 'require'
    (command line):1: in main chunk
    [C]: at 0x55afa000e1d0

Running with luajit which comes with zerobrane studio, also failes

/opt/zbstudio/bin/linux/x64/lua -e'print(jit.version)require"lanes"'
LuaJIT 2.0.4
/opt/zbstudio/bin/linux/x64/lua: bad light userdata pointer
stack traceback:
    [C]: at 0x7f1c9cd09c40
    [C]: in function 'require'
    /usr/local/share/lua/5.1/lanes.lua:38: in main chunk
    [C]: in function 'require'
    (command line):1: in main chunk
    [C]: at 0x00404f08

Because already the require is failing, there is no chance to call configure to fix any options.

benoit-germain commented 5 months ago

I'll have to find time to setup a VM at home to see what's going on if you didn't already identified the issue.

jjvbsag commented 5 months ago

If you need any more details, just ask.

benoit-germain commented 4 months ago

Using VirtualBox 7.0.14r161095, I've setup a box using the latest Ubuntu 64 bits ubuntu-22.04.3-desktop-amd64.iso. I have compiled the latest luajit from github, resulting in luajit-2.1.1706185428. Then I cloned, built, and installed lanes from github as well, and everything is running just fine. The error you see is raised by luajit when we provide a pointer to lua_pushlightuserdata where bits 48-63 are non-0. I've checked the code, there are some pointers to C objects in Lanes implementation which are stored are light userdata, but there is no magic regarding them. Some light userdata values are also used as light keys, but they are specifically masked to account for LuaJIT behavior. So all in all, as far as I can tell, Lanes does everything right, and the issue is somewhere else.

jjvbsag commented 4 months ago

Thank you for the test. Ok, maybe the problem results in my machine, having 32GB of ram, which 2^35 bytes. This should not arise address bits above 48-63 but I'm not sure about virtual memory management in linux.vs.lua.vs.userdata. I will try on machines with less memory on the next days and come back to you.

jjvbsag commented 3 months ago

Next test. I instrumented lua_pushlightuserdata by changing Lanes/src/Makefile to LIBFLAG=-shared -Wl,--wrap=lua_pushlightuserdata and add

extern void __real_lua_pushlightuserdata(lua_State *L, void *p);
extern void __wrap_lua_pushlightuserdata(lua_State *L, void *p)
{
    printf("lua_pushlightuserdata(%p,%p)\n",L,p);
    __real_lua_pushlightuserdata(L,p);
}

to lanes.c Result

lua -e'print(_VERSION)require"lanes"'
Lua 5.1
lua_pushlightuserdata(0x55bfb09062a0,0x31cd24894eae8624)

luajit -e'print(jit.version)require"lanes"'
LuaJIT 2.1.0-beta3
lua_pushlightuserdata(0x405b0378,0x31cd24894eae8624)
luajit: bad light userdata pointer

0x31cd 2489 4eae 8624 indeed has bits set in 48-63 (0x31cd)

The confusing is: On my laptop with 16GB ram I get the same output, but there is no crash.

benoit-germain commented 3 months ago

This particular value is in tool.h

static DECLARE_CONST_UNIQUE_KEY( CONFIG_REGKEY, 0x31cd24894eae8624); // 'cancel_error' sentinel

It looks like LUAJIT_FLAVOR is not properly detected in your case (see compat.h). Maybe you are using a lanes built against PUC-Lua to run it with Luajit?

jjvbsag commented 3 months ago

AHHH!!! Thanks. Installed libluajit-5.1-dev (which was not installed before), rebuild and installed lanes and the problem has vanished.

Sorry for the inconvenience.

jjvbsag commented 3 months ago

Postscriptum:

Maybe you should add some kind of runtime error? Something like:

if "build with PUC-Lua" and "loaded by luajit" then error("not build for luajit") end

benoit-germain commented 3 months ago

Yeah, but how can I actually detect that? There is the global _VERSION, and the API function lua_version, but in both cases luajit duck-spoofs itself as version 501, just like PUC-Lua 5.1...

jjvbsag commented 3 months ago

What about lua_getglobal( L, "jit"); ? Should push a table, if luajit and nil if not. Maybe something like

#if LUAJIT_FLAVOR() == 0
    lua_getglobal( L, "jit");
    if( !lua_isnil( L, -1))
        luaL_error( L, "Lanes called from luajit but build for PUC-Lua");
    lua_pop( L, 1); // pop result of jit whatever it is
#endif

in luaopen_lanes_core ? Untested, just as a suggestion ...

jjvbsag commented 3 months ago

@benoit-germain : did you notice my last post ?

benoit-germain commented 3 months ago

Sure :-).