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 2 commits into
Conversation
…eHashTable::putWithExistingId()
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".
| } else { | ||
| // No more slot, break out | ||
| break; |
There was a problem hiding this comment.
Record overflow when preservation exhausts slots
When the scratch table is full during liveness preservation, this new exit path just breaks out and drops the preserved trace. This can happen because putWithExistingId() never expands the scratch/standby table, while the active table can grow beyond the initial 65k slots; if more live traces are preserved than fit in scratch, the next collect() will emit only the entries that fit and no overflow sentinel because _overflow was not incremented. Please handle this the same way as put()'s full-probe path so callers can at least see the overflow trace instead of silently losing live trace continuity.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
@jbachorik Any reason it does not expand table here?
CI Test ResultsRun: #27016835318 | Commit:
Status Overview
Legend: ✅ passed | ❌ failed | ⚪ skipped | 🚫 cancelled Summary: Total: 32 | Passed: 32 | Failed: 0 Updated: 2026-06-05 13:29:08 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".
|
|
||
| bool ok = done.load(); | ||
| if (!ok) { | ||
| t.detach(); |
There was a problem hiding this comment.
Avoid detaching a worker that still uses fixture state
If this timeout ever fires (the regression this test is meant to catch, or just a slow sanitizer/CI run), t.detach() lets the lambda continue executing storage->processTraces() and then write done while the test returns and TearDown() destroys storage and the stack locals captured by reference. That turns the intended timeout failure into a use-after-free/crash or a background thread corrupting later tests; use a lifetime-safe watchdog strategy instead of detaching a thread that captured fixture state.
Useful? React with 👍 / 👎.
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!