From cb578bbe01326bfc4a7b676f6921189d84518f03 Mon Sep 17 00:00:00 2001 From: Azat Khuzhin Date: Sun, 3 Jul 2022 20:23:59 +0300 Subject: [PATCH] Fix possible "nmalloc >= ndalloc" assertion In arena_stats_merge() first nmalloc was read, and after ndalloc. However with this order, it is possible for some thread to incement ndalloc in between, and then nmalloc < ndalloc, and assertion will fail, like again found by ClickHouse CI [1] (even after #2234). [1]: https://github.com/ClickHouse/ClickHouse/issues/31531 Swap the order to avoid possible assertion. Cc: @interwq Follow-up for: #2234 --- src/arena.c | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/arena.c b/src/arena.c index 857b27c5..1ab2775e 100644 --- a/src/arena.c +++ b/src/arena.c @@ -106,18 +106,21 @@ arena_stats_merge(tsdn_t *tsdn, arena_t *arena, unsigned *nthreads, astats->metadata_thp += metadata_thp; for (szind_t i = 0; i < SC_NSIZES - SC_NBINS; i++) { - uint64_t nmalloc = locked_read_u64(tsdn, - LOCKEDINT_MTX(arena->stats.mtx), - &arena->stats.lstats[i].nmalloc); - locked_inc_u64_unsynchronized(&lstats[i].nmalloc, nmalloc); - astats->nmalloc_large += nmalloc; - + /* ndalloc should be read before nmalloc, + * since otherwise it is possible for ndalloc to be incremented, + * and the following can become true: ndalloc > nmalloc */ uint64_t ndalloc = locked_read_u64(tsdn, LOCKEDINT_MTX(arena->stats.mtx), &arena->stats.lstats[i].ndalloc); locked_inc_u64_unsynchronized(&lstats[i].ndalloc, ndalloc); astats->ndalloc_large += ndalloc; + uint64_t nmalloc = locked_read_u64(tsdn, + LOCKEDINT_MTX(arena->stats.mtx), + &arena->stats.lstats[i].nmalloc); + locked_inc_u64_unsynchronized(&lstats[i].nmalloc, nmalloc); + astats->nmalloc_large += nmalloc; + uint64_t nrequests = locked_read_u64(tsdn, LOCKEDINT_MTX(arena->stats.mtx), &arena->stats.lstats[i].nrequests);