diff --git a/changelogs/unreleased/fiber-custom-stack-leak.md b/changelogs/unreleased/fiber-custom-stack-leak.md
new file mode 100644
index 0000000000000000000000000000000000000000..72c48b67d6035ed94608cbd08ab5f91231e6b535
--- /dev/null
+++ b/changelogs/unreleased/fiber-custom-stack-leak.md
@@ -0,0 +1,5 @@
+## bugfix/core
+
+* Fixed a bug due to which all fibers created with `fiber_attr_setstacksize()`
+ leaked until the thread exit. Their stacks also leaked except when
+ `fiber_set_joinable(..., true)` was used.
diff --git a/src/lib/core/fiber.c b/src/lib/core/fiber.c
index 3e19f131b7c37d3a7d1ada639517e57411508c74..08087cfe46586cba582f66f08b5b98f40d099db7 100644
--- a/src/lib/core/fiber.c
+++ b/src/lib/core/fiber.c
@@ -373,7 +373,14 @@ static void
fiber_stack_recycle(struct fiber *fiber);
static void
-fiber_destroy(struct cord *cord, struct fiber *f);
+fiber_delete(struct cord *cord, struct fiber *f);
+
+/**
+ * Try to delete a fiber right now or later if can't do now. The latter happens
+ * for self fiber - can't delete own stack.
+ */
+static void
+cord_add_garbage(struct cord *cord, struct fiber *f);
/**
* Transfer control to callee fiber.
@@ -903,7 +910,7 @@ fiber_recycle(struct fiber *fiber)
if (!has_custom_stack) {
rlist_move_entry(&cord()->dead, fiber, link);
} else {
- fiber_destroy(cord(), fiber);
+ cord_add_garbage(cord(), fiber);
}
}
@@ -1222,6 +1229,7 @@ fiber_new_ex(const char *name, const struct fiber_attr *fiber_attr,
struct cord *cord = cord();
struct fiber *fiber = NULL;
assert(fiber_attr != NULL);
+ cord_collect_garbage(cord);
/* Now we can not reuse fiber if custom attribute was set */
if (!(fiber_attr->flags & FIBER_CUSTOM_STACK) &&
@@ -1297,13 +1305,9 @@ fiber_new(const char *name, fiber_func f)
* cord_destroy().
*/
static void
-fiber_destroy(struct cord *cord, struct fiber *f)
+fiber_delete(struct cord *cord, struct fiber *f)
{
- if (f == fiber()) {
- /** End of the application. */
- assert(cord == &main_cord);
- return;
- }
+ assert(f != cord->fiber);
assert(f != &cord->sched);
trigger_destroy(&f->on_yield);
@@ -1315,18 +1319,27 @@ fiber_destroy(struct cord *cord, struct fiber *f)
diag_destroy(&f->diag);
if (f->name != f->inline_name)
free(f->name);
- f->name = NULL;
+ TRASH(f);
+ mempool_free(&cord->fiber_mempool, f);
+}
+
+/** Delete all fibers in the given list so it becomes empty. */
+static void
+cord_delete_fibers_in_list(struct cord *cord, struct rlist *list)
+{
+ while (!rlist_empty(list)) {
+ struct fiber *f = rlist_first_entry(list, struct fiber, link);
+ fiber_delete(cord, f);
+ }
}
void
-fiber_destroy_all(struct cord *cord)
+fiber_delete_all(struct cord *cord)
{
- while (!rlist_empty(&cord->alive))
- fiber_destroy(cord, rlist_first_entry(&cord->alive,
- struct fiber, link));
- while (!rlist_empty(&cord->dead))
- fiber_destroy(cord, rlist_first_entry(&cord->dead,
- struct fiber, link));
+ cord_collect_garbage(cord);
+ cord_delete_fibers_in_list(cord, &cord->alive);
+ cord_delete_fibers_in_list(cord, &cord->dead);
+ cord_delete_fibers_in_list(cord, &cord->ready);
}
#if ENABLE_FIBER_TOP
@@ -1454,6 +1467,7 @@ cord_create(struct cord *cord, const char *name)
rlist_create(&cord->alive);
rlist_create(&cord->ready);
rlist_create(&cord->dead);
+ cord->garbage = NULL;
cord->fiber_registry = mh_i64ptr_new();
/* sched fiber is not present in alive/ready/dead list. */
@@ -1498,6 +1512,27 @@ cord_create(struct cord *cord, const char *name)
#endif
}
+void
+cord_collect_garbage(struct cord *cord)
+{
+ struct fiber *garbage = cord->garbage;
+ if (garbage == NULL)
+ return;
+ fiber_delete(cord, garbage);
+ cord->garbage = NULL;
+}
+
+static void
+cord_add_garbage(struct cord *cord, struct fiber *f)
+{
+ cord_collect_garbage(cord);
+ assert(cord->garbage == NULL);
+ if (f != cord->fiber)
+ fiber_delete(cord, f);
+ else
+ cord->garbage = f;
+}
+
void
cord_destroy(struct cord *cord)
{
@@ -1506,7 +1541,7 @@ cord_destroy(struct cord *cord)
ev_loop_destroy(cord->loop);
/* Only clean up if initialized. */
if (cord->fiber_registry) {
- fiber_destroy_all(cord);
+ fiber_delete_all(cord);
mh_i64ptr_delete(cord->fiber_registry);
}
region_destroy(&cord->sched.gc);
diff --git a/src/lib/core/fiber.h b/src/lib/core/fiber.h
index 74d89ba135f19bcda48643b9f0a5c0ef22e4e07e..305255c9a602303eec4f3034940c46fe445a7b5f 100644
--- a/src/lib/core/fiber.h
+++ b/src/lib/core/fiber.h
@@ -698,6 +698,17 @@ struct cord {
struct rlist ready;
/** A cache of dead fibers for reuse */
struct rlist dead;
+ /**
+ * Latest dead fiber which couldn't be reused and waits for its
+ * deletion. A fiber can't be reused if it is somehow non-standard. For
+ * instance, has a special stack.
+ * A fiber can't be deleted if it is the current fiber - can't delete
+ * own stack safely. Then it schedules own deletion for later. The
+ * approach is very similar to pthread stacks deletion - pthread can't
+ * delete own stack, so they are saved and deleted later by a newer
+ * pthread or by some other dying pthread. Same here with fibers.
+ */
+ struct fiber *garbage;
/** A watcher to have a single async event for all ready fibers.
* This technique is necessary to be able to suspend
* a single fiber on a few watchers (for example,
@@ -810,6 +821,16 @@ cord_name(struct cord *cord)
bool
cord_is_main(void);
+/**
+ * Delete the latest garbage fiber which couldn't be deleted somewhy before. Can
+ * safely rely on the fiber being not the current one. Because if it was added
+ * here before, it means some previous fiber put itself here, then died
+ * immediately afterwards for good, and gave control to another fiber. It
+ * couldn't be scheduled again.
+ */
+void
+cord_collect_garbage(struct cord *cord);
+
void
fiber_init(int (*fiber_invoke)(fiber_func f, va_list ap));
diff --git a/test/unit/fiber.cc b/test/unit/fiber.cc
index a1e3ded87b0e580f1a87a7484a4f0b48a8cfdca7..c0a46c6123c282e644bd3ffe28b457a60dba5579 100644
--- a/test/unit/fiber.cc
+++ b/test/unit/fiber.cc
@@ -11,6 +11,15 @@ static struct fiber_attr default_attr;
static unsigned long page_size;
#define PAGE_4K 4096
+/** Total count of allocated fibers in the cord. Including dead ones. */
+static int
+fiber_count_total(void)
+{
+ size_t res = mempool_count(&cord()->fiber_mempool);
+ assert(res <= INT_MAX);
+ return (int)res;
+}
+
static int
noop_f(va_list ap)
{
@@ -153,6 +162,7 @@ fiber_stack_test()
struct fiber *fiber;
struct fiber_attr *fiber_attr;
+ struct slab_cache *slabc = &cord()->slabc;
/*
* Test a fiber with the default stack size.
@@ -166,15 +176,22 @@ fiber_stack_test()
/*
* Test a fiber with a custom stack size.
*/
+ int fiber_count = fiber_count_total();
+ size_t used1 = slabc->allocated.stats.used;
fiber_attr = fiber_attr_new();
fiber_attr_setstacksize(fiber_attr, default_attr.stack_size * 2);
stack_expand_limit = default_attr.stack_size * 3 / 2;
fiber = fiber_new_ex("test_stack", fiber_attr, test_stack_f);
+ fail_unless(fiber_count + 1 == fiber_count_total());
fiber_attr_delete(fiber_attr);
if (fiber == NULL)
diag_raise();
fiber_wakeup(fiber);
fiber_sleep(0);
+ cord_collect_garbage(cord());
+ fail_unless(fiber_count == fiber_count_total());
+ size_t used2 = slabc->allocated.stats.used;
+ fail_unless(used2 == used1);
note("big-stack fiber not crashed");
footer();
diff --git a/test/unit/fiber_stack.c b/test/unit/fiber_stack.c
index 1525b5d0736f92b3933cc7c66bf75a8e259e8d6b..5ab1f335804835014adb5d44d03c07947657448a 100644
--- a/test/unit/fiber_stack.c
+++ b/test/unit/fiber_stack.c
@@ -6,6 +6,15 @@
static struct fiber_attr default_attr;
+/** Total count of allocated fibers in the cord. Including dead ones. */
+static int
+fiber_count_total(void)
+{
+ size_t res = mempool_count(&cord()->fiber_mempool);
+ assert(res <= INT_MAX);
+ return (int)res;
+}
+
static int
noop_f(va_list ap)
{
@@ -19,9 +28,10 @@ main_f(va_list ap)
size_t used_before, used_after;
struct errinj *inj;
struct fiber *fiber;
+ int fiber_count = fiber_count_total();
header();
- plan(6);
+ plan(10);
/*
* Set non-default stack size to prevent reusing of an
@@ -57,9 +67,15 @@ main_f(va_list ap)
fiber = fiber_new_ex("test_madvise", fiber_attr, noop_f);
inj->bparam = false;
+ ok(fiber_count_total() == fiber_count + 1, "allocated new");
ok(fiber != NULL, "madvise: non critical error on madvise hint");
ok(diag_get() != NULL, "madvise: diag is armed after error");
+ fiber_wakeup(fiber);
+ fiber_sleep(0);
+ cord_collect_garbage(cord());
+ ok(fiber_count_total() == fiber_count, "fiber is deleted");
+
/*
* Check if we leak on fiber destruction.
* We will print an error and result get
@@ -76,6 +92,7 @@ main_f(va_list ap)
fiber = fiber_new_ex("test_madvise", fiber_attr, noop_f);
ok(fiber != NULL, "fiber with custom stack");
+ ok(fiber_count_total() == fiber_count + 1, "allocated new");
fiber_set_joinable(fiber, true);
inj = errinj(ERRINJ_FIBER_MPROTECT, ERRINJ_INT);
@@ -96,6 +113,9 @@ main_f(va_list ap)
used_after = slabc->allocated.stats.used;
ok(used_after > used_before, "expected leak detected");
+ cord_collect_garbage(cord());
+ ok(fiber_count_total() == fiber_count, "fiber is deleted");
+
fiber_attr_delete(fiber_attr);
footer();
diff --git a/test/unit/fiber_stack.result b/test/unit/fiber_stack.result
index 278db4fc589412b3f7aeb36f660f27cba6a9e497..e6fca997697ca80b3c0b7315b6ccc0c44cc588ce 100644
--- a/test/unit/fiber_stack.result
+++ b/test/unit/fiber_stack.result
@@ -1,10 +1,14 @@
SystemError: fiber mprotect failed: Cannot allocate memory
*** main_f ***
-1..6
+1..10
ok 1 - mprotect: failed to setup fiber guard page
ok 2 - mprotect: diag is armed after error
-ok 3 - madvise: non critical error on madvise hint
-ok 4 - madvise: diag is armed after error
-ok 5 - fiber with custom stack
-ok 6 - expected leak detected
+ok 3 - allocated new
+ok 4 - madvise: non critical error on madvise hint
+ok 5 - madvise: diag is armed after error
+ok 6 - fiber is deleted
+ok 7 - fiber with custom stack
+ok 8 - allocated new
+ok 9 - expected leak detected
+ok 10 - fiber is deleted
*** main_f: done ***