Add extent_destroy_t and use it during arena destruction.
Add the extent_destroy_t extent destruction hook to extent_hooks_t, and use it during arena destruction. This hook explicitly communicates to the callee that the extent must be destroyed or tracked for later reuse, lest it be permanently leaked. Prior to this change, retained extents could unintentionally be leaked if extent retention was enabled. This resolves #560.
This commit is contained in:
20
src/arena.c
20
src/arena.c
@@ -1138,21 +1138,19 @@ arena_reset(tsd_t *tsd, arena_t *arena) {
|
||||
static void
|
||||
arena_destroy_retained(tsdn_t *tsdn, arena_t *arena) {
|
||||
/*
|
||||
* Iterate over the retained extents and blindly attempt to deallocate
|
||||
* them. This gives the extent allocator underlying the extent hooks an
|
||||
* opportunity to unmap all retained memory without having to keep its
|
||||
* own metadata structures, but if deallocation fails, that is the
|
||||
* application's decision/problem. In practice, retained extents are
|
||||
* leaked here if opt_retain unless the application provided custom
|
||||
* extent hooks, so best practice is to either disable retain (and avoid
|
||||
* dss for arenas to be destroyed), or provide custom extent hooks that
|
||||
* either unmap retained extents or track them for later use.
|
||||
* Iterate over the retained extents and destroy them. This gives the
|
||||
* extent allocator underlying the extent hooks an opportunity to unmap
|
||||
* all retained memory without having to keep its own metadata
|
||||
* structures. In practice, virtual memory for dss-allocated extents is
|
||||
* leaked here, so best practice is to avoid dss for arenas to be
|
||||
* destroyed, or provide custom extent hooks that track retained
|
||||
* dss-based extents for later reuse.
|
||||
*/
|
||||
extent_hooks_t *extent_hooks = extent_hooks_get(arena);
|
||||
extent_t *extent;
|
||||
while ((extent = extents_evict(tsdn, arena, &extent_hooks,
|
||||
&arena->extents_retained, 0)) != NULL) {
|
||||
extent_dalloc_wrapper_try(tsdn, arena, &extent_hooks, extent);
|
||||
extent_destroy_wrapper(tsdn, arena, &extent_hooks, extent);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1169,7 +1167,7 @@ arena_destroy(tsd_t *tsd, arena_t *arena) {
|
||||
*/
|
||||
assert(extents_npages_get(&arena->extents_dirty) == 0);
|
||||
|
||||
/* Attempt to deallocate retained memory. */
|
||||
/* Deallocate retained memory. */
|
||||
arena_destroy_retained(tsd_tsdn(tsd), arena);
|
||||
|
||||
/*
|
||||
|
47
src/extent.c
47
src/extent.c
@@ -19,6 +19,8 @@ static void *extent_alloc_default(extent_hooks_t *extent_hooks,
|
||||
unsigned arena_ind);
|
||||
static bool extent_dalloc_default(extent_hooks_t *extent_hooks, void *addr,
|
||||
size_t size, bool committed, unsigned arena_ind);
|
||||
static void extent_destroy_default(extent_hooks_t *extent_hooks, void *addr,
|
||||
size_t size, bool committed, unsigned arena_ind);
|
||||
static bool extent_commit_default(extent_hooks_t *extent_hooks, void *addr,
|
||||
size_t size, size_t offset, size_t length, unsigned arena_ind);
|
||||
static bool extent_decommit_default(extent_hooks_t *extent_hooks,
|
||||
@@ -43,6 +45,7 @@ static bool extent_merge_default(extent_hooks_t *extent_hooks, void *addr_a,
|
||||
const extent_hooks_t extent_hooks_default = {
|
||||
extent_alloc_default,
|
||||
extent_dalloc_default,
|
||||
extent_destroy_default,
|
||||
extent_commit_default,
|
||||
extent_decommit_default
|
||||
#ifdef PAGES_CAN_PURGE_LAZY
|
||||
@@ -1366,7 +1369,7 @@ extent_dalloc_default(extent_hooks_t *extent_hooks, void *addr, size_t size,
|
||||
return extent_dalloc_default_impl(addr, size);
|
||||
}
|
||||
|
||||
bool
|
||||
static bool
|
||||
extent_dalloc_wrapper_try(tsdn_t *tsdn, arena_t *arena,
|
||||
extent_hooks_t **r_extent_hooks, extent_t *extent) {
|
||||
bool err;
|
||||
@@ -1443,6 +1446,48 @@ extent_dalloc_wrapper(tsdn_t *tsdn, arena_t *arena,
|
||||
extent);
|
||||
}
|
||||
|
||||
static void
|
||||
extent_destroy_default_impl(void *addr, size_t size) {
|
||||
if (!have_dss || !extent_in_dss(addr)) {
|
||||
pages_unmap(addr, size);
|
||||
}
|
||||
}
|
||||
|
||||
static void
|
||||
extent_destroy_default(extent_hooks_t *extent_hooks, void *addr, size_t size,
|
||||
bool committed, unsigned arena_ind) {
|
||||
assert(extent_hooks == &extent_hooks_default);
|
||||
|
||||
extent_destroy_default_impl(addr, size);
|
||||
}
|
||||
|
||||
void
|
||||
extent_destroy_wrapper(tsdn_t *tsdn, arena_t *arena,
|
||||
extent_hooks_t **r_extent_hooks, extent_t *extent) {
|
||||
assert(extent_base_get(extent) != NULL);
|
||||
assert(extent_size_get(extent) != 0);
|
||||
witness_assert_depth_to_rank(tsdn, WITNESS_RANK_CORE, 0);
|
||||
|
||||
/* Deregister first to avoid a race with other allocating threads. */
|
||||
extent_deregister(tsdn, extent);
|
||||
|
||||
extent_addr_set(extent, extent_base_get(extent));
|
||||
|
||||
extent_hooks_assure_initialized(arena, r_extent_hooks);
|
||||
/* Try to destroy; silently fail otherwise. */
|
||||
if (*r_extent_hooks == &extent_hooks_default) {
|
||||
/* Call directly to propagate tsdn. */
|
||||
extent_destroy_default_impl(extent_base_get(extent),
|
||||
extent_size_get(extent));
|
||||
} else if ((*r_extent_hooks)->destroy != NULL) {
|
||||
(*r_extent_hooks)->destroy(*r_extent_hooks,
|
||||
extent_base_get(extent), extent_size_get(extent),
|
||||
extent_committed_get(extent), arena_ind_get(arena));
|
||||
}
|
||||
|
||||
extent_dalloc(tsdn, arena, extent);
|
||||
}
|
||||
|
||||
static bool
|
||||
extent_commit_default(extent_hooks_t *extent_hooks, void *addr, size_t size,
|
||||
size_t offset, size_t length, unsigned arena_ind) {
|
||||
|
Reference in New Issue
Block a user