Commit 14bbcc9d authored by Alexander Griboedov's avatar Alexander Griboedov Committed by Commit Bot

Fix NonMainThreadTaskQueue leak

NonMainThreadTaskQueue holds ptr of the voter. Task voter in its turn
holds refptr of TaskQueue it was created by. This results in a leak
of the both instances. According to other usages of the voter, its ptr
should be held by owner of the task queue. As a result, voter ptr and
setPaused method is moved to a shared owner.

Change-Id: I055bb82db2c1906eb3fd4c5c3329484f07f3b09d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1670870
Commit-Queue: Alexander Timin <altimin@chromium.org>
Reviewed-by: default avatarAlexander Timin <altimin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#672049}
parent ccc93519
...@@ -7,6 +7,7 @@ ...@@ -7,6 +7,7 @@
#include "base/memory/weak_ptr.h" #include "base/memory/weak_ptr.h"
#include "base/single_thread_task_runner.h" #include "base/single_thread_task_runner.h"
#include "base/task/sequence_manager/task_queue.h"
#include "third_party/blink/public/platform/task_type.h" #include "third_party/blink/public/platform/task_type.h"
#include "third_party/blink/renderer/platform/scheduler/public/frame_or_worker_scheduler.h" #include "third_party/blink/renderer/platform/scheduler/public/frame_or_worker_scheduler.h"
#include "third_party/blink/renderer/platform/scheduler/public/frame_scheduler.h" #include "third_party/blink/renderer/platform/scheduler/public/frame_scheduler.h"
...@@ -94,6 +95,12 @@ class PLATFORM_EXPORT WorkerScheduler : public FrameOrWorkerScheduler { ...@@ -94,6 +95,12 @@ class PLATFORM_EXPORT WorkerScheduler : public FrameOrWorkerScheduler {
scoped_refptr<NonMainThreadTaskQueue> pausable_task_queue_; scoped_refptr<NonMainThreadTaskQueue> pausable_task_queue_;
scoped_refptr<NonMainThreadTaskQueue> unpausable_task_queue_; scoped_refptr<NonMainThreadTaskQueue> unpausable_task_queue_;
using TaskQueueVoterMap = std::map<
scoped_refptr<NonMainThreadTaskQueue>,
std::unique_ptr<base::sequence_manager::TaskQueue::QueueEnabledVoter>>;
TaskQueueVoterMap task_runners_;
SchedulingLifecycleState lifecycle_state_ = SchedulingLifecycleState lifecycle_state_ =
SchedulingLifecycleState::kNotThrottled; SchedulingLifecycleState::kNotThrottled;
......
...@@ -38,11 +38,5 @@ void NonMainThreadTaskQueue::OnTaskCompleted( ...@@ -38,11 +38,5 @@ void NonMainThreadTaskQueue::OnTaskCompleted(
} }
} }
void NonMainThreadTaskQueue::SetPaused(bool paused) {
if (!task_queue_voter_)
task_queue_voter_ = CreateQueueEnabledVoter();
task_queue_voter_->SetVoteToEnable(!paused);
}
} // namespace scheduler } // namespace scheduler
} // namespace blink } // namespace blink
...@@ -34,11 +34,7 @@ class PLATFORM_EXPORT NonMainThreadTaskQueue ...@@ -34,11 +34,7 @@ class PLATFORM_EXPORT NonMainThreadTaskQueue
return TaskQueue::CreateTaskRunner(static_cast<int>(task_type)); return TaskQueue::CreateTaskRunner(static_cast<int>(task_type));
} }
void SetPaused(bool paused);
private: private:
std::unique_ptr<QueueEnabledVoter> task_queue_voter_;
// Not owned. // Not owned.
NonMainThreadSchedulerImpl* non_main_thread_scheduler_; NonMainThreadSchedulerImpl* non_main_thread_scheduler_;
}; };
......
...@@ -33,6 +33,13 @@ WorkerScheduler::WorkerScheduler(WorkerThreadScheduler* worker_thread_scheduler, ...@@ -33,6 +33,13 @@ WorkerScheduler::WorkerScheduler(WorkerThreadScheduler* worker_thread_scheduler,
worker_thread_scheduler->CreateTaskQueue("worker_unpausable_tq")), worker_thread_scheduler->CreateTaskQueue("worker_unpausable_tq")),
thread_scheduler_(worker_thread_scheduler), thread_scheduler_(worker_thread_scheduler),
weak_factory_(this) { weak_factory_(this) {
task_runners_.insert(
std::make_pair(throttleable_task_queue_,
throttleable_task_queue_->CreateQueueEnabledVoter()));
task_runners_.insert(std::make_pair(
pausable_task_queue_, pausable_task_queue_->CreateQueueEnabledVoter()));
task_runners_.insert(std::make_pair(unpausable_task_queue_, nullptr));
thread_scheduler_->RegisterWorkerScheduler(this); thread_scheduler_->RegisterWorkerScheduler(this);
SetUpThrottling(); SetUpThrottling();
...@@ -62,8 +69,11 @@ void WorkerScheduler::PauseImpl() { ...@@ -62,8 +69,11 @@ void WorkerScheduler::PauseImpl() {
thread_scheduler_->helper()->CheckOnValidThread(); thread_scheduler_->helper()->CheckOnValidThread();
paused_count_++; paused_count_++;
if (paused_count_ == 1) { if (paused_count_ == 1) {
throttleable_task_queue_->SetPaused(true); for (const auto& pair : task_runners_) {
pausable_task_queue_->SetPaused(true); if (pair.second) {
pair.second->SetVoteToEnable(false);
}
}
} }
} }
...@@ -71,8 +81,11 @@ void WorkerScheduler::ResumeImpl() { ...@@ -71,8 +81,11 @@ void WorkerScheduler::ResumeImpl() {
thread_scheduler_->helper()->CheckOnValidThread(); thread_scheduler_->helper()->CheckOnValidThread();
paused_count_--; paused_count_--;
if (paused_count_ == 0 && !is_disposed_) { if (paused_count_ == 0 && !is_disposed_) {
throttleable_task_queue_->SetPaused(false); for (const auto& pair : task_runners_) {
pausable_task_queue_->SetPaused(false); if (pair.second) {
pair.second->SetVoteToEnable(true);
}
}
} }
} }
...@@ -110,9 +123,11 @@ void WorkerScheduler::Dispose() { ...@@ -110,9 +123,11 @@ void WorkerScheduler::Dispose() {
thread_scheduler_->UnregisterWorkerScheduler(this); thread_scheduler_->UnregisterWorkerScheduler(this);
unpausable_task_queue_->ShutdownTaskQueue(); for (const auto& pair : task_runners_) {
pausable_task_queue_->ShutdownTaskQueue(); pair.first->ShutdownTaskQueue();
throttleable_task_queue_->ShutdownTaskQueue(); }
task_runners_.clear();
is_disposed_ = true; is_disposed_ = true;
} }
......
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