Should LuaRef use the main thread instead?

[merlinblack]

I've run into an issue when a bound class that has a LuaRef as a member, gets created by a short lived Lua thread, and subsequently the LuaRef is assigned a value by another (could be the main one) thread. You get an error or a crash, depending if the original thread is still around.

I've got round this with Lua 5.2 by changing LuaRef to grab the associated main thread for the thread it's created with and use that for the m_L member.

Not hard to do with Lua 5.2 - however doing this with 5.1 requires manually storing the main thread somewhere (registry probably) which means you need a LuaBridge 'open' or 'init' function.

It does make sense to me to use the main thread - as the object that the LuaRef is holding a reference to is really associated with the main thread rather than any "sub" thread that may have created the LuaRef, or even the object itself.

What are peoples thoughts? I've done enough to make it work for me - but I'll figure something out for Lua 5.1 if the need is there.

Here's an example of how it goes splat...

#include <cstdlib>
#include <iostream>
#include <lua.hpp>
#include <LuaBridge.h>

using std::cout;
using std::endl;
using namespace luabridge;

struct SomeClass
{
    SomeClass( lua_State* L ) : overide(L) {}

    LuaRef overide;
    void SomeMember()
    {
        if( overide.isFunction() )
            overide();
        else
        {
            cout << "Overide is not a function" << endl;
            // Call C++ imp here.
        }
    }
};

void test( lua_State* L )
{
    getGlobalNamespace( L )
        .beginClass<SomeClass>("SomeClass")
        .addConstructor<void (*)(lua_State*)>()
        .addFunction( "SomeMember", &SomeClass::SomeMember )
        .addData( "SomeMemberOveride", &SomeClass::overide )
        .endClass();

    const char *source =
        "function test()\n"
        "    c:SomeMember()\n"
        "    c.SomeMemberOveride = MyHandler\n"
        "    c:SomeMember()\n"
        "    --This is pretty cool too!\n"
        "    c:SomeMemberOveride()\n"
        "    --Revert to C++ version\n"
        "    c.SomeMemberOveride = nil\n"
        "    c:SomeMember()\n"
        "    return\n"
        "end\n"
        "function MyHandler()\n"
        "    print 'SomeMember Overidden by Lua'\n"
        "end\n"
        "";

    // Use a thread to make Lua object
    const char *threadSource =
        "c = SomeClass()\n";

    cout << "Thread:\n" << threadSource << endl;

    lua_State* thread = lua_newthread( L );

    if( luaL_dostring( thread, threadSource ) )
    {
        cout << lua_tostring( thread, -1 ) << endl;
        lua_pop( thread, 1 );
        return;
    }

    // Pop and garbage collect thread.
    lua_pop( L, 1 );
    lua_gc( L, LUA_GCCOLLECT, 0 );

    cout << "Main:\n" << source << endl;

    if( luaL_dostring( L, source ) )
    {
        cout << lua_tostring( L, -1 ) << endl;
        lua_pop( L, 1 );
        return;
    }

    LuaRef test = getGlobal( L, "test" );

    test();

    return;
}

int main()
{
    lua_State* L = luaL_newstate();

    luaL_openlibs( L );

    test( L );

    lua_close( L );

    return EXIT_SUCCESS;
}

[vinniefalco]

I never considered threads when I ported LuaRef over

[merlinblack]

Nether did I when I wrote the original! Well I considered having different main threads, but I wasn't using co-routines at the time.

I got round it by making a get_main_thread function in LuaHelpers.h and sprinkled calls to it where LuaRef assigns it's m_L member from an 'outside' source.

inline lua_State* get_main_thread( lua_State* thread )
{
    lua_rawgeti( thread, LUA_REGISTRYINDEX, LUA_RIDX_MAINTHREAD );
    lua_State* L = lua_tothread( thread, -1 );
    lua_pop( thread, 1 );
    return L;
}

Although this only works in Lua 5.2. I guess a 5.1 version could use the registry too, and in the case it does not find the main thread (set elsewhere by a program using Lua Bridge) could just return the thread handed to it. It wouldn't break any existing code then.

[merlinblack]

Here's what I did...

https://github.com/merlinblack/LuaBridge/commit/56ec6794152811f695fd1f5fcc4c00daa739aff1

Not sure if it would apply cleanly though - my copy of the repo has Jakob Progsch's additions merged in. At any rate it does not handle Lua 5.1 yet.