Commit 0181d8b3 authored by Francois Doray's avatar Francois Doray Committed by Commit Bot

Fix TaskSchedulerWorkerPoolHistogramTest.NumTasksBeforeCleanup failure on Fuschia.

This test assumes that 3 tasks posted to the same sequence from the
main thread will be scheduled on the same worker. This is incorrect
because of this:

1. Worker #1: Runs a tasks and empties the sequence, without adding
   itself to the idle stack yet.
2. Posting thread: Posts another task to the now empty sequence.
   Wakes up a new worker, since worker #1 isn't on the idle stack yet.
3. Worker #2: Runs the tasks, violating the expectation that the
   3 initial tasks run on the same worker.

This CL fixes the issue by starting the pool *after* the 3 tasks
have been posted.

Bug: 844009
Change-Id: Idcc74e8bea90b94ecba8e3a52abc4091c89044b2
Reviewed-on: https://chromium-review.googlesource.com/1064016Reviewed-by: default avatarGabriel Charette <gab@chromium.org>
Commit-Queue: François Doray <fdoray@chromium.org>
Cr-Commit-Position: refs/heads/master@{#559534}
parent aa703718
...@@ -688,11 +688,7 @@ TEST_F(TaskSchedulerWorkerPoolHistogramTest, ...@@ -688,11 +688,7 @@ TEST_F(TaskSchedulerWorkerPoolHistogramTest,
} }
TEST_F(TaskSchedulerWorkerPoolHistogramTest, NumTasksBeforeCleanup) { TEST_F(TaskSchedulerWorkerPoolHistogramTest, NumTasksBeforeCleanup) {
// Strictly use two workers for this test to avoid depending on the CreateWorkerPool();
// scheduler's logic to always keep one extra idle worker.
constexpr size_t kTwoWorkers = 2;
CreateAndStartWorkerPool(kReclaimTimeForCleanupTests, kTwoWorkers);
auto histogrammed_thread_task_runner = auto histogrammed_thread_task_runner =
worker_pool_->CreateSequencedTaskRunnerWithTraits( worker_pool_->CreateSequencedTaskRunnerWithTraits(
{WithBaseSyncPrimitives()}); {WithBaseSyncPrimitives()});
...@@ -736,6 +732,23 @@ TEST_F(TaskSchedulerWorkerPoolHistogramTest, NumTasksBeforeCleanup) { ...@@ -736,6 +732,23 @@ TEST_F(TaskSchedulerWorkerPoolHistogramTest, NumTasksBeforeCleanup) {
Unretained(&thread_ref), Unretained(&cleanup_thread_running), Unretained(&thread_ref), Unretained(&cleanup_thread_running),
Unretained(&cleanup_thread_continue))); Unretained(&cleanup_thread_continue)));
// Start the worker pool with 2 workers, to avoid depending on the scheduler's
// logic to always keep one extra idle worker.
//
// The pool is started after the 3 initial tasks have been posted to ensure
// that they are scheduled on the same worker. If the tasks could run as they
// are posted, there would be a chance that:
// 1. Worker #1: Runs a tasks and empties the sequence, without adding
// itself to the idle stack yet.
// 2. Posting thread: Posts another task to the now empty sequence. Wakes
// up a new worker, since worker #1 isn't on the idle
// stack yet.
// 3: Worker #2: Runs the tasks, violating the expectation that the 3
// initial tasks run on the same worker.
constexpr size_t kTwoWorkers = 2;
StartWorkerPool(kReclaimTimeForCleanupTests, kTwoWorkers);
// Wait until the 3rd task is scheduled.
cleanup_thread_running.Wait(); cleanup_thread_running.Wait();
// To allow the SchedulerWorker associated with // To allow the SchedulerWorker associated with
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment