Commit e2bfbf32 authored by Francois Doray's avatar Francois Doray Committed by Commit Bot

[sequence_manager] Fix race in SequenceManagerPerfTest.

Tests involving TaskScheduler accessed the members of
SameThreadTaskSource from different threads without synchronization.
This CL fixes the issue by making sure that the initial invocation of
TestTask() happens on the same thread as other invocations.

TSAN report before this CL:

WARNING: ThreadSanitizer: data race (pid=126426)
  Write of size 4 at 0x7b1400001c0c by thread T4:
    #0 base::sequence_manager::SameThreadTaskSource::TestTask() base/task/sequence_manager/sequence_manager_perftest.cc:311:25
    #1 Invoke<void (base::sequence_manager::SameThreadTaskSource::*)(), base::sequence_manager::SameThreadTaskSource *> base/bind_internal.h:516:12
    #2 MakeItSo<void (base::sequence_manager::SameThreadTaskSource::*const &)(), base::sequence_manager::SameThreadTaskSource *> base/bind_internal.h:616
    #3 RunImpl<void (base::sequence_manager::SameThreadTaskSource::*const &)(), const std::__1::tuple<base::internal::UnretainedWrapper<base::sequence_manager::SameThreadTaskSource> > &, 0> base/bind_internal.h:689
    #4 base::internal::Invoker<base::internal::BindState<void (base::sequence_manager::SameThreadTaskSource::*)(), base::internal::UnretainedWrapper<base::sequence_manager::SameThreadTaskSource> >, void ()>::Run(base::internal::BindStateBase*) base/bind_internal.h:671
    #5 Run base/callback.h:99:12
    #6 base::debug::TaskAnnotator::RunTask(char const*, base::PendingTask*) base/debug/task_annotator.cc:99
    #7 base::internal::TaskTracker::RunOrSkipTask(base::internal::Task, base::internal::Sequence*, bool) base/task/task_scheduler/task_tracker.cc:641:23
    #8 base::internal::TaskTrackerPosix::RunOrSkipTask(base::internal::Task, base::internal::Sequence*, bool) base/task/task_scheduler/task_tracker_posix.cc:23:16
    #9 base::internal::TaskTracker::RunAndPopNextTask(scoped_refptr<base::internal::Sequence>, base::internal::CanScheduleSequenceObserver*) base/task/task_scheduler/task_tracker.cc:496:3
    #10 base::internal::SchedulerWorker::RunWorker() base/task/task_scheduler/scheduler_worker.cc:333:24
    #11 base::internal::SchedulerWorker::RunSharedWorker() base/task/task_scheduler/scheduler_worker.cc:237:3
    #12 base::internal::SchedulerWorker::ThreadMain() base/task/task_scheduler/scheduler_worker.cc:207:7
    #13 base::(anonymous namespace)::ThreadFunc(void*) base/threading/platform_thread_posix.cc:81:13

  Previous write of size 4 at 0x7b1400001c0c by main thread:
    #0 TestTask base/task/sequence_manager/sequence_manager_perftest.cc:330:27
    #1 base::sequence_manager::SameThreadTaskSource::Start() base/task/sequence_manager/sequence_manager_perftest.cc:297
    #2 base::sequence_manager::SingleThreadImmediateTestCase::Start() base/task/sequence_manager/sequence_manager_perftest.cc:408:41
    #3 base::sequence_manager::SequenceManagerPerfTest::Benchmark(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, base::sequence_manager::TestCase*) base/task/sequence_manager/sequence_manager_perftest.cc:636:15
    #4 base::sequence_manager::SequenceManagerPerfTest_PostImmediateTasks_OneQueue_Test::TestBody() base/task/sequence_manager/sequence_manager_perftest.cc:714:3
    #5 HandleExceptionsInMethodIfSupported<testing::Test, void> third_party/googletest/src/googletest/src/gtest.cc
    #6 testing::Test::Run() third_party/googletest/src/googletest/src/gtest.cc:2522
    #7 testing::TestInfo::Run() third_party/googletest/src/googletest/src/gtest.cc:2703:11
    #8 testing::TestCase::Run() third_party/googletest/src/googletest/src/gtest.cc:2825:28
    #9 testing::internal::UnitTestImpl::RunAllTests() third_party/googletest/src/googletest/src/gtest.cc:5227:43
    #10 HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool> third_party/googletest/src/googletest/src/gtest.cc
    #11 testing::UnitTest::Run() third_party/googletest/src/googletest/src/gtest.cc:4835
    #12 RUN_ALL_TESTS third_party/googletest/src/googletest/include/gtest/gtest.h:2369:46
    #13 base::TestSuite::Run() base/test/test_suite.cc:294
    #14 main base/test/run_all_perftests.cc:8:42

TSAN report after this CL:
  No races found.

Change-Id: I03b816a01fe33eed86966f89cb1ad1b9ab34c2e3
Reviewed-on: https://chromium-review.googlesource.com/c/1340534Reviewed-by: default avatarAlex Clarke <alexclarke@chromium.org>
Commit-Queue: François Doray <fdoray@chromium.org>
Cr-Commit-Position: refs/heads/master@{#609276}
parent 3ac04b3b
......@@ -11,6 +11,7 @@
#include "base/message_loop/message_loop.h"
#include "base/message_loop/message_pump_default.h"
#include "base/run_loop.h"
#include "base/sequence_checker.h"
#include "base/single_thread_task_runner.h"
#include "base/strings/stringprintf.h"
#include "base/synchronization/condition_variable.h"
......@@ -288,13 +289,17 @@ class SameThreadTaskSource : public TaskSource {
num_tasks_(num_tasks),
task_closure_(
BindRepeating(&SameThreadTaskSource::TestTask, Unretained(this))),
task_runners_(std::move(task_runners)) {}
task_runners_(std::move(task_runners)) {
DETACH_FROM_SEQUENCE(sequence_checker_);
}
void Start() override {
num_tasks_in_flight_ = 1;
num_tasks_to_post_ = num_tasks_;
num_tasks_to_run_ = num_tasks_;
TestTask();
// Post the initial task instead of running it synchronously to ensure that
// all invocations happen on the same sequence.
PostTask(0);
}
protected:
......@@ -303,6 +308,8 @@ class SameThreadTaskSource : public TaskSource {
virtual void SignalDone() = 0;
void TestTask() {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
if (--num_tasks_to_run_ == 0) {
SignalDone();
return;
......@@ -340,6 +347,7 @@ class SameThreadTaskSource : public TaskSource {
unsigned int num_tasks_in_flight_;
unsigned int num_tasks_to_post_;
unsigned int num_tasks_to_run_;
SEQUENCE_CHECKER(sequence_checker_);
};
class CrossThreadTaskSource : public TaskSource {
......
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