Prevent smoke-test teardown race - #12455
Conversation
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Codex Review: Didn't find any major issues. You're on a roll. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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.
More details
The teardown loop now uses the count from ThreadGroup.enumerate(). It does not read null array entries when a thread stops during teardown.
🤖 Datadog Autotest · Commit da93a6f · What is Autotest? · @DataDog review to ask questions · Any feedback? Reach out in #autotest
🟢 Java Benchmark SLOs — All performance SLOs passed
PR vs. master results
Commit: Load and DaCapo benchmarks can be triggered manually in the GitLab pipeline. Results will appear in the Benchmarking Platform UI after completion. |
What Does This Do
Use the number of threads actually returned by
ThreadGroup.enumerate()when joining smoke-test output readers during teardown.Add a deterministic regression test that models an output thread disappearing between
activeCount()andenumerate().Motivation
ThreadGroup.activeCount()is only an estimate. The teardown code sized an array from that estimate, ignored the smaller count returned byenumerate(), and attempted to join every array entry. This could dereference an unused null entry after an output reader exited.Additional Notes
The failure was flaky because teardown first interrupts the output-reader thread. If that thread exited after
activeCount()reported it but beforeenumerate()populated the array, the array retained a null tail entry:Whether the reader exited inside that narrow window depended on JVM thread scheduling, so the same test could pass on retry. Joining only indexes below the count returned by
enumerate()follows the API contract and removes the race.Validation:
./gradlew :dd-smoke-tests:test -PtestJvm=11 --tests datadog.smoketest.ErrorLogSmokeTest --tests datadog.smoketest.OutputThreadsTest— 2 passed./gradlew :dd-smoke-tests:test -PtestJvm=11— 60 passed./gradlew :dd-smoke-tests:spotlessJavaCheck— passedNo public API or user-facing behavior changes.
Contributor Checklist
type:and (comp:orinst:) labels in addition to any other useful labelsclose,fix, or any linking keywords when referencing an issueUse
solvesinstead, and assign the PR milestone to the issueJira ticket: N/A