Commit 99fb6b82 authored by tzik's avatar tzik Committed by Commit Bot

Replace ref-counted TaskQueueManager::DeletionSentinel with a WeakPtr

DeletionSentinel may fail to detect TaskQueueManager deletion on a
nested message loop:
If there's a nested message loop that spins in task_annotator_.RunTask()
at line 531, ProcessTaskFromWorkQueue() is reentered. As both outer and
inner PTFWQ hold a ref to the DeletionSentinel instance,
protect->HasOneRef() at line 534 doesn't hit even when the
TaskQueueManager is gone.

This CL replaces it with WeakPtr that covers the reentered case.

Change-Id: Ia15947fcac399dc45994b0df7514a3872c6ba25c
Reviewed-on: https://chromium-review.googlesource.com/721219
Commit-Queue: Taiju Tsuiki <tzik@chromium.org>
Reviewed-by: default avatarAlex Clarke <alexclarke@chromium.org>
Reviewed-by: default avatarAlexander Timin <altimin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#509322}
parent ff18039b
...@@ -48,7 +48,6 @@ TaskQueueManager::TaskQueueManager( ...@@ -48,7 +48,6 @@ TaskQueueManager::TaskQueueManager(
task_count_(0), task_count_(0),
currently_executing_task_queue_(nullptr), currently_executing_task_queue_(nullptr),
observer_(nullptr), observer_(nullptr),
deletion_sentinel_(new DeletionSentinel()),
weak_factory_(this) { weak_factory_(this) {
DCHECK(delegate->RunsTasksInCurrentSequence()); DCHECK(delegate->RunsTasksInCurrentSequence());
TRACE_EVENT_OBJECT_CREATED_WITH_ID( TRACE_EVENT_OBJECT_CREATED_WITH_ID(
...@@ -474,7 +473,7 @@ TaskQueueManager::ProcessTaskResult TaskQueueManager::ProcessTaskFromWorkQueue( ...@@ -474,7 +473,7 @@ TaskQueueManager::ProcessTaskResult TaskQueueManager::ProcessTaskFromWorkQueue(
LazyNow time_before_task, LazyNow time_before_task,
base::TimeTicks* time_after_task) { base::TimeTicks* time_after_task) {
DCHECK(main_thread_checker_.CalledOnValidThread()); DCHECK(main_thread_checker_.CalledOnValidThread());
scoped_refptr<DeletionSentinel> protect(deletion_sentinel_); base::WeakPtr<TaskQueueManager> protect = GetWeakPtr();
internal::TaskQueueImpl::Task pending_task = internal::TaskQueueImpl::Task pending_task =
work_queue->TakeTaskFromWorkQueue(); work_queue->TakeTaskFromWorkQueue();
...@@ -531,7 +530,7 @@ TaskQueueManager::ProcessTaskResult TaskQueueManager::ProcessTaskFromWorkQueue( ...@@ -531,7 +530,7 @@ TaskQueueManager::ProcessTaskResult TaskQueueManager::ProcessTaskFromWorkQueue(
task_annotator_.RunTask("TaskQueueManager::PostTask", &pending_task); task_annotator_.RunTask("TaskQueueManager::PostTask", &pending_task);
// Detect if the TaskQueueManager just got deleted. If this happens we must // Detect if the TaskQueueManager just got deleted. If this happens we must
// not access any member variables after this point. // not access any member variables after this point.
if (protect->HasOneRef()) if (!protect)
return ProcessTaskResult::TASK_QUEUE_MANAGER_DELETED; return ProcessTaskResult::TASK_QUEUE_MANAGER_DELETED;
currently_executing_task_queue_ = prev_executing_task_queue; currently_executing_task_queue_ = prev_executing_task_queue;
......
...@@ -11,6 +11,7 @@ ...@@ -11,6 +11,7 @@
#include "base/cancelable_callback.h" #include "base/cancelable_callback.h"
#include "base/debug/task_annotator.h" #include "base/debug/task_annotator.h"
#include "base/macros.h" #include "base/macros.h"
#include "base/memory/scoped_refptr.h"
#include "base/memory/weak_ptr.h" #include "base/memory/weak_ptr.h"
#include "base/message_loop/message_loop.h" #include "base/message_loop/message_loop.h"
#include "base/pending_task.h" #include "base/pending_task.h"
...@@ -229,12 +230,6 @@ class PLATFORM_EXPORT TaskQueueManager ...@@ -229,12 +230,6 @@ class PLATFORM_EXPORT TaskQueueManager
TimeDomain* time_domain_; TimeDomain* time_domain_;
}; };
class DeletionSentinel : public base::RefCounted<DeletionSentinel> {
private:
friend class base::RefCounted<DeletionSentinel>;
~DeletionSentinel() {}
};
// TaskQueueSelector::Observer implementation: // TaskQueueSelector::Observer implementation:
void OnTaskQueueEnabled(internal::TaskQueueImpl* queue) override; void OnTaskQueueEnabled(internal::TaskQueueImpl* queue) override;
void OnTriedToSelectBlockedWorkQueue( void OnTriedToSelectBlockedWorkQueue(
...@@ -382,7 +377,6 @@ class PLATFORM_EXPORT TaskQueueManager ...@@ -382,7 +377,6 @@ class PLATFORM_EXPORT TaskQueueManager
internal::TaskQueueImpl* currently_executing_task_queue_; // NOT OWNED internal::TaskQueueImpl* currently_executing_task_queue_; // NOT OWNED
Observer* observer_; // NOT OWNED Observer* observer_; // NOT OWNED
scoped_refptr<DeletionSentinel> deletion_sentinel_;
base::WeakPtrFactory<TaskQueueManager> weak_factory_; base::WeakPtrFactory<TaskQueueManager> weak_factory_;
DISALLOW_COPY_AND_ASSIGN(TaskQueueManager); DISALLOW_COPY_AND_ASSIGN(TaskQueueManager);
......
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