diff options
author | Allan Sandfeld Jensen <allan.jensen@qt.io> | 2023-04-14 15:24:54 +0200 |
---|---|---|
committer | Allan Sandfeld Jensen <allan.jensen@qt.io> | 2023-05-26 11:29:55 +0000 |
commit | ab965b1c2c3e7e4cd62a4b45abfaf393f4fb4618 (patch) | |
tree | 906ba4a71162c3ebcae0b742dbe01076af2abb22 /chromium/base/task/thread_pool | |
parent | 813d9ae984a99e739b99cf694a9d5b24d0a6b7a7 (diff) |
BASELINE: Update Chromium to 112.0.5615.132
Change-Id: I59e23789618066826010171a36efbf8a68965ed0
Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/472282
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>
Diffstat (limited to 'chromium/base/task/thread_pool')
40 files changed, 374 insertions, 188 deletions
diff --git a/chromium/base/task/thread_pool/delayed_priority_queue_unittest.cc b/chromium/base/task/thread_pool/delayed_priority_queue_unittest.cc index 1543479bff9..4300988980c 100644 --- a/chromium/base/task/thread_pool/delayed_priority_queue_unittest.cc +++ b/chromium/base/task/thread_pool/delayed_priority_queue_unittest.cc @@ -7,7 +7,7 @@ #include <memory> #include <utility> -#include "base/callback_helpers.h" +#include "base/functional/callback_helpers.h" #include "base/memory/ptr_util.h" #include "base/memory/ref_counted.h" #include "base/task/task_traits.h" diff --git a/chromium/base/task/thread_pool/delayed_task_manager.cc b/chromium/base/task/thread_pool/delayed_task_manager.cc index fdc391dc42b..4b61aa62903 100644 --- a/chromium/base/task/thread_pool/delayed_task_manager.cc +++ b/chromium/base/task/thread_pool/delayed_task_manager.cc @@ -6,9 +6,9 @@ #include <algorithm> -#include "base/bind.h" #include "base/check.h" #include "base/feature_list.h" +#include "base/functional/bind.h" #include "base/task/common/checked_lock.h" #include "base/task/sequenced_task_runner.h" #include "base/task/task_features.h" diff --git a/chromium/base/task/thread_pool/delayed_task_manager.h b/chromium/base/task/thread_pool/delayed_task_manager.h index a04b4e3f7a7..b055a500f38 100644 --- a/chromium/base/task/thread_pool/delayed_task_manager.h +++ b/chromium/base/task/thread_pool/delayed_task_manager.h @@ -8,8 +8,8 @@ #include <functional> #include "base/base_export.h" -#include "base/callback.h" #include "base/containers/intrusive_heap.h" +#include "base/functional/callback.h" #include "base/memory/ptr_util.h" #include "base/memory/raw_ptr.h" #include "base/synchronization/atomic_flag.h" diff --git a/chromium/base/task/thread_pool/delayed_task_manager_unittest.cc b/chromium/base/task/thread_pool/delayed_task_manager_unittest.cc index c9c0dd50798..70f5fe85eb0 100644 --- a/chromium/base/task/thread_pool/delayed_task_manager_unittest.cc +++ b/chromium/base/task/thread_pool/delayed_task_manager_unittest.cc @@ -7,9 +7,9 @@ #include <memory> #include <utility> -#include "base/bind.h" -#include "base/callback_helpers.h" #include "base/cancelable_callback.h" +#include "base/functional/bind.h" +#include "base/functional/callback_helpers.h" #include "base/memory/ptr_util.h" #include "base/memory/ref_counted.h" #include "base/synchronization/waitable_event.h" diff --git a/chromium/base/task/thread_pool/job_task_source.cc b/chromium/base/task/thread_pool/job_task_source.cc index de92c80d194..529f53c61c9 100644 --- a/chromium/base/task/thread_pool/job_task_source.cc +++ b/chromium/base/task/thread_pool/job_task_source.cc @@ -7,10 +7,10 @@ #include <type_traits> #include <utility> -#include "base/bind.h" #include "base/bits.h" -#include "base/callback_helpers.h" #include "base/check_op.h" +#include "base/functional/bind.h" +#include "base/functional/callback_helpers.h" #include "base/memory/ptr_util.h" #include "base/notreached.h" #include "base/task/common/checked_lock.h" diff --git a/chromium/base/task/thread_pool/job_task_source.h b/chromium/base/task/thread_pool/job_task_source.h index 5c431272285..71d902a239e 100644 --- a/chromium/base/task/thread_pool/job_task_source.h +++ b/chromium/base/task/thread_pool/job_task_source.h @@ -13,7 +13,7 @@ #include <utility> #include "base/base_export.h" -#include "base/callback.h" +#include "base/functional/callback.h" #include "base/memory/raw_ptr.h" #include "base/synchronization/condition_variable.h" #include "base/task/common/checked_lock.h" diff --git a/chromium/base/task/thread_pool/job_task_source_unittest.cc b/chromium/base/task/thread_pool/job_task_source_unittest.cc index 4ff3f69ada5..82a23e416c8 100644 --- a/chromium/base/task/thread_pool/job_task_source_unittest.cc +++ b/chromium/base/task/thread_pool/job_task_source_unittest.cc @@ -6,7 +6,7 @@ #include <utility> -#include "base/callback_helpers.h" +#include "base/functional/callback_helpers.h" #include "base/memory/ptr_util.h" #include "base/task/thread_pool/pooled_task_runner_delegate.h" #include "base/task/thread_pool/test_utils.h" diff --git a/chromium/base/task/thread_pool/pooled_parallel_task_runner.h b/chromium/base/task/thread_pool/pooled_parallel_task_runner.h index d2a37457ce5..cd364e8fb48 100644 --- a/chromium/base/task/thread_pool/pooled_parallel_task_runner.h +++ b/chromium/base/task/thread_pool/pooled_parallel_task_runner.h @@ -6,7 +6,7 @@ #define BASE_TASK_THREAD_POOL_POOLED_PARALLEL_TASK_RUNNER_H_ #include "base/base_export.h" -#include "base/callback_forward.h" +#include "base/functional/callback_forward.h" #include "base/location.h" #include "base/memory/raw_ptr.h" #include "base/task/task_runner.h" @@ -37,9 +37,7 @@ class BASE_EXPORT PooledParallelTaskRunner : public TaskRunner { ~PooledParallelTaskRunner() override; const TaskTraits traits_; - // TODO(crbug.com/1298696): Breaks storage_unittests. - const raw_ptr<PooledTaskRunnerDelegate, DegradeToNoOpWhenMTE> - pooled_task_runner_delegate_; + const raw_ptr<PooledTaskRunnerDelegate> pooled_task_runner_delegate_; }; } // namespace internal diff --git a/chromium/base/task/thread_pool/pooled_sequenced_task_runner.h b/chromium/base/task/thread_pool/pooled_sequenced_task_runner.h index 5d5a3fe52c6..cecd937cde8 100644 --- a/chromium/base/task/thread_pool/pooled_sequenced_task_runner.h +++ b/chromium/base/task/thread_pool/pooled_sequenced_task_runner.h @@ -6,7 +6,7 @@ #define BASE_TASK_THREAD_POOL_POOLED_SEQUENCED_TASK_RUNNER_H_ #include "base/base_export.h" -#include "base/callback_forward.h" +#include "base/functional/callback_forward.h" #include "base/location.h" #include "base/memory/raw_ptr.h" #include "base/task/task_traits.h" @@ -52,9 +52,7 @@ class BASE_EXPORT PooledSequencedTaskRunner private: ~PooledSequencedTaskRunner() override; - // TODO(crbug.com/1298696): Breaks base_unittests. - const raw_ptr<PooledTaskRunnerDelegate, DegradeToNoOpWhenMTE> - pooled_task_runner_delegate_; + const raw_ptr<PooledTaskRunnerDelegate> pooled_task_runner_delegate_; // Sequence for all Tasks posted through this TaskRunner. const scoped_refptr<Sequence> sequence_; diff --git a/chromium/base/task/thread_pool/pooled_single_thread_task_runner_manager.cc b/chromium/base/task/thread_pool/pooled_single_thread_task_runner_manager.cc index 23d88f32ba8..17843567419 100644 --- a/chromium/base/task/thread_pool/pooled_single_thread_task_runner_manager.cc +++ b/chromium/base/task/thread_pool/pooled_single_thread_task_runner_manager.cc @@ -8,8 +8,9 @@ #include <string> #include <utility> -#include "base/bind.h" -#include "base/callback.h" +#include "base/debug/leak_annotations.h" +#include "base/functional/bind.h" +#include "base/functional/callback.h" #include "base/memory/ptr_util.h" #include "base/memory/raw_ptr.h" #include "base/ranges/algorithm.h" @@ -227,6 +228,11 @@ class WorkerThreadDelegate : public WorkerThread::Delegate { TransactionWithRegisteredTaskSource transaction_with_task_source) { CheckedAutoLock auto_lock(lock_); auto sort_key = transaction_with_task_source.task_source->GetSortKey(); + // When moving |task_source| into |priority_queue_|, it may be destroyed + // on another thread as soon as |lock_| is released, since we're no longer + // holding a reference to it. To prevent UAF, release |transaction| before + // moving |task_source|. Ref. crbug.com/1412008 + transaction_with_task_source.transaction.Release(); priority_queue_.Push(std::move(transaction_with_task_source.task_source), sort_key); if (!worker_awake_ && CanRunNextTaskSource()) { @@ -379,6 +385,12 @@ class WorkerThreadCOMDelegate : public WorkerThreadDelegate { return nullptr; transaction.PushImmediateTask(std::move(pump_message_task)); return registered_task_source; + } else { + // `pump_message_task`'s destructor may run sequence-affine code, so it + // must be leaked when `WillPostTask` returns false. + auto leak = std::make_unique<Task>(std::move(pump_message_task)); + ANNOTATE_LEAKING_OBJECT_PTR(leak.get()); + leak.release(); } } return nullptr; @@ -486,6 +498,11 @@ class PooledSingleThreadTaskRunnerManager::PooledSingleThreadTaskRunner bool PostTask(Task task) { if (!outer_->task_tracker_->WillPostTask(&task, sequence_->shutdown_behavior())) { + // `task`'s destructor may run sequence-affine code, so it must be leaked + // when `WillPostTask` returns false. + auto leak = std::make_unique<Task>(std::move(task)); + ANNOTATE_LEAKING_OBJECT_PTR(leak.get()); + leak.release(); return false; } @@ -506,7 +523,11 @@ class PooledSingleThreadTaskRunnerManager::PooledSingleThreadTaskRunner return static_cast<WorkerThreadDelegate*>(worker_->delegate()); } - const raw_ptr<PooledSingleThreadTaskRunnerManager> outer_; + // Dangling but safe since use is controlled by `g_manager_is_alive`. + const raw_ptr<PooledSingleThreadTaskRunnerManager, + DisableDanglingPtrDetection> + outer_; + const raw_ptr<WorkerThread, DanglingUntriaged> worker_; const SingleThreadTaskRunnerThreadMode thread_mode_; const scoped_refptr<Sequence> sequence_; diff --git a/chromium/base/task/thread_pool/pooled_single_thread_task_runner_manager_unittest.cc b/chromium/base/task/thread_pool/pooled_single_thread_task_runner_manager_unittest.cc index 136ea27dae3..bd98250e339 100644 --- a/chromium/base/task/thread_pool/pooled_single_thread_task_runner_manager_unittest.cc +++ b/chromium/base/task/thread_pool/pooled_single_thread_task_runner_manager_unittest.cc @@ -4,8 +4,8 @@ #include "base/task/thread_pool/pooled_single_thread_task_runner_manager.h" -#include "base/bind.h" -#include "base/callback_helpers.h" +#include "base/functional/bind.h" +#include "base/functional/callback_helpers.h" #include "base/memory/ptr_util.h" #include "base/memory/raw_ptr.h" #include "base/synchronization/atomic_flag.h" diff --git a/chromium/base/task/thread_pool/priority_queue_unittest.cc b/chromium/base/task/thread_pool/priority_queue_unittest.cc index 46d461854b1..a42ee1e8de2 100644 --- a/chromium/base/task/thread_pool/priority_queue_unittest.cc +++ b/chromium/base/task/thread_pool/priority_queue_unittest.cc @@ -7,7 +7,7 @@ #include <memory> #include <utility> -#include "base/callback_helpers.h" +#include "base/functional/callback_helpers.h" #include "base/memory/ptr_util.h" #include "base/memory/ref_counted.h" #include "base/task/task_traits.h" diff --git a/chromium/base/task/thread_pool/sequence.cc b/chromium/base/task/thread_pool/sequence.cc index 0efc4f534e4..13034037f64 100644 --- a/chromium/base/task/thread_pool/sequence.cc +++ b/chromium/base/task/thread_pool/sequence.cc @@ -6,10 +6,10 @@ #include <utility> -#include "base/bind.h" #include "base/check.h" #include "base/critical_closure.h" #include "base/feature_list.h" +#include "base/functional/bind.h" #include "base/memory/ptr_util.h" #include "base/task/task_features.h" #include "base/time/time.h" @@ -37,6 +37,26 @@ class SCOPED_LOCKABLE AnnotateLockAcquired { const raw_ref<const CheckedLock> acquired_lock_; }; +void MaybeMakeCriticalClosure(TaskShutdownBehavior shutdown_behavior, + Task& task) { + switch (shutdown_behavior) { + case TaskShutdownBehavior::CONTINUE_ON_SHUTDOWN: + // Nothing to do. + break; + case TaskShutdownBehavior::SKIP_ON_SHUTDOWN: + // MakeCriticalClosure() is arguably useful for SKIP_ON_SHUTDOWN, possibly + // in combination with is_immediate=false. However, SKIP_ON_SHUTDOWN is + // the default and hence the theoretical benefits don't warrant the + // performance implications. + break; + case TaskShutdownBehavior::BLOCK_SHUTDOWN: + task.task = + MakeCriticalClosure(task.posted_from, std::move(task.task), + /*is_immediate=*/task.delayed_run_time.is_null()); + break; + } +} + } // namespace Sequence::Transaction::Transaction(Sequence* sequence) @@ -46,29 +66,19 @@ Sequence::Transaction::Transaction(Sequence::Transaction&& other) = default; Sequence::Transaction::~Transaction() = default; -bool Sequence::DelayedSortKeyWillChange(const Task& delayed_task) const { - AnnotateLockAcquired annotate(lock_); - // If sequence has already been picked up by a worker or moved, no need to - // proceed further here. - if (is_immediate_.load(std::memory_order_relaxed)) - return false; - - if (IsEmpty()) - return true; - - return delayed_task.latest_delayed_run_time() < - delayed_queue_.top().latest_delayed_run_time(); -} - bool Sequence::Transaction::WillPushImmediateTask() { + // In a Transaction. AnnotateLockAcquired annotate(sequence()->lock_); + bool was_immediate = sequence()->is_immediate_.exchange(true, std::memory_order_relaxed); return !was_immediate; } void Sequence::Transaction::PushImmediateTask(Task task) { + // In a Transaction. AnnotateLockAcquired annotate(sequence()->lock_); + // Use CHECK instead of DCHECK to crash earlier. See http://crbug.com/711167 // for details. CHECK(task.task); @@ -77,12 +87,8 @@ void Sequence::Transaction::PushImmediateTask(Task task) { bool was_unretained = sequence()->IsEmpty() && !sequence()->has_worker_; bool queue_was_empty = sequence()->queue_.empty(); - task.task = sequence()->traits_.shutdown_behavior() == - TaskShutdownBehavior::BLOCK_SHUTDOWN - ? MakeCriticalClosure( - task.posted_from, std::move(task.task), - /*is_immediate=*/task.delayed_run_time.is_null()) - : std::move(task.task); + + MaybeMakeCriticalClosure(sequence()->traits_.shutdown_behavior(), task); sequence()->queue_.push(std::move(task)); @@ -96,7 +102,9 @@ void Sequence::Transaction::PushImmediateTask(Task task) { } bool Sequence::Transaction::PushDelayedTask(Task task) { + // In a Transaction. AnnotateLockAcquired annotate(sequence()->lock_); + // Use CHECK instead of DCHECK to crash earlier. See http://crbug.com/711167 // for details. CHECK(task.task); @@ -105,11 +113,8 @@ bool Sequence::Transaction::PushDelayedTask(Task task) { bool top_will_change = sequence()->DelayedSortKeyWillChange(task); bool was_empty = sequence()->IsEmpty(); - task.task = - sequence()->traits_.shutdown_behavior() == - TaskShutdownBehavior::BLOCK_SHUTDOWN - ? MakeCriticalClosure(task.posted_from, std::move(task.task), false) - : std::move(task.task); + + MaybeMakeCriticalClosure(sequence()->traits_.shutdown_behavior(), task); sequence()->delayed_queue_.insert(std::move(task)); @@ -257,6 +262,21 @@ bool Sequence::WillReEnqueue(TimeTicks now, return has_ready_tasks; } +bool Sequence::DelayedSortKeyWillChange(const Task& delayed_task) const { + // If sequence has already been picked up by a worker or moved, no need to + // proceed further here. + if (is_immediate_.load(std::memory_order_relaxed)) { + return false; + } + + if (IsEmpty()) { + return true; + } + + return delayed_task.latest_delayed_run_time() < + delayed_queue_.top().latest_delayed_run_time(); +} + bool Sequence::HasReadyTasks(TimeTicks now) const { return now >= TS_UNCHECKED_READ(earliest_ready_time_) .load(std::memory_order_relaxed); diff --git a/chromium/base/task/thread_pool/sequence.h b/chromium/base/task/thread_pool/sequence.h index decdbdfb1d2..c3a10f455d2 100644 --- a/chromium/base/task/thread_pool/sequence.h +++ b/chromium/base/task/thread_pool/sequence.h @@ -132,7 +132,8 @@ class BASE_EXPORT Sequence : public TaskSource { // Returns true if the delayed task to be posted will cause the delayed sort // key to change. - bool DelayedSortKeyWillChange(const Task& delayed_task) const; + bool DelayedSortKeyWillChange(const Task& delayed_task) const + EXCLUSIVE_LOCKS_REQUIRED(lock_); // Selects the earliest task to run, either from immediate or // delayed queue and return it. diff --git a/chromium/base/task/thread_pool/sequence_unittest.cc b/chromium/base/task/thread_pool/sequence_unittest.cc index 2588415df03..84a7bf1421a 100644 --- a/chromium/base/task/thread_pool/sequence_unittest.cc +++ b/chromium/base/task/thread_pool/sequence_unittest.cc @@ -6,8 +6,8 @@ #include <utility> -#include "base/bind.h" -#include "base/callback_helpers.h" +#include "base/functional/bind.h" +#include "base/functional/callback_helpers.h" #include "base/memory/ptr_util.h" #include "base/test/gtest_util.h" #include "base/time/time.h" diff --git a/chromium/base/task/thread_pool/service_thread_unittest.cc b/chromium/base/task/thread_pool/service_thread_unittest.cc index 4e16b0a281d..9a7e7965eb9 100644 --- a/chromium/base/task/thread_pool/service_thread_unittest.cc +++ b/chromium/base/task/thread_pool/service_thread_unittest.cc @@ -6,8 +6,8 @@ #include <string> -#include "base/bind.h" #include "base/debug/stack_trace.h" +#include "base/functional/bind.h" #include "build/build_config.h" #include "testing/gtest/include/gtest/gtest.h" diff --git a/chromium/base/task/thread_pool/task.h b/chromium/base/task/thread_pool/task.h index 4e89eea2bf8..2af88aa1d14 100644 --- a/chromium/base/task/thread_pool/task.h +++ b/chromium/base/task/thread_pool/task.h @@ -6,8 +6,8 @@ #define BASE_TASK_THREAD_POOL_TASK_H_ #include "base/base_export.h" -#include "base/callback.h" #include "base/containers/intrusive_heap.h" +#include "base/functional/callback.h" #include "base/location.h" #include "base/pending_task.h" #include "base/task/sequenced_task_runner.h" diff --git a/chromium/base/task/thread_pool/task_source.cc b/chromium/base/task/thread_pool/task_source.cc index c8966f435d0..84e49658394 100644 --- a/chromium/base/task/thread_pool/task_source.cc +++ b/chromium/base/task/thread_pool/task_source.cc @@ -27,8 +27,7 @@ TaskSource::Transaction::Transaction(TaskSource::Transaction&& other) TaskSource::Transaction::~Transaction() { if (task_source_) { - task_source_->lock_.AssertAcquired(); - task_source_->lock_.Release(); + Release(); } } @@ -38,6 +37,13 @@ void TaskSource::Transaction::UpdatePriority(TaskPriority priority) { std::memory_order_relaxed); } +void TaskSource::Transaction::Release() NO_THREAD_SAFETY_ANALYSIS { + DCHECK(task_source_); + task_source_->lock_.AssertAcquired(); + task_source_->lock_.Release(); + task_source_ = nullptr; +} + void TaskSource::SetImmediateHeapHandle(const HeapHandle& handle) { immediate_pq_heap_handle_ = handle; } @@ -66,7 +72,13 @@ TaskSource::TaskSource(const TaskTraits& traits, execution_mode_ == TaskSourceExecutionMode::kJob); } -TaskSource::~TaskSource() = default; +TaskSource::~TaskSource() { + // If this fails, a Transaction was likely held while releasing a reference to + // its associated task source, which lead to its destruction. Owners of + // Transaction must ensure to hold onto a reference of the associated task + // source at least until the Transaction is released to prevent UAF. + lock_.AssertNotHeld(); +} TaskSource::Transaction TaskSource::BeginTransaction() { return Transaction(this); diff --git a/chromium/base/task/thread_pool/task_source.h b/chromium/base/task/thread_pool/task_source.h index f9c6dff2148..371cfb28f42 100644 --- a/chromium/base/task/thread_pool/task_source.h +++ b/chromium/base/task/thread_pool/task_source.h @@ -11,6 +11,7 @@ #include "base/containers/intrusive_heap.h" #include "base/dcheck_is_on.h" #include "base/memory/raw_ptr.h" +#include "base/memory/raw_ptr_exclusion.h" #include "base/memory/ref_counted.h" #include "base/sequence_token.h" #include "base/task/common/checked_lock.h" @@ -103,7 +104,8 @@ class BASE_EXPORT TaskSource : public RefCountedThreadSafe<TaskSource> { // A Transaction can perform multiple operations atomically on a // TaskSource. While a Transaction is alive, it is guaranteed that nothing // else will access the TaskSource; the TaskSource's lock is held for the - // lifetime of the Transaction. + // lifetime of the Transaction. No Transaction must be held when ~TaskSource() + // is called. class BASE_EXPORT Transaction { public: Transaction(Transaction&& other); @@ -121,13 +123,17 @@ class BASE_EXPORT TaskSource : public RefCountedThreadSafe<TaskSource> { TaskSource* task_source() const { return task_source_; } + void Release(); + protected: explicit Transaction(TaskSource* task_source); private: friend class TaskSource; - TaskSource* task_source_; + // This field is not a raw_ptr<> because it was filtered by the rewriter + // for: #union + RAW_PTR_EXCLUSION TaskSource* task_source_; }; // |traits| is metadata that applies to all Tasks in the TaskSource. @@ -336,7 +342,9 @@ class BASE_EXPORT RegisteredTaskSource { #endif // DCHECK_IS_ON() scoped_refptr<TaskSource> task_source_; - TaskTracker* task_tracker_ = nullptr; + // This field is not a raw_ptr<> because it was filtered by the rewriter for: + // #union + RAW_PTR_EXCLUSION TaskTracker* task_tracker_ = nullptr; }; // A pair of Transaction and RegisteredTaskSource. Useful to carry a diff --git a/chromium/base/task/thread_pool/task_tracker.cc b/chromium/base/task/thread_pool/task_tracker.cc index 98f316c71d3..51f2e15b1b5 100644 --- a/chromium/base/task/thread_pool/task_tracker.cc +++ b/chromium/base/task/thread_pool/task_tracker.cc @@ -9,14 +9,15 @@ #include <utility> #include "base/base_switches.h" -#include "base/callback.h" #include "base/command_line.h" #include "base/compiler_specific.h" #include "base/debug/alias.h" +#include "base/functional/callback.h" #include "base/json/json_writer.h" #include "base/logging.h" #include "base/memory/ptr_util.h" #include "base/metrics/histogram_macros.h" +#include "base/no_destructor.h" #include "base/notreached.h" #include "base/sequence_token.h" #include "base/strings/string_util.h" @@ -120,6 +121,12 @@ auto EmitThreadPoolTraceEventMetadata(perfetto::EventContext& ctx, #endif // BUILDFLAG(ENABLE_BASE_TRACING) } +base::ThreadLocalBoolean& GetFizzleBlockShutdownTaskFlag() { + static base::NoDestructor<base::ThreadLocalBoolean> + fizzle_block_shutdown_tasks; + return *fizzle_block_shutdown_tasks; +} + } // namespace // Atomic internal state used by TaskTracker to track items that are blocking @@ -311,12 +318,14 @@ bool TaskTracker::WillPostTask(Task* task, // A non BLOCK_SHUTDOWN task is allowed to be posted iff shutdown hasn't // started and the task is not delayed. if (shutdown_behavior != TaskShutdownBehavior::BLOCK_SHUTDOWN || - !task->delayed_run_time.is_null()) { + !task->delayed_run_time.is_null() || + GetFizzleBlockShutdownTaskFlag().Get()) { return false; } - // A BLOCK_SHUTDOWN task posted after shutdown has completed is an - // ordering bug. This aims to catch those early. + // A BLOCK_SHUTDOWN task posted after shutdown has completed without setting + // `fizzle_block_shutdown_tasks` is an ordering bug. This aims to catch + // those early. CheckedAutoLock auto_lock(shutdown_lock_); DCHECK(shutdown_event_); DCHECK(!shutdown_event_->IsSignaled()) @@ -414,6 +423,14 @@ bool TaskTracker::IsShutdownComplete() const { return shutdown_event_ && shutdown_event_->IsSignaled(); } +void TaskTracker::BeginFizzlingBlockShutdownTasks() { + GetFizzleBlockShutdownTaskFlag().Set(true); +} + +void TaskTracker::EndFizzlingBlockShutdownTasks() { + GetFizzleBlockShutdownTaskFlag().Set(false); +} + void TaskTracker::RunTask(Task task, TaskSource* task_source, const TaskTraits& traits) { @@ -452,21 +469,21 @@ void TaskTracker::RunTask(Task task, // Set up TaskRunner CurrentDefaultHandle as expected for the scope of the // task. absl::optional<SequencedTaskRunner::CurrentDefaultHandle> - sequenced_task_runner_handle; + sequenced_task_runner_current_default_handle; absl::optional<SingleThreadTaskRunner::CurrentDefaultHandle> - single_thread_task_runner_handle; + single_thread_task_runner_current_default_handle; switch (task_source->execution_mode()) { case TaskSourceExecutionMode::kJob: case TaskSourceExecutionMode::kParallel: break; case TaskSourceExecutionMode::kSequenced: DCHECK(task_source->task_runner()); - sequenced_task_runner_handle.emplace( + sequenced_task_runner_current_default_handle.emplace( static_cast<SequencedTaskRunner*>(task_source->task_runner())); break; case TaskSourceExecutionMode::kSingleThread: DCHECK(task_source->task_runner()); - single_thread_task_runner_handle.emplace( + single_thread_task_runner_current_default_handle.emplace( static_cast<SingleThreadTaskRunner*>(task_source->task_runner())); break; } diff --git a/chromium/base/task/thread_pool/task_tracker.h b/chromium/base/task/thread_pool/task_tracker.h index ce86c56c522..2a25c36f7ea 100644 --- a/chromium/base/task/thread_pool/task_tracker.h +++ b/chromium/base/task/thread_pool/task_tracker.h @@ -14,8 +14,8 @@ #include "base/atomicops.h" #include "base/base_export.h" -#include "base/callback_forward.h" #include "base/containers/circular_deque.h" +#include "base/functional/callback_forward.h" #include "base/sequence_checker.h" #include "base/strings/string_piece.h" #include "base/synchronization/waitable_event.h" @@ -26,6 +26,7 @@ #include "base/task/thread_pool/task_source.h" #include "base/task/thread_pool/tracked_ref.h" #include "base/thread_annotations.h" +#include "base/threading/thread_local.h" namespace base { @@ -95,6 +96,9 @@ class BASE_EXPORT TaskTracker { // DelayedTaskManager (if delayed). Returns true if this operation is allowed // (the operation should be performed if-and-only-if it is). This method may // also modify metadata on |task| if desired. + // If this returns false, `task` must be leaked by the caller if deleting it + // on the current sequence may invoke sequence-affine code that belongs to + // another sequence. bool WillPostTask(Task* task, TaskShutdownBehavior shutdown_behavior); // Informs this TaskTracker that |task| that is about to be pushed to a task @@ -133,6 +137,9 @@ class BASE_EXPORT TaskTracker { return tracked_ref_factory_.GetTrackedRef(); } + void BeginFizzlingBlockShutdownTasks(); + void EndFizzlingBlockShutdownTasks(); + // Returns true if there are task sources that haven't completed their // execution (still queued or in progress). If it returns false: the side- // effects of all completed tasks are guaranteed to be visible to the caller. diff --git a/chromium/base/task/thread_pool/task_tracker_unittest.cc b/chromium/base/task/thread_pool/task_tracker_unittest.cc index 55b89640228..b89082a032c 100644 --- a/chromium/base/task/thread_pool/task_tracker_unittest.cc +++ b/chromium/base/task/thread_pool/task_tracker_unittest.cc @@ -11,10 +11,10 @@ #include <vector> #include "base/barrier_closure.h" -#include "base/bind.h" -#include "base/callback.h" -#include "base/callback_helpers.h" #include "base/check_op.h" +#include "base/functional/bind.h" +#include "base/functional/callback.h" +#include "base/functional/callback_helpers.h" #include "base/memory/ptr_util.h" #include "base/memory/raw_ptr.h" #include "base/memory/ref_counted.h" @@ -472,6 +472,15 @@ TEST_P(ThreadPoolTaskTrackerTest, WillPostAfterShutdown) { // |task_tracker_| shouldn't allow a task to be posted after shutdown. if (GetParam() == TaskShutdownBehavior::BLOCK_SHUTDOWN) { + // When the task tracker is allowed to fizzle block shutdown tasks, + // WillPostTask will return false and leak the task. + tracker_.BeginFizzlingBlockShutdownTasks(); + EXPECT_FALSE(tracker_.WillPostTask(&task, GetParam())); + tracker_.EndFizzlingBlockShutdownTasks(); + + // If a BLOCK_SHUTDOWN task is posted after shutdown without explicitly + // allowing BLOCK_SHUTDOWN task fizzling, WillPostTask DCHECKs to find + // ordering bugs. EXPECT_DCHECK_DEATH(tracker_.WillPostTask(&task, GetParam())); } else { EXPECT_FALSE(tracker_.WillPostTask(&task, GetParam())); @@ -527,7 +536,7 @@ TEST_P(ThreadPoolTaskTrackerTest, IOAllowed) { RunAndPopNextTask(std::move(sequence_without_may_block)); } -static void RunTaskRunnerHandleVerificationTask( +static void RunTaskRunnerCurrentDefaultHandleVerificationTask( TaskTracker* tracker, Task verify_task, TaskTraits traits, @@ -536,8 +545,8 @@ static void RunTaskRunnerHandleVerificationTask( // Pretend |verify_task| is posted to respect TaskTracker's contract. EXPECT_TRUE(tracker->WillPostTask(&verify_task, traits.shutdown_behavior())); - // Confirm that the test conditions are right (no TaskRunnerHandles set - // already). + // Confirm that the test conditions are right (no + // task runner CurrentDefaultHandles set already). EXPECT_FALSE(SingleThreadTaskRunner::HasCurrentDefault()); EXPECT_FALSE(SequencedTaskRunner::HasCurrentDefault()); @@ -546,12 +555,12 @@ static void RunTaskRunnerHandleVerificationTask( test::CreateSequenceWithTask(std::move(verify_task), traits, std::move(task_runner), execution_mode)); - // TaskRunnerHandle state is reset outside of task's scope. + // task runner CurrentDefaultHandle state is reset outside of task's scope. EXPECT_FALSE(SingleThreadTaskRunner::HasCurrentDefault()); EXPECT_FALSE(SequencedTaskRunner::HasCurrentDefault()); } -static void VerifyNoTaskRunnerHandle() { +static void VerifyNoTaskRunnerCurrentDefaultHandle() { EXPECT_FALSE(SingleThreadTaskRunner::HasCurrentDefault()); EXPECT_FALSE(SequencedTaskRunner::HasCurrentDefault()); } @@ -559,58 +568,63 @@ static void VerifyNoTaskRunnerHandle() { TEST_P(ThreadPoolTaskTrackerTest, TaskRunnerHandleIsNotSetOnParallel) { // Create a task that will verify that TaskRunnerHandles are not set in its // scope per no TaskRunner ref being set to it. - Task verify_task(FROM_HERE, BindOnce(&VerifyNoTaskRunnerHandle), + Task verify_task(FROM_HERE, BindOnce(&VerifyNoTaskRunnerCurrentDefaultHandle), TimeTicks::Now(), TimeDelta()); - RunTaskRunnerHandleVerificationTask(&tracker_, std::move(verify_task), - TaskTraits(GetParam()), nullptr, - TaskSourceExecutionMode::kParallel); + RunTaskRunnerCurrentDefaultHandleVerificationTask( + &tracker_, std::move(verify_task), TaskTraits(GetParam()), nullptr, + TaskSourceExecutionMode::kParallel); } -static void VerifySequencedTaskRunnerHandle( +static void VerifySequencedTaskRunnerCurrentDefaultHandle( const SequencedTaskRunner* expected_task_runner) { EXPECT_FALSE(SingleThreadTaskRunner::HasCurrentDefault()); EXPECT_TRUE(SequencedTaskRunner::HasCurrentDefault()); EXPECT_EQ(expected_task_runner, SequencedTaskRunner::GetCurrentDefault()); } -TEST_P(ThreadPoolTaskTrackerTest, SequencedTaskRunnerHandleIsSetOnSequenced) { +TEST_P(ThreadPoolTaskTrackerTest, + SequencedTaskRunnerHasCurrentDefaultOnSequenced) { scoped_refptr<SequencedTaskRunner> test_task_runner(new TestSimpleTaskRunner); - // Create a task that will verify that SequencedTaskRunnerHandle is properly - // set to |test_task_runner| in its scope per |sequenced_task_runner_ref| - // being set to it. + // Create a task that will verify that + // SequencedTaskRunner::CurrentDefaultHandle is properly set to + // |test_task_runner| in its scope per |sequenced_task_runner_ref| being set + // to it. Task verify_task(FROM_HERE, - BindOnce(&VerifySequencedTaskRunnerHandle, + BindOnce(&VerifySequencedTaskRunnerCurrentDefaultHandle, Unretained(test_task_runner.get())), TimeTicks::Now(), TimeDelta()); - RunTaskRunnerHandleVerificationTask( + RunTaskRunnerCurrentDefaultHandleVerificationTask( &tracker_, std::move(verify_task), TaskTraits(GetParam()), std::move(test_task_runner), TaskSourceExecutionMode::kSequenced); } -static void VerifyThreadTaskRunnerHandle( +static void VerifySingleThreadTaskRunnerCurrentDefaultHandle( const SingleThreadTaskRunner* expected_task_runner) { EXPECT_TRUE(SingleThreadTaskRunner::HasCurrentDefault()); - // SequencedTaskRunnerHandle inherits ThreadTaskRunnerHandle for thread. + // SequencedTaskRunner::CurrentDefaultHandle inherits + // SingleThreadTaskRunner::CurrentDefaultHandle for thread. EXPECT_TRUE(SequencedTaskRunner::HasCurrentDefault()); EXPECT_EQ(expected_task_runner, SingleThreadTaskRunner::GetCurrentDefault()); } -TEST_P(ThreadPoolTaskTrackerTest, ThreadTaskRunnerHandleIsSetOnSingleThreaded) { +TEST_P(ThreadPoolTaskTrackerTest, + SingleThreadTaskRunnerCurrentDefaultHandleIsSetOnSingleThreaded) { scoped_refptr<SingleThreadTaskRunner> test_task_runner( new TestSimpleTaskRunner); - // Create a task that will verify that ThreadTaskRunnerHandle is properly set - // to |test_task_runner| in its scope per |single_thread_task_runner_ref| - // being set on it. + // Create a task that will verify that + // SingleThreadTaskRunner::CurrentDefaultHandle is properly set to + // |test_task_runner| in its scope per |single_thread_task_runner_ref| being + // set on it. Task verify_task(FROM_HERE, - BindOnce(&VerifyThreadTaskRunnerHandle, + BindOnce(&VerifySingleThreadTaskRunnerCurrentDefaultHandle, Unretained(test_task_runner.get())), TimeTicks::Now(), TimeDelta()); - RunTaskRunnerHandleVerificationTask( + RunTaskRunnerCurrentDefaultHandleVerificationTask( &tracker_, std::move(verify_task), TaskTraits(GetParam()), std::move(test_task_runner), TaskSourceExecutionMode::kSingleThread); } diff --git a/chromium/base/task/thread_pool/test_task_factory.cc b/chromium/base/task/thread_pool/test_task_factory.cc index b1c17f9f986..a9d9fa99edf 100644 --- a/chromium/base/task/thread_pool/test_task_factory.cc +++ b/chromium/base/task/thread_pool/test_task_factory.cc @@ -4,10 +4,10 @@ #include "base/task/thread_pool/test_task_factory.h" -#include "base/bind.h" -#include "base/callback.h" -#include "base/callback_helpers.h" #include "base/check_op.h" +#include "base/functional/bind.h" +#include "base/functional/callback.h" +#include "base/functional/callback_helpers.h" #include "base/location.h" #include "base/synchronization/waitable_event.h" #include "base/task/sequenced_task_runner.h" @@ -59,7 +59,8 @@ void TestTaskFactory::RunTaskCallback(size_t task_index, ->RunsTasksInCurrentSequence()); } - // Verify TaskRunnerHandles are set as expected in the task's scope. + // Verify task runner CurrentDefaultHandles are set as expected in the task's + // scope. switch (execution_mode_) { case TaskSourceExecutionMode::kJob: case TaskSourceExecutionMode::kParallel: @@ -72,8 +73,9 @@ void TestTaskFactory::RunTaskCallback(size_t task_index, EXPECT_EQ(task_runner_, SequencedTaskRunner::GetCurrentDefault()); break; case TaskSourceExecutionMode::kSingleThread: - // SequencedTaskRunnerHandle inherits from ThreadTaskRunnerHandle so - // both are expected to be "set" in the kSingleThread case. + // SequencedTaskRunner::CurrentDefaultHandle inherits from + // SingleThreadTaskRunner::CurrentDefaultHandle so both are expected to be + // "set" in the kSingleThread case. EXPECT_TRUE(SingleThreadTaskRunner::HasCurrentDefault()); EXPECT_TRUE(SequencedTaskRunner::HasCurrentDefault()); EXPECT_EQ(task_runner_, SingleThreadTaskRunner::GetCurrentDefault()); diff --git a/chromium/base/task/thread_pool/test_task_factory.h b/chromium/base/task/thread_pool/test_task_factory.h index 4a894d9b465..9805eb3c038 100644 --- a/chromium/base/task/thread_pool/test_task_factory.h +++ b/chromium/base/task/thread_pool/test_task_factory.h @@ -9,7 +9,7 @@ #include <unordered_set> -#include "base/callback_forward.h" +#include "base/functional/callback_forward.h" #include "base/memory/ref_counted.h" #include "base/synchronization/condition_variable.h" #include "base/synchronization/lock.h" @@ -27,8 +27,8 @@ namespace test { // - The RunsTasksInCurrentSequence() method of the SequencedTaskRunner // (kSequenced or kSingleThread modes) returns false on a thread on which a // Task is run. -// - The TaskRunnerHandles set in the context of the task don't match what's -// expected for the tested TaskSourceExecutionMode. +// - The task runner CurrentDefaultHandles set in the context of the task don't +// match what's expected for the tested TaskSourceExecutionMode. // - The TaskSourceExecutionMode of the TaskRunner is kSequenced or // kSingleThread and Tasks don't run in posting order. // - The TaskSourceExecutionMode of the TaskRunner is kSingleThread and Tasks diff --git a/chromium/base/task/thread_pool/test_utils.cc b/chromium/base/task/thread_pool/test_utils.cc index 46e4cbd72d2..880b419fcaf 100644 --- a/chromium/base/task/thread_pool/test_utils.cc +++ b/chromium/base/task/thread_pool/test_utils.cc @@ -6,7 +6,8 @@ #include <utility> -#include "base/bind.h" +#include "base/debug/leak_annotations.h" +#include "base/functional/bind.h" #include "base/memory/raw_ptr.h" #include "base/synchronization/condition_variable.h" #include "base/task/thread_pool/pooled_parallel_task_runner.h" @@ -164,8 +165,14 @@ bool MockPooledTaskRunnerDelegate::PostTaskWithSequence( DCHECK(task.task); DCHECK(sequence); - if (!task_tracker_->WillPostTask(&task, sequence->shutdown_behavior())) + if (!task_tracker_->WillPostTask(&task, sequence->shutdown_behavior())) { + // `task`'s destructor may run sequence-affine code, so it must be leaked + // when `WillPostTask` returns false. + auto leak = std::make_unique<Task>(std::move(task)); + ANNOTATE_LEAKING_OBJECT_PTR(leak.get()); + leak.release(); return false; + } if (task.delayed_run_time.is_null()) { PostTaskWithSequenceNow(std::move(task), std::move(sequence)); diff --git a/chromium/base/task/thread_pool/test_utils.h b/chromium/base/task/thread_pool/test_utils.h index e7a43dbcd96..1db33ffe808 100644 --- a/chromium/base/task/thread_pool/test_utils.h +++ b/chromium/base/task/thread_pool/test_utils.h @@ -8,7 +8,7 @@ #include <atomic> #include <memory> -#include "base/callback.h" +#include "base/functional/callback.h" #include "base/memory/raw_ptr.h" #include "base/task/common/checked_lock.h" #include "base/task/post_job.h" diff --git a/chromium/base/task/thread_pool/thread_group.cc b/chromium/base/task/thread_pool/thread_group.cc index 56d8747540f..2855a21fd42 100644 --- a/chromium/base/task/thread_pool/thread_group.cc +++ b/chromium/base/task/thread_pool/thread_group.cc @@ -6,9 +6,9 @@ #include <utility> -#include "base/bind.h" -#include "base/callback_helpers.h" #include "base/feature_list.h" +#include "base/functional/bind.h" +#include "base/functional/callback_helpers.h" #include "base/lazy_instance.h" #include "base/task/task_features.h" #include "base/task/thread_pool/task_tracker.h" @@ -17,9 +17,7 @@ #if BUILDFLAG(IS_WIN) #include "base/win/com_init_check_hook.h" -#include "base/win/scoped_com_initializer.h" #include "base/win/scoped_winrt_initializer.h" -#include "base/win/windows_version.h" #endif namespace base { @@ -176,8 +174,13 @@ void ThreadGroup::ReEnqueueTaskSourceLockRequired( } else { // If the TaskSource should be reenqueued in the current thread group, // reenqueue it inside the scope of the lock. - auto sort_key = transaction_with_task_source.task_source->GetSortKey(); if (push_to_immediate_queue) { + auto sort_key = transaction_with_task_source.task_source->GetSortKey(); + // When moving |task_source| into |priority_queue_|, it may be destroyed + // on another thread as soon as |lock_| is released, since we're no + // longer holding a reference to it. To prevent UAF, release + // |transaction| before moving |task_source|. Ref. crbug.com/1412008 + transaction_with_task_source.transaction.Release(); priority_queue_.Push( std::move(transaction_with_task_source.task_source), sort_key); } @@ -252,6 +255,11 @@ void ThreadGroup::PushTaskSourceAndWakeUpWorkersImpl( return; } auto sort_key = transaction_with_task_source.task_source->GetSortKey(); + // When moving |task_source| into |priority_queue_|, it may be destroyed + // on another thread as soon as |lock_| is released, since we're no longer + // holding a reference to it. To prevent UAF, release |transaction| before + // moving |task_source|. Ref. crbug.com/1412008 + transaction_with_task_source.transaction.Release(); priority_queue_.Push(std::move(transaction_with_task_source.task_source), sort_key); EnsureEnoughWorkersLockRequired(executor); @@ -327,18 +335,8 @@ bool ThreadGroup::ShouldYield(TaskSourceSortKey sort_key) { std::unique_ptr<win::ScopedWindowsThreadEnvironment> ThreadGroup::GetScopedWindowsThreadEnvironment(WorkerEnvironment environment) { std::unique_ptr<win::ScopedWindowsThreadEnvironment> scoped_environment; - switch (environment) { - case WorkerEnvironment::COM_MTA: { - if (win::GetVersion() >= win::Version::WIN8) { - scoped_environment = std::make_unique<win::ScopedWinrtInitializer>(); - } else { - scoped_environment = std::make_unique<win::ScopedCOMInitializer>( - win::ScopedCOMInitializer::kMTA); - } - break; - } - default: - break; + if (environment == WorkerEnvironment::COM_MTA) { + scoped_environment = std::make_unique<win::ScopedWinrtInitializer>(); } DCHECK(!scoped_environment || scoped_environment->Succeeded()); diff --git a/chromium/base/task/thread_pool/thread_group_impl.cc b/chromium/base/task/thread_pool/thread_group_impl.cc index 32036d47f1f..24f41898fcd 100644 --- a/chromium/base/task/thread_pool/thread_group_impl.cc +++ b/chromium/base/task/thread_pool/thread_group_impl.cc @@ -12,11 +12,11 @@ #include "base/atomicops.h" #include "base/auto_reset.h" -#include "base/bind.h" -#include "base/callback_helpers.h" #include "base/compiler_specific.h" #include "base/containers/stack_container.h" #include "base/feature_list.h" +#include "base/functional/bind.h" +#include "base/functional/callback_helpers.h" #include "base/location.h" #include "base/memory/ptr_util.h" #include "base/memory/raw_ptr.h" @@ -791,8 +791,11 @@ void ThreadGroupImpl::WorkerThreadDelegateImpl::RecordUnnecessaryWakeup() { void ThreadGroupImpl::WorkerThreadDelegateImpl::BlockingStarted( BlockingType blocking_type) { DCHECK_CALLED_ON_VALID_THREAD(worker_thread_checker_); - DCHECK(read_worker().current_task_priority); DCHECK(worker_only().worker_thread_); + // Skip if this blocking scope happened outside of a RunTask. + if (!read_worker().current_task_priority) { + return; + } worker_only().worker_thread_->MaybeUpdateThreadType(); @@ -828,7 +831,10 @@ void ThreadGroupImpl::WorkerThreadDelegateImpl::BlockingStarted( void ThreadGroupImpl::WorkerThreadDelegateImpl::BlockingTypeUpgraded() { DCHECK_CALLED_ON_VALID_THREAD(worker_thread_checker_); - DCHECK(read_worker().current_task_priority); + // Skip if this blocking scope happened outside of a RunTask. + if (!read_worker().current_task_priority) { + return; + } // The blocking type always being WILL_BLOCK in this experiment and with time // overrides, it should never be considered "upgraded". @@ -854,9 +860,12 @@ void ThreadGroupImpl::WorkerThreadDelegateImpl::BlockingTypeUpgraded() { void ThreadGroupImpl::WorkerThreadDelegateImpl::BlockingEnded() { DCHECK_CALLED_ON_VALID_THREAD(worker_thread_checker_); + // Skip if this blocking scope happened outside of a RunTask. + if (!read_worker().current_task_priority) { + return; + } CheckedAutoLock auto_lock(outer_->lock_); - DCHECK(read_worker().current_task_priority); DCHECK(!read_worker().blocking_start_time.is_null()); write_worker().blocking_start_time = TimeTicks(); if (!incremented_max_tasks_for_shutdown_) { diff --git a/chromium/base/task/thread_pool/thread_group_impl_unittest.cc b/chromium/base/task/thread_pool/thread_group_impl_unittest.cc index 01a82feb290..3bbc5c95a89 100644 --- a/chromium/base/task/thread_pool/thread_group_impl_unittest.cc +++ b/chromium/base/task/thread_pool/thread_group_impl_unittest.cc @@ -15,9 +15,9 @@ #include "base/atomicops.h" #include "base/barrier_closure.h" -#include "base/bind.h" -#include "base/callback.h" -#include "base/callback_helpers.h" +#include "base/functional/bind.h" +#include "base/functional/callback.h" +#include "base/functional/callback_helpers.h" #include "base/memory/ptr_util.h" #include "base/memory/raw_ptr.h" #include "base/memory/ref_counted.h" @@ -84,6 +84,7 @@ class ThreadGroupImplImplTestBase : public ThreadGroup::Delegate { task_tracker_.FlushForTesting(); if (thread_group_) thread_group_->JoinForTesting(); + mock_pooled_task_runner_delegate_.SetThreadGroup(nullptr); thread_group_.reset(); } @@ -1423,6 +1424,7 @@ TEST_F(ThreadGroupImplBlockingTest, ThreadBusyShutdown) { task_tracker_.FlushForTesting(); thread_group_->JoinForTesting(); EXPECT_EQ(thread_group_->GetMaxTasksForTesting(), kMaxTasks); + mock_pooled_task_runner_delegate_.SetThreadGroup(nullptr); thread_group_.reset(); } @@ -1907,6 +1909,7 @@ TEST_F(ThreadGroupImplImplStartInBodyTest, RacyCleanup) { // Unwinding this test will be racy if worker cleanup can race with // ThreadGroupImpl destruction : https://crbug.com/810464. + mock_pooled_task_runner_delegate_.SetThreadGroup(nullptr); thread_group_.reset(); } diff --git a/chromium/base/task/thread_pool/thread_group_unittest.cc b/chromium/base/task/thread_pool/thread_group_unittest.cc index 233c84b0d86..19b60d9dfd4 100644 --- a/chromium/base/task/thread_pool/thread_group_unittest.cc +++ b/chromium/base/task/thread_pool/thread_group_unittest.cc @@ -9,8 +9,8 @@ #include <utility> #include "base/barrier_closure.h" -#include "base/bind.h" -#include "base/callback_helpers.h" +#include "base/functional/bind.h" +#include "base/functional/callback_helpers.h" #include "base/location.h" #include "base/memory/ref_counted.h" #include "base/task/task_runner.h" @@ -103,9 +103,7 @@ class ThreadGroupTestBase : public testing::Test, public ThreadGroup::Delegate { void TearDown() override { delayed_task_manager_.Shutdown(); service_thread_.Stop(); - if (thread_group_) - thread_group_->JoinForTesting(); - thread_group_.reset(); + DestroyThreadGroup(); } void CreateThreadGroup() { @@ -127,6 +125,16 @@ class ThreadGroupTestBase : public testing::Test, public ThreadGroup::Delegate { worker_environment); } + void DestroyThreadGroup() { + if (!thread_group_) { + return; + } + + thread_group_->JoinForTesting(); + mock_pooled_task_runner_delegate_.SetThreadGroup(nullptr); + thread_group_.reset(); + } + Thread service_thread_{"ThreadPoolServiceThread"}; TaskTracker task_tracker_; DelayedTaskManager delayed_task_manager_; @@ -733,6 +741,7 @@ TEST_F(ThreadGroupTest, JoinJobTaskSource) { thread_group_->JoinForTesting(); EXPECT_EQ(1U, task_source->HasOneRef()); // Prevent TearDown() from calling JoinForTesting() again. + mock_pooled_task_runner_delegate_.SetThreadGroup(nullptr); thread_group_ = nullptr; } diff --git a/chromium/base/task/thread_pool/thread_pool_impl.cc b/chromium/base/task/thread_pool/thread_pool_impl.cc index ad911cdd90f..4eedf8f24b4 100644 --- a/chromium/base/task/thread_pool/thread_pool_impl.cc +++ b/chromium/base/task/thread_pool/thread_pool_impl.cc @@ -9,12 +9,13 @@ #include <utility> #include "base/base_switches.h" -#include "base/bind.h" -#include "base/callback_helpers.h" #include "base/command_line.h" #include "base/compiler_specific.h" #include "base/debug/alias.h" +#include "base/debug/leak_annotations.h" #include "base/feature_list.h" +#include "base/functional/bind.h" +#include "base/functional/callback_helpers.h" #include "base/message_loop/message_pump_type.h" #include "base/metrics/field_trial_params.h" #include "base/strings/string_util.h" @@ -76,7 +77,8 @@ ThreadPoolImpl::ThreadPoolImpl(StringPiece histogram_label) : ThreadPoolImpl(histogram_label, std::make_unique<TaskTrackerImpl>()) {} ThreadPoolImpl::ThreadPoolImpl(StringPiece histogram_label, - std::unique_ptr<TaskTrackerImpl> task_tracker) + std::unique_ptr<TaskTrackerImpl> task_tracker, + bool use_background_threads) : histogram_label_(histogram_label), task_tracker_(std::move(task_tracker)), single_thread_task_runner_manager_(task_tracker_->GetTrackedRef(), @@ -101,7 +103,9 @@ ThreadPoolImpl::ThreadPoolImpl(StringPiece histogram_label, kBackgroundPoolEnvironmentParams.name_suffix}, "."), kBackgroundPoolEnvironmentParams.name_suffix, - kBackgroundPoolEnvironmentParams.thread_type_hint, + use_background_threads + ? kBackgroundPoolEnvironmentParams.thread_type_hint + : kForegroundPoolEnvironmentParams.thread_type_hint, task_tracker_->GetTrackedRef(), tracked_ref_factory_.GetTrackedRef()); } } @@ -386,6 +390,14 @@ void ThreadPoolImpl::EndBestEffortFence() { UpdateCanRunPolicy(); } +void ThreadPoolImpl::BeginFizzlingBlockShutdownTasks() { + task_tracker_->BeginFizzlingBlockShutdownTasks(); +} + +void ThreadPoolImpl::EndFizzlingBlockShutdownTasks() { + task_tracker_->EndFizzlingBlockShutdownTasks(); +} + bool ThreadPoolImpl::PostTaskWithSequenceNow(Task task, scoped_refptr<Sequence> sequence) { auto transaction = sequence->BeginTransaction(); @@ -424,8 +436,14 @@ bool ThreadPoolImpl::PostTaskWithSequence(Task task, DEBUG_ALIAS_FOR_CSTR(task_posted_from, task.posted_from.file_name(), 32); #endif - if (!task_tracker_->WillPostTask(&task, sequence->shutdown_behavior())) + if (!task_tracker_->WillPostTask(&task, sequence->shutdown_behavior())) { + // `task`'s destructor may run sequence-affine code, so it must be leaked + // when `WillPostTask` returns false. + auto leak = std::make_unique<Task>(std::move(task)); + ANNOTATE_LEAKING_OBJECT_PTR(leak.get()); + leak.release(); return false; + } if (task.delayed_run_time.is_null()) { return PostTaskWithSequenceNow(std::move(task), std::move(sequence)); diff --git a/chromium/base/task/thread_pool/thread_pool_impl.h b/chromium/base/task/thread_pool/thread_pool_impl.h index b1a21edcdef..6b113ee7c1c 100644 --- a/chromium/base/task/thread_pool/thread_pool_impl.h +++ b/chromium/base/task/thread_pool/thread_pool_impl.h @@ -8,8 +8,8 @@ #include <memory> #include "base/base_export.h" -#include "base/callback.h" #include "base/dcheck_is_on.h" +#include "base/functional/callback.h" #include "base/memory/ptr_util.h" #include "base/sequence_checker.h" #include "base/strings/string_piece.h" @@ -54,8 +54,11 @@ class BASE_EXPORT ThreadPoolImpl : public ThreadPoolInstance, explicit ThreadPoolImpl(StringPiece histogram_label); // For testing only. Creates a ThreadPoolImpl with a custom TaskTracker. + // If |!use_background_threads|, background threads will run with default + // priority. ThreadPoolImpl(StringPiece histogram_label, - std::unique_ptr<TaskTrackerImpl> task_tracker); + std::unique_ptr<TaskTrackerImpl> task_tracker, + bool use_background_threads = true); ThreadPoolImpl(const ThreadPoolImpl&) = delete; ThreadPoolImpl& operator=(const ThreadPoolImpl&) = delete; @@ -76,6 +79,8 @@ class BASE_EXPORT ThreadPoolImpl : public ThreadPoolInstance, void EndFence() override; void BeginBestEffortFence() override; void EndBestEffortFence() override; + void BeginFizzlingBlockShutdownTasks() override; + void EndFizzlingBlockShutdownTasks() override; // TaskExecutor: bool PostDelayedTask(const Location& from_here, diff --git a/chromium/base/task/thread_pool/thread_pool_impl_unittest.cc b/chromium/base/task/thread_pool/thread_pool_impl_unittest.cc index 0f882b3513b..e0f712cf737 100644 --- a/chromium/base/task/thread_pool/thread_pool_impl_unittest.cc +++ b/chromium/base/task/thread_pool/thread_pool_impl_unittest.cc @@ -13,12 +13,12 @@ #include <vector> #include "base/base_switches.h" -#include "base/bind.h" -#include "base/callback.h" -#include "base/callback_helpers.h" #include "base/cfi_buildflags.h" #include "base/containers/span.h" #include "base/debug/stack_trace.h" +#include "base/functional/bind.h" +#include "base/functional/callback.h" +#include "base/functional/callback_helpers.h" #include "base/memory/raw_ptr.h" #include "base/message_loop/message_pump_type.h" #include "base/metrics/field_trial.h" @@ -293,8 +293,8 @@ class ThreadPoolImplTestBase : public testing::Test { virtual bool GetUseResourceEfficientThreadGroup() const = 0; void set_worker_thread_observer( - WorkerThreadObserver* worker_thread_observer) { - worker_thread_observer_ = worker_thread_observer; + std::unique_ptr<WorkerThreadObserver> worker_thread_observer) { + worker_thread_observer_ = std::move(worker_thread_observer); } void StartThreadPool( @@ -307,7 +307,7 @@ class ThreadPoolImplTestBase : public testing::Test { max_num_utility_threads); init_params.suggested_reclaim_time = reclaim_time; - thread_pool_->Start(init_params, worker_thread_observer_); + thread_pool_->Start(init_params, worker_thread_observer_.get()); } void TearDown() override { @@ -317,6 +317,7 @@ class ThreadPoolImplTestBase : public testing::Test { if (thread_pool_) { thread_pool_->FlushForTesting(); thread_pool_->JoinForTesting(); + thread_pool_.reset(); } did_tear_down_ = true; } @@ -336,7 +337,7 @@ class ThreadPoolImplTestBase : public testing::Test { } base::test::ScopedFeatureList feature_list_; - raw_ptr<WorkerThreadObserver> worker_thread_observer_ = nullptr; + std::unique_ptr<WorkerThreadObserver> worker_thread_observer_; bool did_tear_down_ = false; }; @@ -961,7 +962,7 @@ TEST_P(ThreadPoolImplTest, FileDescriptorWatcherNoOpsAfterShutdown) { thread_pool_->Shutdown(); constexpr char kByte = '!'; - ASSERT_TRUE(WriteFileDescriptor(pipes[1], as_bytes(make_span(&kByte, 1)))); + ASSERT_TRUE(WriteFileDescriptor(pipes[1], as_bytes(make_span(&kByte, 1u)))); // Give a chance for the file watcher to fire before closing the handles. PlatformThread::Sleep(TestTimeouts::tiny_timeout()); @@ -1152,8 +1153,10 @@ TEST_P(ThreadPoolImplTest, MAYBE_IdentifiableStacks) { } TEST_P(ThreadPoolImplTest, WorkerThreadObserver) { - testing::StrictMock<test::MockWorkerThreadObserver> observer; - set_worker_thread_observer(&observer); + auto owned_observer = + std::make_unique<testing::StrictMock<test::MockWorkerThreadObserver>>(); + auto* observer = owned_observer.get(); + set_worker_thread_observer(std::move(owned_observer)); // A worker should be created for each thread group. After that, 4 threads // should be created for each SingleThreadTaskRunnerThreadMode (8 on Windows). @@ -1195,7 +1198,7 @@ TEST_P(ThreadPoolImplTest, WorkerThreadObserver) { 0; #endif - EXPECT_CALL(observer, OnWorkerThreadMainEntry()) + EXPECT_CALL(*observer, OnWorkerThreadMainEntry()) .Times(kExpectedNumPoolWorkers + kExpectedNumSharedSingleThreadedWorkers + kExpectedNumDedicatedSingleThreadedWorkers + kExpectedNumCOMSharedSingleThreadedWorkers + @@ -1285,18 +1288,18 @@ TEST_P(ThreadPoolImplTest, WorkerThreadObserver) { // Release single-threaded workers. This should cause dedicated workers to // invoke OnWorkerThreadMainExit(). - observer.AllowCallsOnMainExit(kExpectedNumDedicatedSingleThreadedWorkers + - kExpectedNumCOMDedicatedSingleThreadedWorkers); + observer->AllowCallsOnMainExit(kExpectedNumDedicatedSingleThreadedWorkers + + kExpectedNumCOMDedicatedSingleThreadedWorkers); task_runners.clear(); - observer.WaitCallsOnMainExit(); + observer->WaitCallsOnMainExit(); // Join all remaining workers. This should cause shared single-threaded // workers and thread pool workers to invoke OnWorkerThreadMainExit(). - observer.AllowCallsOnMainExit(kExpectedNumPoolWorkers + - kExpectedNumSharedSingleThreadedWorkers + - kExpectedNumCOMSharedSingleThreadedWorkers); + observer->AllowCallsOnMainExit(kExpectedNumPoolWorkers + + kExpectedNumSharedSingleThreadedWorkers + + kExpectedNumCOMSharedSingleThreadedWorkers); TearDown(); - observer.WaitCallsOnMainExit(); + observer->WaitCallsOnMainExit(); } // Verify a basic EnqueueJobTaskSource() runs the worker task. diff --git a/chromium/base/task/thread_pool/thread_pool_instance.cc b/chromium/base/task/thread_pool/thread_pool_instance.cc index d5173c7afb9..2a915be21a4 100644 --- a/chromium/base/task/thread_pool/thread_pool_instance.cc +++ b/chromium/base/task/thread_pool/thread_pool_instance.cc @@ -25,7 +25,8 @@ size_t GetDefaultMaxNumUtilityThreads(size_t max_num_foreground_threads_in) { int num_of_efficient_processors = SysInfo::NumberOfEfficientProcessors(); if (num_of_efficient_processors != 0) { DCHECK_GT(num_of_efficient_processors, 0); - return static_cast<size_t>(num_of_efficient_processors); + return std::min(max_num_foreground_threads_in, + static_cast<size_t>(num_of_efficient_processors)); } return std::max<size_t>(1, max_num_foreground_threads_in / 2); } @@ -66,6 +67,20 @@ ThreadPoolInstance::ScopedBestEffortExecutionFence:: g_thread_pool->EndBestEffortFence(); } +ThreadPoolInstance::ScopedFizzleBlockShutdownTasks:: + ScopedFizzleBlockShutdownTasks() { + // It's possible for this to be called without a ThreadPool present in tests. + if (g_thread_pool) + g_thread_pool->BeginFizzlingBlockShutdownTasks(); +} + +ThreadPoolInstance::ScopedFizzleBlockShutdownTasks:: + ~ScopedFizzleBlockShutdownTasks() { + // It's possible for this to be called without a ThreadPool present in tests. + if (g_thread_pool) + g_thread_pool->EndFizzlingBlockShutdownTasks(); +} + #if !BUILDFLAG(IS_NACL) // static void ThreadPoolInstance::CreateAndStartWithDefaultParams(StringPiece name) { diff --git a/chromium/base/task/thread_pool/thread_pool_instance.h b/chromium/base/task/thread_pool/thread_pool_instance.h index 4f0214661a0..a4f78b39c42 100644 --- a/chromium/base/task/thread_pool/thread_pool_instance.h +++ b/chromium/base/task/thread_pool/thread_pool_instance.h @@ -8,7 +8,7 @@ #include <memory> #include "base/base_export.h" -#include "base/callback.h" +#include "base/functional/callback.h" #include "base/gtest_prod_util.h" #include "base/strings/string_piece.h" #include "base/task/sequenced_task_runner.h" @@ -123,6 +123,19 @@ class BASE_EXPORT ThreadPoolInstance { ~ScopedBestEffortExecutionFence(); }; + // Used to allow posting `BLOCK_SHUTDOWN` tasks after shutdown in a scope. The + // tasks will fizzle (not run) but not trigger any checks that aim to catch + // this class of ordering bugs. + class BASE_EXPORT ScopedFizzleBlockShutdownTasks { + public: + ScopedFizzleBlockShutdownTasks(); + ScopedFizzleBlockShutdownTasks(const ScopedFizzleBlockShutdownTasks&) = + delete; + ScopedFizzleBlockShutdownTasks& operator=( + const ScopedFizzleBlockShutdownTasks&) = delete; + ~ScopedFizzleBlockShutdownTasks(); + }; + // Destroying a ThreadPoolInstance is not allowed in production; it is always // leaked. In tests, it should only be destroyed after JoinForTesting() has // returned. @@ -180,6 +193,9 @@ class BASE_EXPORT ThreadPoolInstance { // after this call. virtual void JoinForTesting() = 0; + virtual void BeginFizzlingBlockShutdownTasks() = 0; + virtual void EndFizzlingBlockShutdownTasks() = 0; + // CreateAndStartWithDefaultParams(), Create(), and SetInstance() register a // ThreadPoolInstance to handle tasks posted through the thread_pool.h API for // this process. diff --git a/chromium/base/task/thread_pool/thread_pool_perftest.cc b/chromium/base/task/thread_pool/thread_pool_perftest.cc index a29d57d3ff4..f0f8cfbd1cb 100644 --- a/chromium/base/task/thread_pool/thread_pool_perftest.cc +++ b/chromium/base/task/thread_pool/thread_pool_perftest.cc @@ -8,9 +8,9 @@ #include <vector> #include "base/barrier_closure.h" -#include "base/bind.h" -#include "base/callback.h" -#include "base/callback_helpers.h" +#include "base/functional/bind.h" +#include "base/functional/callback.h" +#include "base/functional/callback_helpers.h" #include "base/memory/raw_ptr.h" #include "base/synchronization/waitable_event.h" #include "base/task/thread_pool.h" diff --git a/chromium/base/task/thread_pool/tracked_ref.h b/chromium/base/task/thread_pool/tracked_ref.h index 191d2656cf5..14ad4bfa9b2 100644 --- a/chromium/base/task/thread_pool/tracked_ref.h +++ b/chromium/base/task/thread_pool/tracked_ref.h @@ -10,6 +10,7 @@ #include "base/gtest_prod_util.h" #include "base/memory/ptr_util.h" #include "base/memory/raw_ptr.h" +#include "base/memory/raw_ptr_exclusion.h" #include "base/synchronization/waitable_event.h" #include "base/template_util.h" #include "third_party/abseil-cpp/absl/types/optional.h" @@ -119,8 +120,12 @@ class TrackedRef { factory_->live_tracked_refs_.Increment(); } - T* ptr_; - TrackedRefFactory<T>* factory_; + // This field is not a raw_ptr<> because it was filtered by the rewriter for: + // #union + RAW_PTR_EXCLUSION T* ptr_; + // This field is not a raw_ptr<> because it was filtered by the rewriter for: + // #union + RAW_PTR_EXCLUSION TrackedRefFactory<T>* factory_; }; // TrackedRefFactory<T> should be the last member of T. diff --git a/chromium/base/task/thread_pool/tracked_ref_unittest.cc b/chromium/base/task/thread_pool/tracked_ref_unittest.cc index a846646c2b9..2c29b007ad2 100644 --- a/chromium/base/task/thread_pool/tracked_ref_unittest.cc +++ b/chromium/base/task/thread_pool/tracked_ref_unittest.cc @@ -8,7 +8,7 @@ #include <utility> #include <vector> -#include "base/bind.h" +#include "base/functional/bind.h" #include "base/synchronization/atomic_flag.h" #include "base/test/test_timeouts.h" #include "base/threading/thread.h" diff --git a/chromium/base/task/thread_pool/worker_thread.cc b/chromium/base/task/thread_pool/worker_thread.cc index f5807b06a2d..508d314a770 100644 --- a/chromium/base/task/thread_pool/worker_thread.cc +++ b/chromium/base/task/thread_pool/worker_thread.cc @@ -10,13 +10,13 @@ #include <atomic> #include <utility> -#include "base/allocator/buildflags.h" +#include "base/allocator/partition_allocator/partition_alloc_buildflags.h" #include "base/allocator/partition_allocator/partition_alloc_config.h" -#include "base/callback_helpers.h" #include "base/check_op.h" #include "base/compiler_specific.h" #include "base/debug/alias.h" #include "base/feature_list.h" +#include "base/functional/callback_helpers.h" #include "base/synchronization/waitable_event.h" #include "base/task/task_features.h" #include "base/task/thread_pool/environment_config.h" @@ -38,7 +38,7 @@ #endif #if BUILDFLAG(USE_PARTITION_ALLOC_AS_MALLOC) && \ - defined(PA_THREAD_CACHE_SUPPORTED) + PA_CONFIG(THREAD_CACHE_SUPPORTED) #include "base/allocator/partition_allocator/thread_cache.h" #endif @@ -47,7 +47,7 @@ namespace base::internal { namespace { #if BUILDFLAG(USE_PARTITION_ALLOC_AS_MALLOC) && \ - defined(PA_THREAD_CACHE_SUPPORTED) + PA_CONFIG(THREAD_CACHE_SUPPORTED) // Returns the desired sleep time before the worker has to wake up to purge // the cache thread or reclaim itself. |min_sleep_time| contains the minimal // acceptable amount of time to sleep. @@ -77,7 +77,8 @@ TimeDelta GetSleepTimeBeforePurge(TimeDelta min_sleep_time) { // that's too short. return std::max(snapped_wake - now, first_scheduled_wake - now); } -#endif +#endif // BUILDFLAG(USE_PARTITION_ALLOC_AS_MALLOC) && + // PA_CONFIG(THREAD_CACHE_SUPPORTED) bool IsDelayFirstWorkerSleepEnabled() { static bool state = FeatureList::IsEnabled(kDelayFirstWorkerWake); @@ -107,7 +108,7 @@ void WorkerThread::Delegate::WaitForWork(WaitableEvent* wake_up_event) { // that we do no work for short sleeps, and that threads do not get awaken // many times. #if BUILDFLAG(USE_PARTITION_ALLOC_AS_MALLOC) && \ - defined(PA_THREAD_CACHE_SUPPORTED) + PA_CONFIG(THREAD_CACHE_SUPPORTED) TimeDelta min_sleep_time = std::min(sleep_time, kPurgeThreadCacheIdleDelay); if (IsDelayFirstWorkerSleepEnabled()) @@ -130,7 +131,7 @@ void WorkerThread::Delegate::WaitForWork(WaitableEvent* wake_up_event) { #else wake_up_event->TimedWait(sleep_time); #endif // BUILDFLAG(USE_PARTITION_ALLOC_AS_MALLOC) && - // defined(PA_THREAD_CACHE_SUPPORTED) + // PA_CONFIG(THREAD_CACHE_SUPPORTED) } WorkerThread::WorkerThread(ThreadType thread_type_hint, @@ -151,7 +152,6 @@ WorkerThread::WorkerThread(ThreadType thread_type_hint, DCHECK(CanUseUtilityThreadTypeForWorkerThread() || thread_type_hint != ThreadType::kUtility); wake_up_event_.declare_only_used_while_idle(); - wake_up_event_.opt_out_of_wakeup_flow_events(); } bool WorkerThread::Start( diff --git a/chromium/base/task/thread_pool/worker_thread_unittest.cc b/chromium/base/task/thread_pool/worker_thread_unittest.cc index 54c40918b01..02439b8edff 100644 --- a/chromium/base/task/thread_pool/worker_thread_unittest.cc +++ b/chromium/base/task/thread_pool/worker_thread_unittest.cc @@ -11,13 +11,13 @@ #include <utility> #include <vector> -#include "base/allocator/buildflags.h" +#include "base/allocator/partition_allocator/partition_alloc_buildflags.h" #include "base/allocator/partition_allocator/partition_alloc_config.h" #include "base/allocator/partition_allocator/shim/allocator_shim.h" #include "base/allocator/partition_allocator/shim/allocator_shim_default_dispatch_to_partition_alloc.h" -#include "base/bind.h" -#include "base/callback_helpers.h" #include "base/compiler_specific.h" +#include "base/functional/bind.h" +#include "base/functional/callback_helpers.h" #include "base/memory/ptr_util.h" #include "base/memory/raw_ptr.h" #include "base/memory/ref_counted.h" @@ -41,10 +41,10 @@ #include "testing/gtest/include/gtest/gtest.h" #if BUILDFLAG(USE_PARTITION_ALLOC_AS_MALLOC) && \ - defined(PA_THREAD_CACHE_SUPPORTED) + PA_CONFIG(THREAD_CACHE_SUPPORTED) #include "base/allocator/partition_allocator/thread_cache.h" #endif // BUILDFLAG(USE_PARTITION_ALLOC_AS_MALLOC) && - // defined(PA_THREAD_CACHE_SUPPORTED) + // PA_CONFIG(THREAD_CACHE_SUPPORTED) using testing::_; using testing::Mock; @@ -697,13 +697,13 @@ class CallJoinFromDifferentThread : public SimpleThread { void Run() override { run_started_event_.Signal(); - worker_to_join_->JoinForTesting(); + worker_to_join_.ExtractAsDangling()->JoinForTesting(); } void WaitForRunToStart() { run_started_event_.Wait(); } private: - const raw_ptr<WorkerThread> worker_to_join_; + raw_ptr<WorkerThread> worker_to_join_; TestWaitableEvent run_started_event_; }; @@ -881,7 +881,7 @@ TEST(ThreadPoolWorkerTest, WorkerThreadObserver) { } #if BUILDFLAG(USE_PARTITION_ALLOC_AS_MALLOC) && \ - defined(PA_THREAD_CACHE_SUPPORTED) + PA_CONFIG(THREAD_CACHE_SUPPORTED) namespace { NOINLINE void FreeForTest(void* data) { free(data); @@ -957,7 +957,7 @@ TEST(ThreadPoolWorkerThreadCachePurgeTest, Purge) { } #endif // BUILDFLAG(USE_PARTITION_ALLOC_AS_MALLOC) && - // defined(PA_THREAD_CACHE_SUPPORTED) && + // PA_CONFIG(THREAD_CACHE_SUPPORTED) } // namespace internal } // namespace base |