From e8c2c3682e22a6b8eb42caae86d191f01fb04df0 Mon Sep 17 00:00:00 2001
From: Piotr Balcer <piotr.balcer@intel.com>
Date: Fri, 22 Mar 2019 14:59:29 +0100
Subject: [PATCH] obj: fix crash after large undo log recovery
The 'operation_context' structure was being created twice, once as part
of lane initialization, and once as part of the recovery operation.
The first context is used for all operations, whereas the second context
lives only for the duration of the undo log recovery.
The problem was that the recovery process can change the persistent
state in such a way that the context runtime information becomes stale.
This is perfectly fine, and it's handled for the operation that
does the recovery - but at the same time, this makes the data of the
lane context invalid.
This bug was triggering ASSERTs in debug builds and might have led to
NULL dereference on release builds. Fortunately the crash happens
after recovery has been finished and simply restarting the application
gets rid of the problem.
The fix is quite simple: instead of duplicating the runtime state for
the purpose of recovery, simply use the one that already exists in the
lane.
---
src/libpmemobj/lane.c | 18 +--------
src/test/obj_recovery/Makefile | 3 +-
src/test/obj_recovery/TEST9 | 56 ++++++++++++++++++++++++++++
src/test/obj_recovery/obj_recovery.c | 33 ++++++++++++++--
4 files changed, 90 insertions(+), 20 deletions(-)
create mode 100755 src/test/obj_recovery/TEST9
diff --git a/src/libpmemobj/lane.c b/src/libpmemobj/lane.c
index 28e64fa85..1849e9858 100644
--- a/src/libpmemobj/lane.c
+++ b/src/libpmemobj/lane.c
@@ -1,5 +1,5 @@
/*
- * Copyright 2015-2018, Intel Corporation
+ * Copyright 2015-2019, Intel Corporation
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -402,24 +402,10 @@ lane_recover_and_section_boot(PMEMobjpool *pop)
* a undo recovery might require deallocation of the next ulogs.
*/
for (i = 0; i < pop->nlanes; ++i) {
- layout = lane_get_layout(pop, i);
-
- struct ulog *undo = (struct ulog *)&layout->undo;
-
- struct operation_context *ctx = operation_new(
- undo,
- LANE_UNDO_SIZE,
- lane_undo_extend, (ulog_free_fn)pfree, &pop->p_ops,
- LOG_TYPE_UNDO);
- if (ctx == NULL) {
- LOG(2, "undo recovery failed %" PRIu64 " %d",
- i, err);
- return err;
- }
+ struct operation_context *ctx = pop->lanes_desc.lane[i].undo;
operation_resume(ctx);
operation_process(ctx);
operation_finish(ctx);
- operation_delete(ctx);
}
return 0;
diff --git a/src/test/obj_recovery/Makefile b/src/test/obj_recovery/Makefile
index 40fcd353d..d8afac8b2 100644
--- a/src/test/obj_recovery/Makefile
+++ b/src/test/obj_recovery/Makefile
@@ -1,5 +1,5 @@
#
-# Copyright 2015-2018, Intel Corporation
+# Copyright 2015-2019, Intel Corporation
#
# Redistribution and use in source and binary forms, with or without
# modification, are permitted provided that the following conditions
@@ -39,5 +39,6 @@ OBJS = obj_recovery.o
LIBPMEM=y
LIBPMEMOBJ=y
+LIBPMEMCOMMON=y
include ../Makefile.inc
diff --git a/src/test/obj_recovery/TEST9 b/src/test/obj_recovery/TEST9
new file mode 100755
index 000000000..a039777f9
--- /dev/null
+++ b/src/test/obj_recovery/TEST9
@@ -0,0 +1,56 @@
+#!/usr/bin/env bash
+#
+# Copyright 2019, Intel Corporation
+#
+# Redistribution and use in source and binary forms, with or without
+# modification, are permitted provided that the following conditions
+# are met:
+#
+# * Redistributions of source code must retain the above copyright
+# notice, this list of conditions and the following disclaimer.
+#
+# * Redistributions in binary form must reproduce the above copyright
+# notice, this list of conditions and the following disclaimer in
+# the documentation and/or other materials provided with the
+# distribution.
+#
+# * Neither the name of the copyright holder nor the names of its
+# contributors may be used to endorse or promote products derived
+# from this software without specific prior written permission.
+#
+# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+# A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+# OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+#
+
+#
+# src/test/obj_recovery/TEST9 -- unit test for pool recovery
+#
+
+# standard unit test setup
+. ../unittest/unittest.sh
+
+require_test_type medium
+require_no_asan
+
+setup
+
+# exits in the middle of transaction, so pool cannot be closed
+export MEMCHECK_DONT_CHECK_LEAKS=1
+
+create_holey_file 16M $DIR/testfile
+
+expect_normal_exit ./obj_recovery$EXESUFFIX $DIR/testfile n c l
+expect_normal_exit ./obj_recovery$EXESUFFIX $DIR/testfile n o l
+
+check
+
+pass
diff --git a/src/test/obj_recovery/obj_recovery.c b/src/test/obj_recovery/obj_recovery.c
index b53840771..348a843aa 100644
--- a/src/test/obj_recovery/obj_recovery.c
+++ b/src/test/obj_recovery/obj_recovery.c
@@ -1,5 +1,5 @@
/*
- * Copyright 2015-2018, Intel Corporation
+ * Copyright 2015-2019, Intel Corporation
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -46,6 +46,8 @@ POBJ_LAYOUT_ROOT(recovery, struct root);
POBJ_LAYOUT_TOID(recovery, struct foo);
POBJ_LAYOUT_END(recovery);
+#define MB (1 << 20)
+
struct foo {
int bar;
};
@@ -53,6 +55,7 @@ struct foo {
struct root {
PMEMmutex lock;
TOID(struct foo) foo;
+ char large_data[MB];
};
#define BAR_VALUE 5
@@ -67,14 +70,14 @@ main(int argc, char *argv[])
if (argc != 5)
UT_FATAL("usage: %s [file] [lock: y/n] "
- "[cmd: c/o] [type: n/f/s]",
+ "[cmd: c/o] [type: n/f/s/l]",
argv[0]);
const char *path = argv[1];
PMEMobjpool *pop = NULL;
int exists = argv[3][0] == 'o';
- enum { TEST_NEW, TEST_FREE, TEST_SET } type;
+ enum { TEST_NEW, TEST_FREE, TEST_SET, TEST_LARGE } type;
if (argv[4][0] == 'n')
type = TEST_NEW;
@@ -82,6 +85,8 @@ main(int argc, char *argv[])
type = TEST_FREE;
else if (argv[4][0] == 's')
type = TEST_SET;
+ else if (argv[4][0] == 'l')
+ type = TEST_LARGE;
else
UT_FATAL("invalid type");
@@ -143,6 +148,28 @@ main(int argc, char *argv[])
} else {
UT_ASSERT(D_RW(D_RW(root)->foo)->bar == BAR_VALUE);
}
+ } else if (type == TEST_LARGE) {
+ if (!exists) {
+ TX_BEGIN(pop) {
+ TX_MEMSET(D_RW(root)->large_data, 0xc, MB);
+ pmemobj_persist(pop,
+ D_RW(root)->large_data, MB);
+ VALGRIND_PMEMCHECK_END_TX;
+
+ exit(0);
+ } TX_END
+ } else {
+ UT_ASSERT(util_is_zeroed(D_RW(root)->large_data, MB));
+
+ TX_BEGIN(pop) { /* we should be able to start TX */
+ TX_MEMSET(D_RW(root)->large_data, 0xc, MB);
+ pmemobj_persist(pop,
+ D_RW(root)->large_data, MB);
+ VALGRIND_PMEMCHECK_END_TX;
+
+ pmemobj_tx_abort(0);
+ } TX_END
+ }
} else if (type == TEST_NEW) {
if (!exists) {
TX_BEGIN_PARAM(pop, lock_type, lock) {
--
GitLab