Replacement print function causes issues when called from coroutine

[JoshEngebretson]

Lua coroutine thread -> C TableFunction_print -> Lua tostring call causes issues as crosses the Lua/C/Lua barrier during coroutine, using lua_tostring instead of global "tostring" fixes the issue (and is a bit less complex)

https://github.com/rdeioris/LuaMachine/blob/master/Source/LuaMachine/Private/LuaState.cpp#L792

int ULuaState::TableFunction_print(lua_State *L)
{
    ULuaState* LuaState = ULuaState::GetFromExtraSpace(L);
    TArray<FString> Messages;

    int n = lua_gettop(L);
    for (int i = 1; i <= n; i++)
    {       
        Messages.Add(UTF8_TO_TCHAR(lua_tostring(L, i)));
    }
    LuaState->Log(FString::Join(Messages, TEXT("\t")));
    return 0;
}

[rdeioris]

Hi, do you have an example script to reproduce the bug ? i would like to try fixing it to continue supporting the tostring() as it allows the user to use the __tostring metamethod for both userdata and tables

[JoshEngebretson]

-- correctly prints 'userdata' as this is a UFunction defined in the LuaState Table TMap
print(type(spawnprojectile))

local coro = coroutine.create(function()

  coroutine.yield()
  coroutine.yield('one', 'two', 'three', 'four', 'five')
  coroutine.yield(17)

  -- prints false, instead of hello
  print("hello")

  -- prints 'nil' with current print C function, prints "string" with replacement method above so looks like something is going wrong with the UFunction wrapping in a coroutine too
  print(type(spawnprojectile))

  return coroutine.yield('ended')
end)

-- call as global per tick from level BP
unreal_tick = function()
  do
    local alive = coroutine.resume(coro)
    if alive then
      return print("alive")
    end
  end
end

[JoshEngebretson]

Unless I am missing something, tostring just calls luaB_tostring https://github.com/lua/lua/blob/c6f7181e910b6b2ff1346b5486a31be87b1da5af/lbaselib.c#L500

which is

static int luaB_tostring (lua_State *L) {
  luaL_checkany(L, 1);
  luaL_tolstring(L, 1, NULL);
  return 1;
}

lua_tostring is a define for lua_tolstring #define lua_tostring(L,i) lua_tolstring(L, (i), NULL)

TableFunction_print breaks the Lua coroutine -> C -> Lua -> C barrier limitation when using the "tostring" global and pcalling it. Lua uses setjmp/longjmp for coroutines, so the second Lua -> C call clobbers the stack frame.

Doesn't explain the weirdness with UFunction wrappiing and coroutines I'm seeing, for some reason accessing within a coroutine makes it think the userdata is a string, whether or not it has already been cached C++ side.

[rdeioris]

Your intuition was right, the stack was clobbered by the pcall usage. I have used lua_call instead (in the same way as the original print) and your code worked. Thanks a lot.

Note: in this way the user can continue overriding the global tostring()

[JoshEngebretson]

Ok, great and you're right on the tostring override flexibility 👍

Unfortunately, the first print outside the coroutine is correctly outputting type "userdata" for a blueprint UFunction exposed by the LuaState Table property. Whereas the print inside the coroutine, prints type 'string'

I can look into this a bit more and try and see why... from the sample code above:

-- correctly prints 'userdata' as this is a UFunction defined in the LuaState Table TMap
print(type(spawnprojectile))

 -- prints "string" when called from the coroutine
print(type(spawnprojectile))

[rdeioris]

I think i have found the issue: when a coroutine spawns it inherits the ULuaState instance from the parent, but the lua_State contained in it is different from the one of the thread/coroutine, basically destroying the stack. I am trying to find a solution

[JoshEngebretson]

Aha! I was just adding the stuff below the separator...

Wondering about the convert to LuaValue then immediately back to string, why not use Messages.Add(FString(UTF8_TO_TCHAR(s)))?

        const char *s = lua_tostring(L, -1);
        if (!s)
            return luaL_error(L, "'tostring must return a string to 'print'");
        FLuaValue Value(FString(UTF8_TO_TCHAR(s)));
        LuaState->Pop();
        Messages.Add(Value.ToString());

One other note, it might be good if pcall wasn't hiding behind LuaState::Call as there is an explicit LuaState::PCall and could cause some subtle hard to track down issues?

bool ULuaState::Call(int NArgs, FLuaValue& Value, int NRet)
{
    if (lua_pcall(L, NArgs, NRet, 0))
    {

---

I think there is still a problem with the stack:

-- correctly prints "hello"
   print("hello")

  -- does not print anything
  print("hello", "goodbye")

coro = coroutine.create(function()
  -- suspend function (without returning values)
  coroutine.yield()
  -- suspend returning 5 values
  coroutine.yield('one', 'two', 'three', 'four', 'five')

  -- correctly prints "hello"
  -- print("hello")

  -- does not print anything
  print("hello", "goodbye")

  -- suspend returning 1 value
  coroutine.yield(17)
  -- suspend returning 1 value
  coroutine.yield('ended')
end)

-- run the coroutine until the first yield
alive = coroutine.resume(coro)

-- ... until second yield (it returns five values)
alive, one, two, three, four, five = coroutine.resume(coro)

-- ... until third yield
alive, seventeen = coroutine.resume(coro)

-- ... the last yield
alive, ended = coroutine.resume(coro)

-- complete the coroutine
alive = coroutine.resume(coro)

-- this time 'alive' will be 'false' as the coroutine is 'dead' (it has finished its work)
alive = coroutine.resume(coro)

[JoshEngebretson]

Just a quick note that with f95db66eeb7ebf7da520f4b6649d3f773b6b9f27 the following moonscript using Lumen is working :)

Pretty involved diff, could be a few corner cases maybe with thread LuaValues

dbg = require "debugger"
sched = require "lumen.sched"

-- task emits 5 signals and pauses itself.
emitter_task = sched.new_task(->
    while true
        for i=1, 5
            sched.signal('ev', i)
            sched.sleep(1)
        sched.running_task\set_pause(true)
    )

-- if stop receiving signals, un-pause the emitter task.
sched.run(->
    waitd={timeout:5, 'ev'}
    while true
        ev, s = sched.wait(waitd)        
        if ev            
            spawnprojectile!
            print(s)
        else
            print ('wakeup!')            
            emitter_task\set_pause(false)
    )

emitter_task\run()

export unreal_tick = ->
    sched.step!

[rdeioris]

I have refactored the whole "sub-states" (thread/coroutines) management.

Can you try the latest code ?

Thanks

[JoshEngebretson]

I tested with f95db66eeb7ebf7da520f4b6649d3f773b6b9f27, check the last comment above :) I also tested debugging and the stacks/locals/upvalues inside the coroutines are correct. Nice work!

[rdeioris]

Note: i have added a new blueprint utility function LuaValueResumeMulti that can be used instead of manually calling coroutine.resume().