Deflake TaskDispatchThreadTest.MultipleDelayedTasksOrder - #57830
Closed
cortinico wants to merge 1 commit into
Closed
Deflake TaskDispatchThreadTest.MultipleDelayedTasksOrder#57830cortinico wants to merge 1 commit into
cortinico wants to merge 1 commit into
Conversation
Summary: WARNING: Generated by Autopilot (alpha) — review carefully, verify the underlying claim before accepting. Agent: React Native Oncall Tests Fixer | Trajectory: https://www.internalfb.com/intern/devai/devmate/inspector/0bb8944e-85d4-4a86-ac19-5147b3b0a48f/ | SC job: https://www.internalfb.com/intern/sandcastle/instance/36028799852645402/ --- `TaskDispatchThreadTest/MultipleDelayedTasksOrder` (test_id 562950261618811) is a timing-sensitive GTest that schedules two delayed tasks (at 50ms and 100ms) and then races a fixed `std::this_thread::sleep_for(120ms)` against the 100ms deadline before asserting `results.size() == 2`. Only 20ms of slack covers the later task, so under CI load / emulator scheduling jitter the second task has not run when the assertion fires, producing the observed `results.size()` is 0 or 1 failure at line 79. TestX reports a 95.7% simple flakiness score for this test, and it has a long history of auto-closed FLAKY issues; it recently surfaced as a hard FAILURE (issue 233255291). Replace the fixed sleep with deterministic synchronization on the later task using the same `std::promise`/`future.wait_for` pattern already used by `RunAsyncWithDelay` in this file. Because both tasks run serially on the single dispatch thread in deadline order, blocking until the 100ms task completes guarantees the 50ms task has already run, and waiting on the future establishes the happens-before needed to read `results` safely. Differential Revision: D114605936
|
@cortinico has exported this pull request. If you are a Meta employee, you can view the originating Diff in D114605936. |
|
This pull request has been merged in 0b3ca28. |
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:
WARNING: Generated by Autopilot (alpha) — review carefully, verify the underlying claim before accepting.
Agent: React Native Oncall Tests Fixer | Trajectory: https://www.internalfb.com/intern/devai/devmate/inspector/0bb8944e-85d4-4a86-ac19-5147b3b0a48f/ | SC job: https://www.internalfb.com/intern/sandcastle/instance/36028799852645402/
TaskDispatchThreadTest/MultipleDelayedTasksOrder(test_id 562950261618811)is a timing-sensitive GTest that schedules two delayed tasks (at 50ms and
100ms) and then races a fixed
std::this_thread::sleep_for(120ms)againstthe 100ms deadline before asserting
results.size() == 2. Only 20ms of slackcovers the later task, so under CI load / emulator scheduling jitter the
second task has not run when the assertion fires, producing the observed
results.size()is 0 or 1 failure at line 79. TestX reports a 95.7% simpleflakiness score for this test, and it has a long history of auto-closed
FLAKY issues; it recently surfaced as a hard FAILURE (issue 233255291).
Replace the fixed sleep with deterministic synchronization on the later
task using the same
std::promise/future.wait_forpattern already used byRunAsyncWithDelayin this file. Because both tasks run serially on thesingle dispatch thread in deadline order, blocking until the 100ms task
completes guarantees the 50ms task has already run, and waiting on the future
establishes the happens-before needed to read
resultssafely.Differential Revision: D114605936