@cryptotaxi247 / netdata-1 / commits / 9f7887ec0

Add test for ARAL race condition and fix (#22212)

* Add stress test and unit test for ARAL allocator race condition fixes * Fix race condition and cleanup tests * Reorder `#ifdef NETDATA_INTERNAL_CHECKS` to fix struct placement and improve readability * Refactor `aral_race_unittest_force_page_full` to return forced entry and ensure consistent page state handling (unittest) * Fix `aral_detect_acquire_to_page_lock_race` to handle retried allocations and improve page state validation * Improve ARAL race unittest to handle thread errors and ensure clean teardown - Add error handling for ARAL allocator thread creation. - Prevent null thread dereference and streamline cleanup with conditional `nd_thread_join`.

Stelios Fragkakis committed Apr 16, 2026 at 19:09 UTC 9f7887ec0da96f66d0c520f342e03588cda8bb1b
1 file changed +228 -13
src/libnetdata/aral/aral.c
+228 -13
@@ -276,6 +276,35 @@ static ALWAYS_INLINE void aral_page_incoming_unlock(ARAL *ar, ARAL_PAGE *page, s
276 spinlock_unlock(&page->incoming[partition].spinlock);
277 }
278
279 +#ifdef NETDATA_INTERNAL_CHECKS
280 +struct aral_race_unittest_hook {
281 + ARAL *ar;
282 + ARAL_PAGE *page;
283 + struct aral_unittest_entry *forced_entry;
284 + bool enabled;
285 + bool first_allocator_waiting;
286 + bool release_first_allocator;
287 + bool first_allocator_claimed;
288 + bool page_force_fully_used;
289 +};
290 +
291 +static struct aral_race_unittest_hook aral_race_unittest_hook = { 0 };
292 +
293 +static ALWAYS_INLINE void aral_unittest_wait_for_race_window(ARAL *ar, ARAL_PAGE *page) {
294 + if(unlikely(__atomic_load_n(&aral_race_unittest_hook.enabled, __ATOMIC_RELAXED) &&
295 + aral_race_unittest_hook.ar == ar)) {
296 + bool expected = false;
297 + if(__atomic_compare_exchange_n(&aral_race_unittest_hook.first_allocator_claimed, &expected, true, false,
298 + __ATOMIC_ACQ_REL, __ATOMIC_RELAXED)) {
299 + aral_race_unittest_hook.page = page;
300 + __atomic_store_n(&aral_race_unittest_hook.first_allocator_waiting, true, __ATOMIC_RELEASE);
301 + while(!__atomic_load_n(&aral_race_unittest_hook.release_first_allocator, __ATOMIC_ACQUIRE))
302 + tinysleep();
303 + }
304 + }
305 +}
306 +#endif
307 +
308 static ALWAYS_INLINE bool aral_adders_trylock(ARAL *ar, bool marked) {
309 if(likely(!(ar->config.options & ARAL_LOCKLESS))) {
310 size_t idx = mark_to_idx(marked);
@@ -676,8 +705,10 @@ static void aral_del_page___no_lock_needed(ARAL *ar, ARAL_PAGE *page TRACE_ALLOC
705 ALWAYS_INLINE WARNUNUSED
706 static bool aral_page_acquire(ARAL_PAGE *page) {
707 REFCOUNT rf = __atomic_add_fetch(&page->refcount, 1, __ATOMIC_ACQUIRE);
679 - if(rf <= 0)
708 + if(rf <= 0) {
709 + __atomic_sub_fetch(&page->refcount, 1, __ATOMIC_RELAXED);
710 return false;
711 + }
712
713 if(rf > (REFCOUNT)page->max_elements) {
714 __atomic_sub_fetch(&page->refcount, 1, __ATOMIC_RELAXED);
@@ -723,7 +754,10 @@ static ALWAYS_INLINE ARAL_PAGE *aral_get_first_page_with_a_free_slot(ARAL *ar, b
754 struct free_space f1, f2;
755 #endif
756
726 - ARAL_PAGE *page;
757 + ARAL_PAGE *page = NULL;
758 +
759 +retry_acquisition:
760 +
761 while(!(page = aral_acquire_first_page(ar, marked))) {
762 #ifdef NETDATA_ARAL_INTERNAL_CHECKS
763 f1 = check_free_space___aral_lock_needed(ar, NULL, marked);
@@ -790,11 +824,19 @@ static ALWAYS_INLINE ARAL_PAGE *aral_get_first_page_with_a_free_slot(ARAL *ar, b
824 "ARAL: '%s' failed to find a page with a free element",
825 ar->config.name);
826
827 +#ifdef NETDATA_INTERNAL_CHECKS
828 + aral_unittest_wait_for_race_window(ar, page);
829 +#endif
830 +
831 aral_page_lock(ar, page);
832
795 - internal_fatal(!page->page_lock.free_elements,
796 - "ARAL: '%s' selected page does not have a free slot in it",
797 - ar->config.name);
833 + if(unlikely(!page->page_lock.free_elements)) {
834 + aral_page_unlock(ar, page);
835 + bool deleted = aral_page_release(page);
836 + (void)deleted;
837 + page = NULL;
838 + goto retry_acquisition;
839 + }
840
841 internal_fatal(page->max_elements != page->page_lock.used_elements + page->page_lock.free_elements,
842 "ARAL: '%s' page element counters do not match, "
@@ -1423,6 +1465,167 @@ static inline struct aral_unittest_entry *unittest_aral_malloc(ARAL *ar, bool ma
1465 return t;
1466 }
1467
1468 +#ifdef NETDATA_INTERNAL_CHECKS
1469 +
1470 +struct aral_race_unittest_allocator {
1471 + ARAL *ar;
1472 + struct aral_unittest_entry *entry;
1473 +};
1474 +static bool aral_unittest_wait_for_flag(bool *flag, usec_t timeout_ut) {
1475 + usec_t started_ut = now_monotonic_usec();
1476 +
1477 + while(!__atomic_load_n(flag, __ATOMIC_ACQUIRE)) {
1478 + if(now_monotonic_usec() - started_ut > timeout_ut)
1479 + return false;
1480 +
1481 + tinysleep();
1482 + }
1483 +
1484 + return true;
1485 +}
1486 +
1487 +static void aral_race_unittest_allocator_thread(void *ptr) {
1488 + struct aral_race_unittest_allocator *ctx = ptr;
1489 + ctx->entry = unittest_aral_malloc(ctx->ar, false);
1490 +}
1491 +
1492 +static struct aral_unittest_entry *aral_race_unittest_force_page_full(ARAL *ar, ARAL_PAGE *page) {
1493 + struct aral_unittest_entry *entry;
1494 + aral_page_lock(ar, page);
1495 +
1496 + internal_fatal(!page->page_lock.free_elements,
1497 + "ARAL race unittest: target page unexpectedly has no free elements");
1498 +
1499 + uint64_t slot = __atomic_fetch_add(&page->elements_segmented, 1, __ATOMIC_ACQUIRE);
1500 + internal_fatal(slot >= page->max_elements,
1501 + "ARAL race unittest: failed to reserve the last free slot");
1502 +
1503 + entry = (struct aral_unittest_entry *)(page->data + (slot * ar->config.element_size));
1504 + aral_set_page_pointer_after_element___do_NOT_have_aral_lock(ar, page, entry, false);
1505 + *entry = UNITTEST_ITEM;
1506 + aral_element_given(ar, page);
1507 +
1508 + page->page_lock.used_elements++;
1509 + page->page_lock.free_elements--;
1510 +
1511 + REFCOUNT rf = __atomic_add_fetch(&page->refcount, 1, __ATOMIC_RELAXED);
1512 + internal_fatal(rf < (REFCOUNT)page->max_elements || rf > (REFCOUNT)page->max_elements + 1,
1513 + "ARAL race unittest: invalid forced refcount %d for max_elements %u",
1514 + rf, page->max_elements);
1515 +
1516 + aral_lock(ar);
1517 + if(page->aral_lock.head_ptr == aral_pages_head_free(ar, false)) {
1518 + DOUBLE_LINKED_LIST_REMOVE_ITEM_UNSAFE(*page->aral_lock.head_ptr, page, aral_lock.prev, aral_lock.next);
1519 +
1520 + ARAL_PAGE **head_ptr_full = aral_pages_head_full(ar, false);
1521 + DOUBLE_LINKED_LIST_APPEND_ITEM_UNSAFE(*head_ptr_full, page, aral_lock.prev, aral_lock.next);
1522 + page->aral_lock.head_ptr = head_ptr_full;
1523 + }
1524 + aral_unlock(ar);
1525 +
1526 + aral_race_unittest_hook.forced_entry = entry;
1527 + __atomic_store_n(&aral_race_unittest_hook.page_force_fully_used, true, __ATOMIC_RELEASE);
1528 +
1529 + aral_page_unlock(ar, page);
1530 + return entry;
1531 +}
1532 +
1533 +static int aral_detect_acquire_to_page_lock_race(void) {
1534 + int errors = 0;
1535 + bool allocator_entry_marked = false;
1536 + ARAL_PAGE *allocator_page = NULL;
1537 + ARAL *ar = aral_create("aral-race-test",
1538 + sizeof(struct aral_unittest_entry),
1539 + 0,
1540 + 0,
1541 + NULL,
1542 + "aral-race-test",
1543 + NULL, false, false, false);
1544 +
1545 + size_t page_elements = aral_elements_in_page_size(ar, ar->ops[0].adders.allocation_size);
1546 + struct aral_unittest_entry **filled = callocz(page_elements, sizeof(*filled));
1547 + struct aral_race_unittest_allocator allocator = {
1548 + .ar = ar,
1549 + .entry = NULL,
1550 + };
1551 +
1552 + for(size_t i = 0; i < page_elements - 1; i++)
1553 + filled[i] = unittest_aral_malloc(ar, false);
1554 +
1555 + aral_race_unittest_hook = (struct aral_race_unittest_hook) {
1556 + .ar = ar,
1557 + .enabled = true,
1558 + };
1559 +
1560 + ND_THREAD *thread = nd_thread_create("ARALRACE", NETDATA_THREAD_OPTION_DONT_LOG,
1561 + aral_race_unittest_allocator_thread, &allocator);
1562 +
1563 + if(!thread) {
1564 + fprintf(stderr, "ARAL race unittest: failed to create allocator thread.\n");
1565 + errors++;
1566 + }
1567 +
1568 + if(thread && !aral_unittest_wait_for_flag(&aral_race_unittest_hook.first_allocator_waiting, 5 * USEC_PER_SEC)) {
1569 + fprintf(stderr, "ARAL race unittest: timed out waiting for the first allocator to pause.\n");
1570 + errors++;
1571 + }
1572 + else if(thread) {
1573 + if(!aral_race_unittest_hook.page) {
1574 + fprintf(stderr, "ARAL race unittest: paused allocator did not publish its target page.\n");
1575 + errors++;
1576 + }
1577 + else
1578 + aral_race_unittest_force_page_full(ar, aral_race_unittest_hook.page);
1579 + }
1580 +
1581 + __atomic_store_n(&aral_race_unittest_hook.release_first_allocator, true, __ATOMIC_RELEASE);
1582 + if(thread)
1583 + nd_thread_join(thread);
1584 + __atomic_store_n(&aral_race_unittest_hook.enabled, false, __ATOMIC_RELEASE);
1585 +
1586 + if(!allocator.entry) {
1587 + fprintf(stderr, "ARAL race unittest: paused allocator failed to complete its allocation.\n");
1588 + errors++;
1589 + }
1590 + else
1591 + allocator_page = aral_get_page_pointer_after_element___do_NOT_have_aral_lock(ar, allocator.entry, &allocator_entry_marked);
1592 +
1593 + (void)allocator_entry_marked;
1594 +
1595 + if(ar->aral_lock.pages_full == NULL) {
1596 + fprintf(stderr, "ARAL race unittest: expected the original page to become full during the race.\n");
1597 + errors++;
1598 + }
1599 +
1600 + if(!__atomic_load_n(&aral_race_unittest_hook.page_force_fully_used, __ATOMIC_ACQUIRE)) {
1601 + fprintf(stderr, "ARAL race unittest: failed to force the page into the fully-used state.\n");
1602 + errors++;
1603 + }
1604 +
1605 + if(errors == 0 && allocator_page == aral_race_unittest_hook.page) {
1606 + fprintf(stderr, "ARAL race unittest: allocator retried on the forced-full page instead of a new page.\n");
1607 + errors++;
1608 + }
1609 +
1610 + if(aral_race_unittest_hook.forced_entry)
1611 + aral_freez(ar, aral_race_unittest_hook.forced_entry);
1612 +
1613 + if(allocator.entry)
1614 + aral_freez(ar, allocator.entry);
1615 +
1616 + for(size_t i = 0; i < page_elements - 1; i++) {
1617 + if(filled[i])
1618 + aral_freez(ar, filled[i]);
1619 + }
1620 +
1621 + freez(filled);
1622 + aral_destroy(ar);
1623 + aral_race_unittest_hook = (struct aral_race_unittest_hook) { 0 };
1624 +
1625 + return errors;
1626 +}
1627 +#endif
1628 +
1629 static void aral_test_thread(void *ptr) {
1630 struct aral_unittest_config *auc = ptr;
1631 ARAL *ar = auc->ar;
@@ -1582,12 +1785,11 @@ int aral_stress_test(size_t threads, size_t elements, size_t seconds) {
1785 __atomic_add_fetch(&auc.errors, 1, __ATOMIC_RELAXED);
1786 }
1787
1585 - netdata_log_info("ARAL: did %zu malloc, %zu free, "
1586 - "using %zu threads, in %"PRIu64" usecs",
1587 - __atomic_load_n(&auc.ar->atomic.user_malloc_operations, __ATOMIC_RELAXED),
1588 - __atomic_load_n(&auc.ar->atomic.user_free_operations, __ATOMIC_RELAXED),
1589 - threads,
1590 - ended_ut - started_ut);
1788 + fprintf(stderr, "ARAL: did %zu malloc, %zu free, using %zu threads, in %"PRIu64" usecs\n",
1789 + __atomic_load_n(&auc.ar->atomic.user_malloc_operations, __ATOMIC_RELAXED),
1790 + __atomic_load_n(&auc.ar->atomic.user_free_operations, __ATOMIC_RELAXED),
1791 + threads,
1792 + ended_ut - started_ut);
1793
1794 aral_destroy(auc.ar);
1795
@@ -1596,6 +1798,16 @@ int aral_stress_test(size_t threads, size_t elements, size_t seconds) {
1798
1799 int aral_unittest(size_t elements) {
1800 const char *cache_dir = "/tmp/";
1801 +#ifdef NETDATA_INTERNAL_CHECKS
1802 + int errors = aral_detect_acquire_to_page_lock_race();
1803 +
1804 + if(errors) {
1805 + fprintf(stderr, "ARAL unittest: FAILED (%d errors)\n", errors);
1806 + return errors;
1807 + }
1808 +#else
1809 + int errors = 0;
1810 +#endif
1811
1812 struct aral_unittest_config auc = {
1813 .single_threaded = true,
@@ -1616,7 +1828,10 @@ int aral_unittest(size_t elements) {
1828
1829 aral_destroy(auc.ar);
1830
1619 - int errors = aral_stress_test(2, elements, 10);
1831 + errors += aral_stress_test(2, elements, 10);
1832 +
1833 + int total_errors = auc.errors + errors;
1834 + fprintf(stderr, "ARAL unittest: %s (%d errors)\n", total_errors ? "FAILED" : "PASSED", total_errors);
1835
1621 - return auc.errors + errors;
1836 + return total_errors;
1837 }