diff --git a/changelogs/unreleased/gh-10187-fix-memleaks-on-dead-fiber-usage.md b/changelogs/unreleased/gh-10187-fix-memleaks-on-dead-fiber-usage.md new file mode 100644 index 0000000000000000000000000000000000000000..eaa4d2e96e8d15e92eeb2d504460639449054056 --- /dev/null +++ b/changelogs/unreleased/gh-10187-fix-memleaks-on-dead-fiber-usage.md @@ -0,0 +1,3 @@ +## bugfix/core + +* Fixed memory leaks on using dead fiber (gh-10187). diff --git a/src/lib/core/fiber.c b/src/lib/core/fiber.c index 0f01f019d83d4daddf1eda161252177b51ff3b2d..6e01b6f20932a84f9fc24e6975c2ecdd14f74ee2 100644 --- a/src/lib/core/fiber.c +++ b/src/lib/core/fiber.c @@ -960,6 +960,7 @@ fiber_reset(struct fiber *fiber) { rlist_create(&fiber->on_yield); rlist_create(&fiber->on_stop); + rlist_create(&fiber->on_destroy); clock_stat_reset(&fiber->clock_stat); } @@ -972,6 +973,11 @@ fiber_recycle(struct fiber *fiber) assert(diag_is_empty(&fiber->diag)); /* no pending wakeup */ assert(rlist_empty(&fiber->state)); + assert(rlist_empty(&fiber->on_stop)); + if (trigger_run(&fiber->on_destroy, fiber) != 0) + panic("on_destroy triggers can't fail"); + /* On destroy triggers are expected to remove themselves. */ + assert(rlist_empty(&fiber->on_destroy)); fiber_stack_recycle(fiber); fiber_reset(fiber); fiber->name[0] = '\0'; @@ -1528,6 +1534,7 @@ fiber_destroy(struct cord *cord, struct fiber *f) assert(f != cord->fiber); trigger_destroy(&f->on_yield); trigger_destroy(&f->on_stop); + trigger_destroy(&f->on_destroy); rlist_del(&f->state); rlist_del(&f->link); rlist_del(&f->wake); diff --git a/src/lib/core/fiber.h b/src/lib/core/fiber.h index 494babd11b5b7a0b68351dd9c7cc1d930908f66b..da9f342e4a8ee6cb57d798552623c3b35c16a846 100644 --- a/src/lib/core/fiber.h +++ b/src/lib/core/fiber.h @@ -625,6 +625,8 @@ struct fiber { * request, even if they are never destroyed. */ struct rlist on_stop; + /** Triggers invoked before fiber is recycled. */ + struct rlist on_destroy; /** * The list of fibers awaiting for this fiber's timely * (or untimely) death. diff --git a/src/lua/fiber.c b/src/lua/fiber.c index 8a8f413084897d795155ceeb85a2ec4075966c72..abe0c4efa41e7ffa0f074b6b5cbe2066c00fb74d 100644 --- a/src/lua/fiber.c +++ b/src/lua/fiber.c @@ -88,14 +88,12 @@ luaL_testcancel(struct lua_State *L) static const char *fiberlib_name = "fiber"; /** - * Trigger invoked when the fiber has stopped execution of its - * current request. Only purpose - delete storage.lua.fid_ref and - * storage.lua.storage_ref keeping a reference of Lua - * fiber and fiber.storage objects. Unlike Lua stack, - * Lua fiber storage may be created not only for fibers born from - * Lua land. For example, an IProto request may execute a Lua - * function, which can create the storage. Trigger guarantees, - * that even for non-Lua fibers the Lua storage is destroyed. + * Trigger invoked when the fiber is stopped. Only purpose - delete + * storage.lua.storage_ref keeping a reference of Lua fiber.storage object. + * Unlike Lua stack, Lua fiber storage may be created not only for fibers born + * from Lua land. For example, an IProto request may execute a Lua function, + * which can create the storage. Trigger guarantees, that even for non-Lua + * fibers the Lua storage is destroyed. */ static int lbox_fiber_on_stop(struct trigger *trigger, void *event) @@ -103,6 +101,16 @@ lbox_fiber_on_stop(struct trigger *trigger, void *event) struct fiber *f = event; luaL_unref(tarantool_L, LUA_REGISTRYINDEX, f->storage.lua.storage_ref); f->storage.lua.storage_ref = FIBER_LUA_NOREF; + trigger_clear(trigger); + free(trigger); + return 0; +} + +/** Cleanup Lua fiber data when fiber is destroyed. */ +static int +lbox_fiber_on_destroy(struct trigger *trigger, void *event) +{ + struct fiber *f = event; luaL_unref(tarantool_L, LUA_REGISTRYINDEX, f->storage.lua.fid_ref); f->storage.lua.fid_ref = FIBER_LUA_NOREF; trigger_clear(trigger); @@ -118,13 +126,10 @@ lbox_pushfiber(struct lua_State *L, struct fiber *f) { int fid_ref = f->storage.lua.fid_ref; if (fid_ref == FIBER_LUA_NOREF) { - struct trigger *t = malloc(sizeof(*t)); - if (t == NULL) { - diag_set(OutOfMemory, sizeof(*t), "malloc", "t"); - luaT_error(L); - } - trigger_create(t, lbox_fiber_on_stop, NULL, (trigger_f0)free); - trigger_add(&f->on_stop, t); + struct trigger *on_destroy = xmalloc(sizeof(*on_destroy)); + trigger_create(on_destroy, lbox_fiber_on_destroy, NULL, + (trigger_f0)free); + trigger_add(&f->on_destroy, on_destroy); uint64_t fid = f->fid; /* create a new userdata */ @@ -642,6 +647,14 @@ lbox_fiber_storage(struct lua_State *L) struct fiber *f = lbox_checkfiber(L, 1); int storage_ref = f->storage.lua.storage_ref; if (storage_ref == FIBER_LUA_NOREF) { + if (f->flags & FIBER_IS_DEAD) { + diag_set(IllegalParams, "the fiber is dead"); + luaT_error(L); + } + struct trigger *on_stop = xmalloc(sizeof(*on_stop)); + trigger_create(on_stop, lbox_fiber_on_stop, NULL, + (trigger_f0)free); + trigger_add(&f->on_stop, on_stop); lua_newtable(L); /* create local storage on demand */ storage_ref = luaL_ref(L, LUA_REGISTRYINDEX); f->storage.lua.storage_ref = storage_ref; diff --git a/test/app-luatest/fiber_test.lua b/test/app-luatest/fiber_test.lua index 8a681b48ed89534f73486a560d7732f96ca23aba..4aa262617e33f58cf5f4107eb3002527268d13f1 100644 --- a/test/app-luatest/fiber_test.lua +++ b/test/app-luatest/fiber_test.lua @@ -57,3 +57,27 @@ g.test_gh_9406_shutdown_with_lingering_fiber_join = function() local cmd = string.format('%s -e "%s"', tarantool_bin, script) t.assert(os.execute(cmd) == 0) end + +g.test_gh_10187_no_memory_leak_on_dead_fiber_search = function() + local f = fiber.new(function() end) + f:set_joinable(true) + f:wakeup() + fiber.yield() + local x = fiber.find(f:id()) + -- Check found the fiber object is the same as the one created after the + -- fiber became dead. + t.assert_equals(x, f) + -- Check we cannot access dead fiber storage. + t.assert_error_covers({ + type = 'IllegalParams', + message = 'the fiber is dead', + }, x.__index, x, 'storage') + -- Check fiber object is GC after fiber is joined. + local weak_table = setmetatable({}, {__mode = 'v'}) + weak_table.fiber = f + x:join() + f = nil -- luacheck: no unused + x = nil -- luacheck: no unused + collectgarbage() + t.assert_equals(weak_table.fiber, nil) +end