fiber: fix leak on dead joinable fiber search

When fiber is accessed from Lua we create a userdata object and keep the
reference for future accesses. The reference is cleared when fiber is
stopped. But if fiber is joinable is still can be found with
`fiber.find`. In this case we create userdata object again.
Unfortunately as fiber is already stopped we fail to clear the
reference. The trigger memory that clear the reference is also leaked.
As well as fiber storage if it is accessed after fiber is stopped.

Let's add `on_destroy` trigger to fiber and clear the references there.

Note that with current set of LSAN suppressions the trigger memory leak
of the issue is not reported.

Closes tarantool#10187

NO_DOC=bugfix

Author: nshy

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).

src/lib/core/fiber.c

@@ -1034,6 +1034,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);
 }
 
@@ -1046,6 +1047,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';
@@ -1642,6 +1648,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);

src/lib/core/fiber.h

@@ -683,6 +683,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.

src/lua/fiber.c

@@ -88,21 +88,29 @@ 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)
 {
 	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 */
@@ -653,6 +658,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;

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