Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use main thread of current Lua state for callbacks, when known #734

Merged
merged 1 commit into from
Nov 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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);

Loading