pmem / pmdk

Persistent Memory Development Kit
https://pmem.io
Other
1.34k stars 510 forks source link

Crash-consistency bug within `pmemobj_tx_commit` #5461

Closed task3r closed 2 years ago

task3r commented 2 years ago

ISSUE: Crash-consistency bug within pmemobj_tx_commit

Environment Information

Please provide a reproduction of the bug:

Minimal working example main.c (based on data_store, assumes PM is mounted on /mnt/pmem0):

/*
 * btree_map example that triggers bug
 */

#include <assert.h>
#include <ex_common.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <sys/stat.h>
#include <time.h>

#include "map.h"
#include "map_btree.h"

POBJ_LAYOUT_BEGIN(data_store);
POBJ_LAYOUT_ROOT(data_store, struct store_root);
POBJ_LAYOUT_TOID(data_store, struct store_item);
POBJ_LAYOUT_END(data_store);

struct store_item {
    uint64_t item_data;
};

struct store_root {
    TOID(struct map) map;
};

static TOID(struct store_item) new_store_item(int value) {
    TOID(struct store_item) item = TX_NEW(struct store_item);
    D_RW(item)->item_data = value;

    return item;
}

int main(int argc, const char *argv[]) {
    const char *path = "/mnt/pmem0/pool";
    int nops = 2052;  // this only happens if nops >= 2052

    PMEMobjpool *pop;
    if (file_exists(path) != 0) {
        if ((pop = pmemobj_create(path, POBJ_LAYOUT_NAME(data_store),
                                  PMEMOBJ_MIN_POOL * 100, 0666)) == NULL) {
            perror("failed to create pool\n");
            return 1;
        }
    } else {
        if ((pop = pmemobj_open(path, POBJ_LAYOUT_NAME(data_store))) == NULL) {
            perror("failed to open pool\n");
            return 1;
        }
    }

    TOID(struct store_root) root = POBJ_ROOT(pop, struct store_root);
    struct map_ctx *mapc = map_ctx_init(MAP_BTREE, pop);

    if (map_check(mapc, D_RW(root)->map)) {
        TX_BEGIN(pop) { map_create(mapc, &D_RW(root)->map, NULL); }
        TX_END
    }

    for (int i = 0; i < nops; ++i) {
        TX_BEGIN(pop) {
            map_insert(mapc, D_RW(root)->map, i, new_store_item(i).oid);
        }
        TX_END
    }

    TX_BEGIN(pop) {
        for (int i = 0; i < nops; ++i) {
            map_remove(mapc, D_RW(root)->map, i);
        }
    }
    TX_END

    map_ctx_free(mapc);
    pmemobj_close(pop);

    return 0;
}

To compile it (assuming PMDK_ROOT points to the root of the repo):

gcc -g main.c -L $PMDK_ROOT/src/examples/libpmemobj/map/ -I $PMDK_ROOT/src/examples/libpmemobj/map/ -I $PMDK_ROOT/src/examples/ -lpmemobj -lmap -o main.elf

To launch gdb:

LD_LIBRARY_PATH=$PMDK_ROOT/src/debug/:$PMDK_ROOT/src/examples/libpmemobj/map/ gdb main.elf

Inside gdb:

# break on the commit for the removals
break main.c:74
run
break palloc.c:663
continue
break flush.h:48
continue
delete 1-3
# simulate crash and re-run the application
run

Output:

<libpmemobj>: <1> [ulog.c:517 ulog_entry_buf_create] assertion failure: ulog_entry_valid(ulog, &e->base)

Program received signal SIGABRT, Aborted.

How often bug is revealed: rare

I only managed to expose this bug using the btree backend and with n>2052. However, in these conditions, it happens every time. This does not seem to be caused by btree, since the bug did not manifest itself in PMDK 1.8 and there have not been significant changes to btree since then.

Actual behavior:

The application crashes in a subsequent run by failing an assertion.

Expected behavior:

The application does not crash in a subsequent run.

Additional information about Priority and Help Requested:

Are you willing to submit a pull request with a proposed change? No

Requested priority: Medium

pbalcer commented 2 years ago

Thanks for reporting the problem. This is the type of issue that's hard to check for with pmreorder and similar tools. It'd be too time-consuming ;)

At first glance, it seems the problem is related to large transactions that need to dynamically allocate extra undo log space - which isn't correctly initialized after a crash for some reason.

pbalcer commented 2 years ago

@task3r we've fixed the issue - can you confirm and close the issue?

I'd also be curious to know how you've discovered the problem - manually or are you working on some new tool? :-)

task3r commented 2 years ago

@pbalcer unfortunately it appears that the bug is still present. I tested branch stable-12 using the instructions shown above and the results are the same: the well-timed crash still leads to an assertion failure in the subsequent execution. Please let me know if I can help with something.

As for the second question, we detected the bug using a new tool we are developing. We cannot share it at the moment as it is under submission but we'll do it as soon as possible.

pbalcer commented 2 years ago

I just ran it on both the old and new versions of stable-1.12, and I cannot reproduce the problem after the patch. Can you double check you are testing against the latest commit?

task3r commented 2 years ago

My bad, you are totally right. I ran the experiments using containers and it used the build cache instead of fetching the latest updates to the branch... I can confirm that the issue is resolved and I'll close the issue. Sorry for the misunderstanding.

pbalcer commented 2 years ago

No worries. Please reach out once you can share your research with us ;)

Thanks.

task3r commented 11 months ago

Sorry for the delayed response, @pbalcer, but I forgot about your request. The research has been published in EuroSys'23: "Mumak: Efficient and Black-Box Bug Detection for Persistent Memory". Thank you again for your help and interest.