Commit 491b1a06 authored by Robert Liao's avatar Robert Liao Committed by Commit Bot

Fix Race in TaskSchedulerSingleThreadTaskRunnerManagerStartTest.PostTaskBeforeStart

This race should have been more common but it only reproed on iPhone 7
hardware running iOS 11 with a release version of base_unittests.
Setting DCHECK_ALWAYS_ON to true would cause the bug to stop reproing.

Because the test task signalled its WaitableEvent at the beginning of
the task instead of at the end, it was possible for the function
holding |manager_started| to return before the task had a chance to
check it.

The fix here moves the signalling to the end of the task, ensuring that
|manager_started| is alive when it is checked by the test task.

BUG=784051

Change-Id: I1e88395bcc45b5390af0c75be806a0a7301e483e
Reviewed-on: https://chromium-review.googlesource.com/773299
Commit-Queue: Robert Liao <robliao@chromium.org>
Reviewed-by: default avatarFrançois Doray <fdoray@chromium.org>
Cr-Commit-Position: refs/heads/master@{#517139}
parent 63fb9850
......@@ -19,7 +19,6 @@
#include "base/threading/platform_thread.h"
#include "base/threading/simple_thread.h"
#include "base/threading/thread.h"
#include "build/build_config.h"
#include "testing/gtest/include/gtest/gtest.h"
#if defined(OS_WIN)
......@@ -588,31 +587,24 @@ class TaskSchedulerSingleThreadTaskRunnerManagerStartTest
} // namespace
// TODO(crbug.com/784051): Reenable when no longer flaky.
#if defined(OS_IOS) && !TARGET_IPHONE_SIMULATOR
#define MAYBE_PostTaskBeforeStart FLAKY_PostTaskBeforeStart
#else
#define MAYBE_PostTaskBeforeStart PostTaskBeforeStart
#endif
// Verify that a task posted before Start() doesn't run until Start() is called.
TEST_F(TaskSchedulerSingleThreadTaskRunnerManagerStartTest,
MAYBE_PostTaskBeforeStart) {
PostTaskBeforeStart) {
AtomicFlag manager_started;
WaitableEvent task_running(WaitableEvent::ResetPolicy::MANUAL,
WaitableEvent::InitialState::NOT_SIGNALED);
WaitableEvent task_finished(WaitableEvent::ResetPolicy::MANUAL,
WaitableEvent::InitialState::NOT_SIGNALED);
single_thread_task_runner_manager_
->CreateSingleThreadTaskRunnerWithTraits(
"A", TaskTraits(), SingleThreadTaskRunnerThreadMode::DEDICATED)
->PostTask(
FROM_HERE,
BindOnce(
[](WaitableEvent* task_running, AtomicFlag* manager_started) {
task_running->Signal();
[](WaitableEvent* task_finished, AtomicFlag* manager_started) {
// The task should not run before Start().
EXPECT_TRUE(manager_started->IsSet());
task_finished->Signal();
},
Unretained(&task_running), Unretained(&manager_started)));
Unretained(&task_finished), Unretained(&manager_started)));
// Wait a little bit to make sure that the task doesn't run before start.
// Note: This test won't catch a case where the task runs between setting
......@@ -622,8 +614,8 @@ TEST_F(TaskSchedulerSingleThreadTaskRunnerManagerStartTest,
manager_started.Set();
single_thread_task_runner_manager_->Start();
// This should not hang if the task runs after Start().
task_running.Wait();
// Wait for the task to complete to keep |manager_started| alive.
task_finished.Wait();
}
} // namespace internal
......
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