Add test for ARAL race condition and fix - #22212
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
No issues found across 1 file
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Architecture diagram
sequenceDiagram
participant T as Allocator Thread
participant AL as ARAL Logic
participant H as Test Hook (Internal Checks)
participant P as ARAL Page
participant O as Racing Thread
T->>AL: aral_get_first_page_with_a_free_slot()
rect rgb(240,240,240)
Note over AL: NEW: retry_acquisition loop
AL->>AL: aral_acquire_first_page()
AL-->>AL: Target Page Found (Refcount++)
opt NETDATA_INTERNAL_CHECKS enabled
AL->>H: NEW: aral_unittest_wait_for_race_window()
Note over H,O: Allocator thread pauses here
O->>P: Racing thread fills last slot
O->>P: Page moved to 'Full' list
H-->>AL: Release paused allocator
end
AL->>P: aral_page_lock(page)
alt Page has free slots (Happy Path)
AL-->>T: Return Page for allocation
else Page is full (Race Condition detected)
Note right of AL: Detected via !page_lock.free_elements
AL->>P: aral_page_unlock(page)
AL->>P: aral_page_release(page) (Refcount--)
AL->>AL: CHANGED: goto retry_acquisition
end
end
stelfrag
marked this pull request as ready for review
April 16, 2026 10:09
stelfrag
marked this pull request as draft
April 16, 2026 10:09
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a deterministic internal-checks unit test intended to reproduce an ARAL allocator race, and updates ARAL page acquisition logic to safely retry when a page becomes full between “acquire” and “page lock”.
Changes:
- Add
NETDATA_INTERNAL_CHECKS-gated race-test hook + unit test (aral_detect_acquire_to_page_lock_race()). - Fix allocator race by retrying if
free_elements == 0after locking the selected page. - Tighten
aral_page_acquire()by rolling back the refcount increment when acquisition is invalid (rf <= 0).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…improve readability
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…and ensure consistent page state handling (unittest)
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…ions and improve page state validation
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…eardown - Add error handling for ARAL allocator thread creation. - Prevent null thread dereference and streamline cleanup with conditional `nd_thread_join`.
stelfrag
marked this pull request as ready for review
April 16, 2026 14:19
thiagoftsm
approved these changes
Apr 16, 2026
thiagoftsm
left a comment
Contributor
There was a problem hiding this comment.
PR is working as expected locally. LGTM!
nedi-app Bot
pushed a commit
that referenced
this pull request
Apr 24, 2026
* 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`.
Merged
Ferroin
pushed a commit
that referenced
this pull request
Jul 15, 2026
* 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`. (cherry picked from commit 9f7887e)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Summary by cubic
Fixes an ARAL allocator race where a page chosen with a free slot becomes full before it’s locked by adding a lock-time retry and a refcount guard. Adds targeted unit and stress tests to reproduce the race and validate retried allocations and page list transitions.
Bug Fixes
free_elementsis 0 after locking; release + reacquire safely.aral_page_acquireto roll back and fail on non-positive refcounts.New Features
aral_detect_acquire_to_page_lock_raceand clean teardown (thread error handling, conditional join).fprintf; ensures forced-full page moves to the full list and validates allocator retry to a new page.Written for commit 4aa380e. Summary will update on new commits.