From c95284df1ab77f233562d9bc826523cfaaf7f41e Mon Sep 17 00:00:00 2001 From: David Goldblatt Date: Tue, 17 Apr 2018 13:16:42 -0700 Subject: [PATCH] Avoid a resource leak down extent split failure paths. Previously, we would leak the extent and memory associated with a salvageable portion of an extent that we were trying to split in three, in the case where the first split attempt succeeded and the second failed. --- src/extent.c | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/src/extent.c b/src/extent.c index 49b6d4b8..09d6d771 100644 --- a/src/extent.c +++ b/src/extent.c @@ -1319,21 +1319,19 @@ extent_grow_retained(tsdn_t *tsdn, arena_t *arena, * cant_alloc case should not occur. */ assert(result == extent_split_interior_error); + if (to_salvage != NULL) { + if (config_prof) { + extent_gdump_add(tsdn, to_salvage); + } + extent_record(tsdn, arena, r_extent_hooks, + &arena->extents_retained, to_salvage, true); + } if (to_leak != NULL) { extent_deregister_no_gdump_sub(tsdn, to_leak); extents_leak(tsdn, arena, r_extent_hooks, &arena->extents_retained, to_leak, true); - goto label_err; } - /* - * Note: we don't handle the non-NULL to_salvage case at all. - * This maintains the behavior that was present when the - * refactor pulling extent_split_interior into a helper function - * was added. I think this is actually a bug (we leak both the - * memory and the extent_t in that case), but since this code is - * getting deleted very shortly (in a subsequent commit), - * ensuring correctness down this path isn't worth the effort. - */ + goto label_err; } if (*commit && !extent_committed_get(extent)) {