Skip to content
Snippets Groups Projects
Commit e8c2c368 authored by Piotr Balcer's avatar Piotr Balcer
Browse files

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.
parent 955edbf3
No related branches found
No related tags found
No related merge requests found
/*
* 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;
......
#
# 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
#!/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
/*
* 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) {
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment