Skip to content

Commit

Permalink
Remove SequencedWorkerPool::PostWorkerTask().
Browse files Browse the repository at this point in the history 
BUG=667892

Review-Url: https://codereview.chromium.org/2904253002
Cr-Commit-Position: refs/heads/master@{#475166}
  • Loading branch information
@fdoray
fdoray authored and Commit bot committed May 26, 2017
1 parent db4ba3e commit 03aa6d0
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 59 deletions.
7 changes: 0 additions & 7 deletions base/threading/sequenced_worker_pool.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1545,13 +1545,6 @@ SequencedWorkerPool::GetTaskRunnerWithShutdownBehavior(
return new SequencedWorkerPoolTaskRunner(this, shutdown_behavior);
}

bool SequencedWorkerPool::PostWorkerTask(
const tracked_objects::Location& from_here,
OnceClosure task) {
return inner_->PostTask(NULL, SequenceToken(), BLOCK_SHUTDOWN, from_here,
std::move(task), TimeDelta());
}

bool SequencedWorkerPool::PostWorkerTaskWithShutdownBehavior(
const tracked_objects::Location& from_here,
OnceClosure task,
Expand Down
25 changes: 1 addition & 24 deletions base/threading/sequenced_worker_pool.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ template <class T> class DeleteHelper;
// You can make named sequence tokens to make it easier to share a token
// across different components.
//
// You can also post tasks to the pool without ordering using PostWorkerTask.
// You can also post tasks to the pool without ordering using PostTask.
// These will be executed in an unspecified order. The order of execution
// between tasks with different sequence tokens is also unspecified.
//
Expand Down Expand Up @@ -255,29 +255,6 @@ class BASE_EXPORT SequencedWorkerPool : public TaskRunner {
scoped_refptr<TaskRunner> GetTaskRunnerWithShutdownBehavior(
WorkerShutdown shutdown_behavior) WARN_UNUSED_RESULT;

// Posts the given task for execution in the worker pool. Tasks posted with
// this function will execute in an unspecified order on a background thread.
// Returns true if the task was posted. If your tasks have ordering
// requirements, see PostSequencedWorkerTask().
//
// This class will attempt to delete tasks that aren't run
// (non-block-shutdown semantics) but can't guarantee that this happens. If
// all worker threads are busy running CONTINUE_ON_SHUTDOWN tasks, there
// will be no workers available to delete these tasks. And there may be
// tasks with the same sequence token behind those CONTINUE_ON_SHUTDOWN
// tasks. Deleting those tasks before the previous one has completed could
// cause nondeterministic crashes because the task could be keeping some
// objects alive which do work in their destructor, which could voilate the
// assumptions of the running task.
//
// The task will be guaranteed to run to completion before shutdown
// (BLOCK_SHUTDOWN semantics).
//
// Returns true if the task was posted successfully. This may fail during
// shutdown regardless of the specified ShutdownBehavior.
bool PostWorkerTask(const tracked_objects::Location& from_here,
OnceClosure task);

// Same as PostWorkerTask but allows specification of the shutdown behavior.
bool PostWorkerTaskWithShutdownBehavior(
const tracked_objects::Location& from_here,
Expand Down
55 changes: 27 additions & 28 deletions base/threading/sequenced_worker_pool_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -322,9 +322,8 @@ class SequencedWorkerPoolTest
// workers to be created.
ThreadBlocker blocker;
for (size_t i = 0; i < kNumWorkerThreads; i++) {
pool()->PostWorkerTask(
FROM_HERE,
base::BindOnce(&TestTracker::BlockTask, tracker(), -1, &blocker));
pool()->PostTask(FROM_HERE, base::BindOnce(&TestTracker::BlockTask,
tracker(), -1, &blocker));
}
tracker()->WaitUntilTasksBlocked(kNumWorkerThreads);

Expand Down Expand Up @@ -453,13 +452,13 @@ TEST_P(SequencedWorkerPoolTest, NamedTokens) {
// Tests that posting a bunch of tasks (many more than the number of worker
// threads) runs them all.
TEST_P(SequencedWorkerPoolTest, LotsOfTasks) {
pool()->PostWorkerTask(FROM_HERE,
base::BindOnce(&TestTracker::SlowTask, tracker(), 0));
pool()->PostTask(FROM_HERE,
base::BindOnce(&TestTracker::SlowTask, tracker(), 0));

const size_t kNumTasks = 20;
for (size_t i = 1; i < kNumTasks; i++) {
pool()->PostWorkerTask(
FROM_HERE, base::BindOnce(&TestTracker::FastTask, tracker(), i));
pool()->PostTask(FROM_HERE,
base::BindOnce(&TestTracker::FastTask, tracker(), i));
}

std::vector<int> result = tracker()->WaitUntilTasksComplete(kNumTasks);
Expand All @@ -475,15 +474,15 @@ TEST_P(SequencedWorkerPoolTest, LotsOfTasksTwoPools) {
SequencedWorkerPoolOwner pool2(kNumWorkerThreads, "test2");

base::Closure slow_task = base::Bind(&TestTracker::SlowTask, tracker(), 0);
pool1.pool()->PostWorkerTask(FROM_HERE, slow_task);
pool2.pool()->PostWorkerTask(FROM_HERE, slow_task);
pool1.pool()->PostTask(FROM_HERE, slow_task);
pool2.pool()->PostTask(FROM_HERE, slow_task);

const size_t kNumTasks = 20;
for (size_t i = 1; i < kNumTasks; i++) {
base::Closure fast_task =
base::Bind(&TestTracker::FastTask, tracker(), i);
pool1.pool()->PostWorkerTask(FROM_HERE, fast_task);
pool2.pool()->PostWorkerTask(FROM_HERE, fast_task);
pool1.pool()->PostTask(FROM_HERE, fast_task);
pool2.pool()->PostTask(FROM_HERE, fast_task);
}

std::vector<int> result =
Expand All @@ -498,9 +497,9 @@ TEST_P(SequencedWorkerPoolTest, Sequence) {
const size_t kNumBackgroundTasks = kNumWorkerThreads - 1;
ThreadBlocker background_blocker;
for (size_t i = 0; i < kNumBackgroundTasks; i++) {
pool()->PostWorkerTask(FROM_HERE,
base::BindOnce(&TestTracker::BlockTask, tracker(), i,
&background_blocker));
pool()->PostTask(FROM_HERE,
base::BindOnce(&TestTracker::BlockTask, tracker(), i,
&background_blocker));
}
tracker()->WaitUntilTasksBlocked(kNumBackgroundTasks);

Expand Down Expand Up @@ -559,8 +558,8 @@ TEST_P(SequencedWorkerPoolTest, DISABLED_IgnoresAfterShutdown) {
EnsureAllWorkersCreated();
ThreadBlocker blocker;
for (size_t i = 0; i < kNumWorkerThreads; i++) {
pool()->PostWorkerTask(FROM_HERE, base::BindOnce(&TestTracker::BlockTask,
tracker(), i, &blocker));
pool()->PostTask(FROM_HERE, base::BindOnce(&TestTracker::BlockTask,
tracker(), i, &blocker));
}
tracker()->WaitUntilTasksBlocked(kNumWorkerThreads);

Expand Down Expand Up @@ -606,7 +605,7 @@ TEST_P(SequencedWorkerPoolTest, AllowsAfterShutdown) {
// Start tasks to take all the threads and block them.
const int kNumBlockTasks = static_cast<int>(kNumWorkerThreads);
for (int i = 0; i < kNumBlockTasks; ++i) {
EXPECT_TRUE(pool()->PostWorkerTask(
EXPECT_TRUE(pool()->PostTask(
FROM_HERE,
base::BindOnce(&TestTracker::BlockTask, tracker(), i, &blocker)));
}
Expand Down Expand Up @@ -670,7 +669,7 @@ TEST_P(SequencedWorkerPoolTest,
// Start tasks to take all the threads and block them.
const int kNumBlockTasks = static_cast<int>(kNumWorkerThreads);
for (int i = 0; i < kNumBlockTasks; ++i) {
EXPECT_TRUE(pool()->PostWorkerTask(
EXPECT_TRUE(pool()->PostTask(
FROM_HERE,
base::BindOnce(&TestTracker::BlockTask, tracker(), i, &blocker)));
}
Expand Down Expand Up @@ -726,8 +725,8 @@ TEST_P(SequencedWorkerPoolTest, DiscardOnShutdown) {
EnsureAllWorkersCreated();
ThreadBlocker blocker;
for (size_t i = 0; i < kNumWorkerThreads; i++) {
pool()->PostWorkerTask(FROM_HERE, base::BindOnce(&TestTracker::BlockTask,
tracker(), i, &blocker));
pool()->PostTask(FROM_HERE, base::BindOnce(&TestTracker::BlockTask,
tracker(), i, &blocker));
}
tracker()->WaitUntilTasksBlocked(kNumWorkerThreads);

Expand Down Expand Up @@ -994,14 +993,14 @@ TEST_P(SequencedWorkerPoolTest, FlushForTesting) {
pool()->PostDelayedTask(FROM_HERE,
base::BindOnce(&TestTracker::FastTask, tracker(), 0),
TimeDelta::FromMinutes(5));
pool()->PostWorkerTask(FROM_HERE,
base::BindOnce(&TestTracker::SlowTask, tracker(), 0));
pool()->PostTask(FROM_HERE,
base::BindOnce(&TestTracker::SlowTask, tracker(), 0));
const size_t kNumFastTasks = 20;
for (size_t i = 0; i < kNumFastTasks; i++) {
pool()->PostWorkerTask(
FROM_HERE, base::BindOnce(&TestTracker::FastTask, tracker(), 0));
pool()->PostTask(FROM_HERE,
base::BindOnce(&TestTracker::FastTask, tracker(), 0));
}
pool()->PostWorkerTask(
pool()->PostTask(
FROM_HERE, base::BindOnce(&TestTracker::PostAdditionalTasks, tracker(), 0,
base::RetainedRef(pool()), true));

Expand Down Expand Up @@ -1078,9 +1077,9 @@ TEST_P(SequencedWorkerPoolTest, GetWorkerPoolAndSequenceTokenForCurrentThread) {
token2, FROM_HERE,
base::BindOnce(&CheckWorkerPoolAndSequenceToken, pool(), token2));

pool()->PostWorkerTask(
FROM_HERE, base::BindOnce(&CheckWorkerPoolAndSequenceToken, pool(),
SequencedWorkerPool::SequenceToken()));
pool()->PostTask(FROM_HERE,
base::BindOnce(&CheckWorkerPoolAndSequenceToken, pool(),
SequencedWorkerPool::SequenceToken()));

pool()->FlushForTesting();
}
Expand Down

0 comments on commit 03aa6d0

Please sign in to comment.