From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <gorcunov@gmail.com>
Received: from mail-lj1-f195.google.com (mail-lj1-f195.google.com
[209.85.208.195])
(using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits))
(No client certificate requested)
by dev.tarantool.org (Postfix) with ESMTPS id BA5E44696C3
for <tarantool-patches@dev.tarantool.org>;
Thu, 13 Feb 2020 23:56:44 +0300 (MSK)
Received: by mail-lj1-f195.google.com with SMTP id r19so8217123ljg.3
for <tarantool-patches@dev.tarantool.org>;
Thu, 13 Feb 2020 12:56:44 -0800 (PST)
From: Cyrill Gorcunov <gorcunov@gmail.com>
Date: Thu, 13 Feb 2020 23:56:18 +0300
Message-Id: <20200213205618.7982-3-gorcunov@gmail.com>
In-Reply-To: <20200213205618.7982-1-gorcunov@gmail.com>
References: <20200213205618.7982-1-gorcunov@gmail.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Subject: [Tarantool-patches] [PATCH v7 2/2] fiber: leak slab if unable to
bring prots back
List-Id: Tarantool development patches <tarantool-patches.dev.tarantool.org>
List-Unsubscribe: <https://lists.tarantool.org/mailman/options/tarantool-patches>,
<mailto:tarantool-patches-request@dev.tarantool.org?subject=unsubscribe>
List-Archive: <https://lists.tarantool.org/pipermail/tarantool-patches/>
List-Post: <mailto:tarantool-patches@dev.tarantool.org>
List-Help: <mailto:tarantool-patches-request@dev.tarantool.org?subject=help>
List-Subscribe: <https://lists.tarantool.org/mailman/listinfo/tarantool-patches>,
<mailto:tarantool-patches-request@dev.tarantool.org?subject=subscribe>
To: tml <tarantool-patches@dev.tarantool.org>
Cc: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
In case if we unable to revert guard page back to
read|write we should never use such slab again.
Initially I thought of just put panic here and
exit but it is too destructive. I think better
print an error and continue. If node admin ignore
this message then one moment at future there won't
be slab left for use and creating new fibers get
prohibited.
In future (hopefully near one) we plan to drop
guard pages to prevent VMA fracturing and use
stack marks instead.
Reviewed-by: Alexander Turenko <alexander.turenko@tarantool.org>
Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
---
src/lib/core/fiber.c | 11 +++++++++--
test/unit/fiber_stack.c | 29 ++++++++++++++++++++++++++---
test/unit/fiber_stack.result | 4 +++-
3 files changed, 38 insertions(+), 6 deletions(-)
diff --git a/src/lib/core/fiber.c b/src/lib/core/fiber.c
index f10687b29..73de7765b 100644
--- a/src/lib/core/fiber.c
+++ b/src/lib/core/fiber.c
@@ -1055,15 +1055,22 @@ fiber_stack_destroy(struct fiber *fiber, struct slab_cache *slabc)
* to setup the original protection back in
* background.
*
+ * For now lets keep such slab referenced and
+ * leaked: if mprotect failed we must not allow
+ * to reuse such slab with PROT_NONE'ed page
+ * inside.
+ *
* Note that in case if we're called from
* fiber_stack_create() the @a mprotect_flags is
* the same as the slab been created with, so
* calling mprotect for VMA with same flags
* won't fail.
*/
- diag_log();
+ say_syserror("fiber: Can't put guard page to slab. "
+ "Leak %zu bytes", (size_t)fiber->stack_size);
+ } else {
+ slab_put(slabc, fiber->stack_slab);
}
- slab_put(slabc, fiber->stack_slab);
}
}
diff --git a/test/unit/fiber_stack.c b/test/unit/fiber_stack.c
index 103dfaf0d..5eb128ea1 100644
--- a/test/unit/fiber_stack.c
+++ b/test/unit/fiber_stack.c
@@ -19,7 +19,7 @@ main_f(va_list ap)
struct fiber *fiber;
header();
- plan(4);
+ plan(5);
/*
* Set non-default stack size to prevent reusing of an
@@ -47,8 +47,7 @@ main_f(va_list ap)
ok(diag_get() != NULL, "mprotect: diag is armed after error");
/*
- * Check madvise. We can't test the fiber destroy
- * path since it is cleaning error.
+ * Check madvise error on fiber creation.
*/
diag_clear(diag_get());
inj = errinj(ERRINJ_FIBER_MADVISE, ERRINJ_BOOL);
@@ -59,6 +58,30 @@ main_f(va_list ap)
ok(fiber != NULL, "madvise: non critical error on madvise hint");
ok(diag_get() != NULL, "madvise: diag is armed after error");
+ /*
+ * Check if we leak on fiber descrution.
+ * We will print an error and result get
+ * compared by testing engine.
+ */
+ fiber_attr_delete(fiber_attr);
+ fiber_attr = fiber_attr_new();
+ fiber_attr->flags |= FIBER_CUSTOM_STACK;
+ fiber_attr->stack_size = 64 << 10;
+
+ diag_clear(diag_get());
+ fiber = fiber_new_ex("test_madvise", fiber_attr, noop_f);
+ ok(fiber != NULL, "fiber with custom stack");
+ fiber_set_joinable(fiber, true);
+
+ inj = errinj(ERRINJ_FIBER_MPROTECT, ERRINJ_INT);
+ inj->iparam = PROT_READ | PROT_WRITE;
+
+ fiber_start(fiber);
+ fiber_join(fiber);
+
+ inj->iparam = -1;
+
+ fiber_attr_delete(fiber_attr);
footer();
ev_break(loop(), EVBREAK_ALL);
diff --git a/test/unit/fiber_stack.result b/test/unit/fiber_stack.result
index 43ff74b2f..f0f5a59c2 100644
--- a/test/unit/fiber_stack.result
+++ b/test/unit/fiber_stack.result
@@ -1,8 +1,10 @@
SystemError fiber mprotect failed: Cannot allocate memory
+fiber: Can't put guard page to slab. Leak 57344 bytes: Cannot allocate memory
*** main_f ***
-1..4
+1..5
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
*** main_f: done ***
--
2.20.1