Potentially infinite loop and store trace in unowned slot in CallTraceHashTable::putWithExistingId()#578
Potentially infinite loop and store trace in unowned slot in CallTraceHashTable::putWithExistingId()#578zhengyu123 wants to merge 5 commits into
Conversation
…eHashTable::putWithExistingId()
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 699ab3f1e3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
CI Test ResultsRun: #27231185436 | Commit:
Status Overview
Legend: ✅ passed | ❌ failed | ⚪ skipped | 🚫 cancelled Summary: Total: 32 | Passed: 32 | Failed: 0 Updated: 2026-06-09 19:57:25 UTC |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 53f193d0df
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9533310501
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| std::this_thread::sleep_for(std::chrono::milliseconds(50)); | ||
| } | ||
|
|
||
| bool ok = completed.load(); |
There was a problem hiding this comment.
If this timeout fires, the detached worker can keep running after the test returns while completed and other stack state captured by reference have been destroyed; if the regression is only very slow rather than permanently hung, the worker will later write through a dangling reference and can corrupt/crash subsequent tests instead of reporting a clean failure.
Useful? React with 👍 / 👎.
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0ba2856759
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (size >= capacity * 3 / 4) { | ||
| // Allocate new table with double capacity using LinearAllocator | ||
| LongHashTable* new_table = LongHashTable::allocate(table, capacity * 2, &_allocator); |
There was a problem hiding this comment.
When one thread has already swapped _table to an expanded table, other put() callers that loaded the old table can still reach this helper after incSize(). Since the new >= condition stays true for every later insert and the replacement table is allocated before the compare-exchange, each failed CAS leaves a full LongHashTable allocated in the LinearAllocator from the hot signal-handler path. Under concurrent high-cardinality traces once the old table is past 75% full, stale writers can burn megabytes per sample until the storage is cleared; recheck that _table is still this table before allocating, or only allocate on the threshold transition.
Useful? React with 👍 / 👎.
|
@copilot resolve the merge conflicts in this pull request |
Resolved the merge conflicts locally in commit |
What does this PR do?:
This PR fixes a corner case, that may result in infinite loop and/or store trace in unowned slot.
Motivation:
Improve stability.
Additional Notes:
How to test the change?:
For Datadog employees:
credentials of any kind, I've requested a review from
@DataDog/security-design-and-guidance.Unsure? Have a question? Request a review!