Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cross-shard realloc shrink crashes #2202

Open
travisdowns opened this issue Apr 24, 2024 · 0 comments · May be fixed by #2217
Open

Cross-shard realloc shrink crashes #2202

travisdowns opened this issue Apr 24, 2024 · 0 comments · May be fixed by #2217
Labels

Comments

@travisdowns
Copy link
Contributor

travisdowns commented Apr 24, 2024

If a pointer is allocated on one shard and then reallocated smaller on another it crashes in most cases [1], e.g., this test crashes:

SEASTAR_TEST_CASE(test_cross_thread_realloc) {
    // needs at least 2 shards to test what we want to test but will
    // still pass fine on 1 shard

    void *p = malloc(100);

    auto other_shard = (this_shard_id() + 1) % smp::count;

    return smp::submit_to(other_shard, [p]{
        BOOST_REQUIRE(realloc(p, 50));
    });
}

Crash:

alloc_test: /home/tdowns/dev/seastar/src/core/memory.cc:1173: void seastar::memory::cpu_pages::shrink(void *, size_t): Assertion `object_cpu_id(ptr) == cpu_id' failed.

This seems to violate the general contract of realloc and I think should be handled in the same way as cross-core frees. Fix incoming.


[1] The exceptions are if the new size is zero or the size class of the new size is the same as the old.

@tchaikov tchaikov added the bug label May 2, 2024
travisdowns added a commit to travisdowns/seastar that referenced this issue May 3, 2024
Cross-shard shrinking realloc crashes because we assert that the
incoming pointer is shard local inside the shrink method but there is
no particular reason to assume this is the case with a realloc: the
initial allocation may have been made on another shard.

Fix this by falling through to the free/malloc/memcpy path. This also
means that realloc using the same size is a way to "rehome" a possibly
foreign pointer: this does nothing if the pointer is already local but
it will allocate a local pointer and copy the allocation contents if
not.

Fixes scylladb#2202.
@travisdowns travisdowns linked a pull request May 3, 2024 that will close this issue
tchaikov pushed a commit to tchaikov/seastar that referenced this issue May 21, 2024
Cross-shard shrinking realloc crashes because we assert that the
incoming pointer is shard local inside the shrink method but there is
no particular reason to assume this is the case with a realloc: the
initial allocation may have been made on another shard.

Fix this by falling through to the free/malloc/memcpy path. This also
means that realloc using the same size is a way to "rehome" a possibly
foreign pointer: this does nothing if the pointer is already local but
it will allocate a local pointer and copy the allocation contents if
not.

Fixes scylladb#2202.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants