TaskScheduler: Bump thread priority during shutdown.
Bump the priority of BACKGROUND threads to NORMAL during shutdown.
This will prevent TaskTracker::Shutdown() from waiting for work to
be done by threads that never get a time slice.
BUG=553459
Review-Url: https://codereview.chromium.org/2161213002
Cr-Commit-Position: refs/heads/master@{#407147}
diff --git a/base/task_scheduler/scheduler_worker.cc b/base/task_scheduler/scheduler_worker.cc
index 5a2a7d6a..e989934 100644
--- a/base/task_scheduler/scheduler_worker.cc
+++ b/base/task_scheduler/scheduler_worker.cc
@@ -10,6 +10,7 @@
#include "base/logging.h"
#include "base/task_scheduler/task_tracker.h"
+#include "build/build_config.h"
namespace base {
namespace internal {
@@ -39,6 +40,11 @@
while (!outer_->task_tracker_->IsShutdownComplete() &&
!outer_->ShouldExitForTesting()) {
DCHECK(outer_);
+
+#if !defined(OS_LINUX)
+ UpdateThreadPriority(GetDesiredThreadPriority());
+#endif
+
// Get the sequence containing the next task to execute.
scoped_refptr<Sequence> sequence = outer_->delegate_->GetWork(outer_);
if (!sequence) {
@@ -91,17 +97,17 @@
private:
Thread(SchedulerWorker* outer)
- : outer_(outer),
- wake_up_event_(WaitableEvent::ResetPolicy::MANUAL,
- WaitableEvent::InitialState::NOT_SIGNALED) {
+ : outer_(outer),
+ wake_up_event_(WaitableEvent::ResetPolicy::MANUAL,
+ WaitableEvent::InitialState::NOT_SIGNALED),
+ current_thread_priority_(GetDesiredThreadPriority()) {
DCHECK(outer_);
}
void Initialize() {
constexpr size_t kDefaultStackSize = 0;
- PlatformThread::CreateWithPriority(kDefaultStackSize, this,
- &thread_handle_,
- outer_->thread_priority_);
+ PlatformThread::CreateWithPriority(kDefaultStackSize, this, &thread_handle_,
+ current_thread_priority_);
}
void WaitForWork() {
@@ -117,6 +123,31 @@
wake_up_event_.Reset();
}
+ // Returns the desired thread priority based on the worker priority and the
+ // current shutdown state.
+ ThreadPriority GetDesiredThreadPriority() {
+ DCHECK(outer_);
+
+ if (outer_->task_tracker_->HasShutdownStarted() &&
+ static_cast<int>(outer_->thread_priority_) <
+ static_cast<int>(ThreadPriority::NORMAL)) {
+ return ThreadPriority::NORMAL;
+ }
+ return outer_->thread_priority_;
+ }
+
+ // Increasing the thread priority requires the CAP_SYS_NICE capability on
+ // Linux.
+#if !defined(OS_LINUX)
+ void UpdateThreadPriority(ThreadPriority desired_thread_priority) {
+ if (desired_thread_priority == current_thread_priority_)
+ return;
+
+ PlatformThread::SetCurrentThreadPriority(desired_thread_priority);
+ current_thread_priority_ = desired_thread_priority;
+ }
+#endif // !defined(OS_LINUX)
+
PlatformThreadHandle thread_handle_;
SchedulerWorker* outer_;
@@ -124,6 +155,10 @@
// Event signaled to wake up this thread.
WaitableEvent wake_up_event_;
+ // Current priority of this thread. May be different from
+ // |outer_->thread_priority_| during shutdown.
+ ThreadPriority current_thread_priority_;
+
DISALLOW_COPY_AND_ASSIGN(Thread);
};
diff --git a/base/task_scheduler/scheduler_worker_unittest.cc b/base/task_scheduler/scheduler_worker_unittest.cc
index 437bbedfd..fb94edc 100644
--- a/base/task_scheduler/scheduler_worker_unittest.cc
+++ b/base/task_scheduler/scheduler_worker_unittest.cc
@@ -14,11 +14,14 @@
#include "base/macros.h"
#include "base/memory/ptr_util.h"
#include "base/synchronization/condition_variable.h"
+#include "base/synchronization/waitable_event.h"
#include "base/task_scheduler/scheduler_lock.h"
#include "base/task_scheduler/sequence.h"
#include "base/task_scheduler/task.h"
#include "base/task_scheduler/task_tracker.h"
+#include "base/threading/platform_thread.h"
#include "base/time/time.h"
+#include "build/build_config.h"
#include "testing/gtest/include/gtest/gtest.h"
namespace base {
@@ -421,6 +424,108 @@
worker->JoinForTesting();
}
+namespace {
+
+class ExpectThreadPriorityDelegate : public SchedulerWorker::Delegate {
+ public:
+ ExpectThreadPriorityDelegate()
+ : priority_verified_in_get_work_event_(
+ WaitableEvent::ResetPolicy::AUTOMATIC,
+ WaitableEvent::InitialState::NOT_SIGNALED),
+ expected_thread_priority_(ThreadPriority::BACKGROUND) {}
+
+ void SetExpectedThreadPriority(ThreadPriority expected_thread_priority) {
+ expected_thread_priority_ = expected_thread_priority;
+ }
+
+ void WaitForPriorityVerifiedInGetWork() {
+ priority_verified_in_get_work_event_.Wait();
+ }
+
+ // SchedulerWorker::Delegate:
+ void OnMainEntry(SchedulerWorker* worker) override { VerifyThreadPriority(); }
+ scoped_refptr<Sequence> GetWork(SchedulerWorker* worker) override {
+ VerifyThreadPriority();
+ priority_verified_in_get_work_event_.Signal();
+ return nullptr;
+ }
+ void ReEnqueueSequence(scoped_refptr<Sequence> sequence) override {}
+ TimeDelta GetSleepTimeout() override { return TimeDelta::Max(); }
+ bool CanDetach(SchedulerWorker* worker) override { return false; }
+
+ private:
+ void VerifyThreadPriority() {
+ AutoSchedulerLock auto_lock(expected_thread_priority_lock_);
+ EXPECT_EQ(expected_thread_priority_,
+ PlatformThread::GetCurrentThreadPriority());
+ }
+
+ // Signaled after GetWork() has verified the priority of the worker thread.
+ WaitableEvent priority_verified_in_get_work_event_;
+
+ // Synchronizes access to |expected_thread_priority_|.
+ SchedulerLock expected_thread_priority_lock_;
+
+ // Expected thread priority for the next call to OnMainEntry() or GetWork().
+ ThreadPriority expected_thread_priority_;
+
+ DISALLOW_COPY_AND_ASSIGN(ExpectThreadPriorityDelegate);
+};
+
+} // namespace
+
+// Increasing the thread priority requires the CAP_SYS_NICE capability on Linux.
+#if !defined(OS_LINUX)
+TEST(TaskSchedulerWorkerTest, BumpPriorityOfAliveThreadDuringShutdown) {
+ TaskTracker task_tracker;
+
+ std::unique_ptr<ExpectThreadPriorityDelegate> delegate(
+ new ExpectThreadPriorityDelegate);
+ ExpectThreadPriorityDelegate* delegate_raw = delegate.get();
+ delegate_raw->SetExpectedThreadPriority(ThreadPriority::BACKGROUND);
+
+ std::unique_ptr<SchedulerWorker> worker = SchedulerWorker::Create(
+ ThreadPriority::BACKGROUND, std::move(delegate), &task_tracker,
+ SchedulerWorker::InitialState::ALIVE);
+
+ // Verify that the initial thread priority is BACKGROUND.
+ worker->WakeUp();
+ delegate_raw->WaitForPriorityVerifiedInGetWork();
+
+ // Verify that the thread priority is bumped to NORMAL during shutdown.
+ delegate_raw->SetExpectedThreadPriority(ThreadPriority::NORMAL);
+ task_tracker.SetHasShutdownStartedForTesting();
+ worker->WakeUp();
+ delegate_raw->WaitForPriorityVerifiedInGetWork();
+
+ worker->JoinForTesting();
+}
+#endif // defined(OS_LINUX)
+
+TEST(TaskSchedulerWorkerTest, BumpPriorityOfDetachedThreadDuringShutdown) {
+ TaskTracker task_tracker;
+
+ std::unique_ptr<ExpectThreadPriorityDelegate> delegate(
+ new ExpectThreadPriorityDelegate);
+ ExpectThreadPriorityDelegate* delegate_raw = delegate.get();
+ delegate_raw->SetExpectedThreadPriority(ThreadPriority::NORMAL);
+
+ // Create a DETACHED thread.
+ std::unique_ptr<SchedulerWorker> worker = SchedulerWorker::Create(
+ ThreadPriority::BACKGROUND, std::move(delegate), &task_tracker,
+ SchedulerWorker::InitialState::DETACHED);
+
+ // Pretend that shutdown has started.
+ task_tracker.SetHasShutdownStartedForTesting();
+
+ // Wake up the thread and verify that its priority is NORMAL when
+ // OnMainEntry() and GetWork() are called.
+ worker->WakeUp();
+ delegate_raw->WaitForPriorityVerifiedInGetWork();
+
+ worker->JoinForTesting();
+}
+
} // namespace
} // namespace internal
} // namespace base
diff --git a/base/task_scheduler/task_tracker.cc b/base/task_scheduler/task_tracker.cc
index 8817baa..1ebc81b 100644
--- a/base/task_scheduler/task_tracker.cc
+++ b/base/task_scheduler/task_tracker.cc
@@ -124,6 +124,7 @@
// This method can only be called once.
DCHECK(!shutdown_event_);
DCHECK(!num_block_shutdown_tasks_posted_during_shutdown_);
+ DCHECK(!state_->HasShutdownStarted());
shutdown_event_.reset(
new WaitableEvent(WaitableEvent::ResetPolicy::MANUAL,
@@ -214,14 +215,17 @@
AfterRunTask(shutdown_behavior);
}
+bool TaskTracker::HasShutdownStarted() const {
+ return state_->HasShutdownStarted();
+}
+
bool TaskTracker::IsShutdownComplete() const {
AutoSchedulerLock auto_lock(shutdown_lock_);
return shutdown_event_ && shutdown_event_->IsSignaled();
}
-bool TaskTracker::IsShuttingDownForTesting() const {
- AutoSchedulerLock auto_lock(shutdown_lock_);
- return shutdown_event_ && !shutdown_event_->IsSignaled();
+void TaskTracker::SetHasShutdownStartedForTesting() {
+ state_->StartShutdown();
}
bool TaskTracker::BeforePostTask(TaskShutdownBehavior shutdown_behavior) {
diff --git a/base/task_scheduler/task_tracker.h b/base/task_scheduler/task_tracker.h
index fc0901f..609edbc 100644
--- a/base/task_scheduler/task_tracker.h
+++ b/base/task_scheduler/task_tracker.h
@@ -44,12 +44,17 @@
// must have allowed |task| to be posted.
void RunTask(const Task* task);
- // Returns true if shutdown has completed.
+ // Returns true once shutdown has started (Shutdown() has been called but
+ // might not have returned).
+ bool HasShutdownStarted() const;
+
+ // Returns true if shutdown has completed (Shutdown() has returned).
bool IsShutdownComplete() const;
- // Returns true while shutdown is in progress (i.e. Shutdown() has been called
- // but hasn't returned).
- bool IsShuttingDownForTesting() const;
+ // Causes HasShutdownStarted() to return true. Unlike when Shutdown() returns,
+ // IsShutdownComplete() won't return true after this returns. Shutdown()
+ // cannot be called after this.
+ void SetHasShutdownStartedForTesting();
private:
class State;
diff --git a/base/task_scheduler/task_tracker_unittest.cc b/base/task_scheduler/task_tracker_unittest.cc
index 58f47b81..326f7aa0 100644
--- a/base/task_scheduler/task_tracker_unittest.cc
+++ b/base/task_scheduler/task_tracker_unittest.cc
@@ -132,10 +132,8 @@
ASSERT_FALSE(thread_calling_shutdown_);
thread_calling_shutdown_.reset(new ThreadCallingShutdown(&tracker_));
thread_calling_shutdown_->Start();
- while (!tracker_.IsShuttingDownForTesting() &&
- !tracker_.IsShutdownComplete()) {
+ while (!tracker_.HasShutdownStarted())
PlatformThread::YieldCurrentThread();
- }
}
void WaitForAsyncIsShutdownComplete() {
@@ -148,8 +146,8 @@
void VerifyAsyncShutdownInProgress() {
ASSERT_TRUE(thread_calling_shutdown_);
EXPECT_FALSE(thread_calling_shutdown_->has_returned());
+ EXPECT_TRUE(tracker_.HasShutdownStarted());
EXPECT_FALSE(tracker_.IsShutdownComplete());
- EXPECT_TRUE(tracker_.IsShuttingDownForTesting());
}
size_t NumTasksExecuted() {
