From 25cc51765567328522bd926c101ef242deb1ec06 Mon Sep 17 00:00:00 2001 From: George Zhao Date: Fri, 26 May 2023 22:42:29 +0800 Subject: [PATCH 1/2] add tests about crash issues --- tests/manual-test-exit.lua | 22 +++++++++++ tests/manual-test-without-uv.run.lua | 4 ++ tests/test-loop.lua | 57 ++++++++++++++++++++++++++++ 3 files changed, 83 insertions(+) create mode 100644 tests/manual-test-exit.lua create mode 100644 tests/manual-test-without-uv.run.lua diff --git a/tests/manual-test-exit.lua b/tests/manual-test-exit.lua new file mode 100644 index 00000000..39db7e6d --- /dev/null +++ b/tests/manual-test-exit.lua @@ -0,0 +1,22 @@ +--come from https://github.com/luvit/luv/issues/599 + +-- run `lua manual-test-exit.lua || echo $?` +-- it shoud print `5` + +local uv = require('luv') + +local function setTimeout(callback, ms) + local timer = uv.new_timer() + timer:start(ms, 0, function() + timer:stop() + timer:close() + callback() + end) + return timer +end + +setTimeout(function() + os.exit(5, true) +end, 1000) + +uv.run() diff --git a/tests/manual-test-without-uv.run.lua b/tests/manual-test-without-uv.run.lua new file mode 100644 index 00000000..c205888d --- /dev/null +++ b/tests/manual-test-without-uv.run.lua @@ -0,0 +1,4 @@ +local uv = require('luv') + +local test = uv.new_pipe(false) +uv.close(test) diff --git a/tests/test-loop.lua b/tests/test-loop.lua index f0a4504c..cfe64b79 100644 --- a/tests/test-loop.lua +++ b/tests/test-loop.lua @@ -1,5 +1,18 @@ return require('lib/tap')(function (test) + local function get_cmd() + local i=1 + repeat + i=i-1 + until not arg[i] + return arg[i+1] + end + + local cmd = get_cmd() + local cwd = require('luv').cwd() + print("cmd: ", cmd) + print("cwd: ", cwd) + test("uv.loop_mode", function (print, p, expect, uv) assert(uv.loop_mode() == nil) local timer = uv.new_timer() @@ -10,4 +23,48 @@ return require('lib/tap')(function (test) end)) end) + test("issue #437, crash without uv.run", function (print, p, expect, uv) + local handle + local stdout = uv.new_pipe(false) + + handle = uv.spawn(cmd, { + args = { "tests/manual-test-without-uv.run.lua" }, + cwd = cwd, + stdio = {nil, stdout}, + }, + expect(function(status, signal) + print('#437', status, signal) + assert(status==0) + assert(signal==0) + handle:close() + end)) + + uv.read_start(stdout, expect(function (err, chunk) + p("stdout", {err=err,chunk=chunk}) + uv.close(stdout) + end)) + end) + + test("issue #599, crash during calling os.exit", function (print, p, expect, uv) + local handle + local stdout = uv.new_pipe(false) + + handle = uv.spawn(cmd, { + args = { "tests/manual-test-exit.lua" }, + cwd = cwd, + stdio = {nil, stdout}, + }, + expect(function(status, signal) + print('#599', status, signal) + assert(status==5) + assert(signal==0) + handle:close() + end)) + + uv.read_start(stdout, expect(function (err, chunk) + p("stdout", {err=err,chunk=chunk}) + uv.close(stdout) + end)) + end) + end) From d1f870cfc138cbda31f349cc5d35d58248648eef Mon Sep 17 00:00:00 2001 From: George Zhao Date: Sat, 27 May 2023 15:01:37 +0800 Subject: [PATCH 2/2] refactor handle gc, that maybe called before close_cb Lua `os.exit` prioritize `userdata` gc, cause `reference` failed. --- src/handle.c | 19 +++++++++++++++---- src/lhandle.c | 1 + tests/manual-test-exit.lua | 16 ++++++++-------- 3 files changed, 24 insertions(+), 12 deletions(-) diff --git a/src/handle.c b/src/handle.c index 9180da34..0977a565 100644 --- a/src/handle.c +++ b/src/handle.c @@ -88,13 +88,19 @@ static int luv_is_closing(lua_State* L) { return 1; } +static void luv_handle_free(uv_handle_t* handle); + static void luv_close_cb(uv_handle_t* handle) { lua_State* L; luv_handle_t* data = (luv_handle_t*)handle->data; if (!data) return; L = data->ctx->L; - luv_call_callback(L, data, LUV_CLOSED, 0); - luv_unref_handle(L, data); + if(data->ref > 0) { + luv_call_callback(L, data, LUV_CLOSED, 0); + luv_unref_handle(L, data); + } else { + luv_handle_free(handle); + } } static int luv_close(lua_State* L) { @@ -127,12 +133,13 @@ static void luv_gc_cb(uv_handle_t* handle) { static int luv_handle_gc(lua_State* L) { uv_handle_t** udata = (uv_handle_t**)lua_touserdata(L, 1); uv_handle_t* handle = *udata; + luv_handle_t* data = (luv_handle_t*)handle->data; // Only cleanup if the handle hasn't been cleaned up yet. - if (handle) { + if (data->ref == LUA_NOREF) { if (!uv_is_closing(handle)) { // If the handle is not closed yet, close it first before freeing memory. - uv_close(handle, luv_gc_cb); + uv_close(handle, luv_handle_free); } else { // Otherwise, free the memory right away. @@ -140,6 +147,10 @@ static int luv_handle_gc(lua_State* L) { } // Mark as cleaned up by wiping the dangling pointer. *udata = NULL; + } else { + // os.exit maybe cause gc before close_cb + // use LUA_REFNIL to tell close_cb to free memory. + data->ref = LUA_REFNIL; } return 0; diff --git a/src/lhandle.c b/src/lhandle.c index 92b725a9..1c4cc800 100644 --- a/src/lhandle.c +++ b/src/lhandle.c @@ -104,6 +104,7 @@ static void luv_call_callback(lua_State* L, luv_handle_t* data, luv_callback_id static void luv_unref_handle(lua_State* L, luv_handle_t* data) { luaL_unref(L, LUA_REGISTRYINDEX, data->ref); + data->ref = LUA_NOREF; luaL_unref(L, LUA_REGISTRYINDEX, data->callbacks[0]); luaL_unref(L, LUA_REGISTRYINDEX, data->callbacks[1]); } diff --git a/tests/manual-test-exit.lua b/tests/manual-test-exit.lua index 39db7e6d..90ef23cc 100644 --- a/tests/manual-test-exit.lua +++ b/tests/manual-test-exit.lua @@ -6,17 +6,17 @@ local uv = require('luv') local function setTimeout(callback, ms) - local timer = uv.new_timer() - timer:start(ms, 0, function() - timer:stop() - timer:close() - callback() - end) - return timer + local timer = uv.new_timer() + timer:start(ms, 0, function() + timer:stop() + timer:close() + callback() + end) + return timer end setTimeout(function() - os.exit(5, true) + os.exit(5, true) end, 1000) uv.run()