Skip to content

Commit

Permalink
Use main thread of current Lua state for callbacks, when known
Browse files Browse the repository at this point in the history
If the library is opened from a Lua thread (coroutine) that is not the main
one, the stored L would be to that thread, which may become suspended (by
calling yield). It is unsafe to call functions on suspended Lua threads, which
luv does a lot (the stored L is used for callbacks).

The main thread of a Lua state can never yield, so it is always safe to call
callbacks on.

This commit ensures the main thread is stored in the luv context instead of
the current thread, when we are able to calculate it (LUA_RIDX_MAINTHREAD was
added in Lua 5.2, and I have not found a way to determine the main thread in
Lua 5.1).

This fixes #503
  • Loading branch information
mwild1 committed Nov 15, 2024
1 parent e2d3d18 commit ba4589c
Show file tree
Hide file tree
Showing 2 changed files with 61 additions and 1 deletion.
11 changes: 10 additions & 1 deletion src/luv.c
Original file line number Diff line number Diff line change
Expand Up @@ -835,6 +835,15 @@ static int loop_gc(lua_State *L) {
}

LUALIB_API int luaopen_luv (lua_State* L) {
#ifdef LUA_RIDX_MAINTHREAD
// Lua 5.2+ - resolve the main thread of the current Lua state, even if
// we were loaded from a different thread (which may become suspended/dead).
lua_geti(L, LUA_REGISTRYINDEX, LUA_RIDX_MAINTHREAD);
lua_State* ctxL = lua_tothread(L, -1);
lua_pop(L, 1);
#else
lua_State* ctxL = L;
#endif
luv_ctx_t* ctx = luv_context(L);

luaL_newlib(L, luv_functions);
Expand Down Expand Up @@ -862,7 +871,7 @@ LUALIB_API int luaopen_luv (lua_State* L) {
lua_rawset(L, -3);

ctx->loop = loop;
ctx->L = L;
ctx->L = ctxL;
ctx->mode = -1;

ret = uv_loop_init(loop);
Expand Down
51 changes: 51 additions & 0 deletions tests/test-coroutines-require.lua
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
-- This is a standalone test because it specifically requires luv to
-- be initially require()d from a thread/coroutine.
-- Test for issue #503 (PR #734).

local thread = coroutine.create(function ()
local uv = require "luv"
coroutine.yield();
end);

-- Resume (start) thread, which will load luv
-- and then it will yield
coroutine.resume(thread);

-- thread where luv was initially loaded is now suspended

return require('lib/tap')(function (test)

if _VERSION == "Lua 5.1" then
-- Lua 5.1 and LuaJIT do not provide an API to determine the main
-- thread. Therefore it is inherently unsafe to require("luv") in
-- a coroutine.
test("callback should be in main thread", function(print,p,expect,uv)
print("Skipping! This test is expected to fail on Lua 5.1 and LuaJIT.");
end);
else
test("callback should be in main thread", function(print,p,expect,uv)
-- Now, in the main thread, load luv and create a timer
local t = uv.new_timer();
t:start(200, 0, expect(function ()
-- If luv calls this callback in the non-main (suspended) thread,
-- it violates a requirement specified in the Lua manual to only
-- call functions on active threads.

local our_thread, is_main = coroutine.running();

-- Basic assertion that we are not running in the suspended thread
assert(our_thread ~= thread)

-- How coroutine.running() reports "main thread" varies between
-- different Lua versions. This should handle them all.
assert(our_thread == nil or is_main == true)

-- If we were called in the wrong thread, this may segfault:
-- coroutine.resume(thread)
t:close();
end));
uv.run("default");
end);
end
end);

0 comments on commit ba4589c

Please sign in to comment.