Commit f128dc75 authored by Alex Clarke's avatar Alex Clarke Committed by Commit Bot

Set crash keys before calling CallbackBase::IsCancelled

As a temporary measure, set crash keys before calling
CallbackBase::IsCancelled to see if there is any pattern among
the crashes.

Bug: 798554
Change-Id: I55fcba6b378abdbbd69531faeda91c7c19874eb6
Reviewed-on: https://chromium-review.googlesource.com/c/1257920
Commit-Queue: Alex Clarke <alexclarke@chromium.org>
Reviewed-by: default avatarAlexander Timin <altimin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#596686}
parent 467e19a6
......@@ -726,6 +726,17 @@ WeakPtr<SequenceManagerImpl> SequenceManagerImpl::GetWeakPtr() {
return weak_factory_.GetWeakPtr();
}
bool SequenceManagerImpl::SetCrashKeysAndCheckIsTaskCancelled(
const PendingTask& task) const {
#if !defined(OS_NACL)
debug::SetCrashKeyString(main_thread_only().file_name_crash_key,
task.posted_from.file_name());
debug::SetCrashKeyString(main_thread_only().function_name_crash_key,
task.posted_from.function_name());
#endif // OS_NACL
return task.task.IsCancelled();
}
void SequenceManagerImpl::SetDefaultTaskRunner(
scoped_refptr<SingleThreadTaskRunner> task_runner) {
controller_->SetDefaultTaskRunner(task_runner);
......
......@@ -159,6 +159,9 @@ class BASE_EXPORT SequenceManagerImpl
WeakPtr<SequenceManagerImpl> GetWeakPtr();
// TODO(alexclarke): Remove when possible.
bool SetCrashKeysAndCheckIsTaskCancelled(const PendingTask& task) const;
protected:
// Create a task queue manager where |controller| controls the thread
// on which the tasks are eventually run.
......
......@@ -403,10 +403,15 @@ Optional<TimeTicks> TaskQueueImpl::GetNextScheduledWakeUp() {
void TaskQueueImpl::WakeUpForDelayedWork(LazyNow* lazy_now) {
// Enqueue all delayed tasks that should be running now, skipping any that
// have been canceled.
const SequenceManagerImpl* sequence_manager =
main_thread_only().sequence_manager;
while (!main_thread_only().delayed_incoming_queue.empty()) {
Task& task =
const_cast<Task&>(main_thread_only().delayed_incoming_queue.top());
if (!task.task || task.task.IsCancelled()) {
// TODO(alexclarke): Use IsCancelled once we've understood the bug.
// See http://crbug.com/798554
if (!task.task ||
sequence_manager->SetCrashKeysAndCheckIsTaskCancelled(task)) {
main_thread_only().delayed_incoming_queue.pop();
continue;
}
......@@ -855,8 +860,13 @@ void TaskQueueImpl::SweepCanceledDelayedTasks(TimeTicks now) {
// Remove canceled tasks.
std::priority_queue<Task> remaining_tasks;
const SequenceManagerImpl* sequence_manager =
main_thread_only().sequence_manager;
while (!main_thread_only().delayed_incoming_queue.empty()) {
if (!main_thread_only().delayed_incoming_queue.top().task.IsCancelled()) {
// TODO(alexclarke): Use IsCancelled once we've understood the bug.
// See http://crbug.com/798554
if (!sequence_manager->SetCrashKeysAndCheckIsTaskCancelled(
main_thread_only().delayed_incoming_queue.top())) {
remaining_tasks.push(std::move(
const_cast<Task&>(main_thread_only().delayed_incoming_queue.top())));
}
......@@ -1000,6 +1010,12 @@ void TaskQueueImpl::ActivateDelayedFenceIfNeeded(TimeTicks now) {
main_thread_only().delayed_fence = nullopt;
}
void TaskQueueImpl::ClearSequenceManagerForTesting() {
AutoLock lock(any_thread_lock_);
any_thread().sequence_manager = nullptr;
main_thread_only().sequence_manager = nullptr;
}
} // namespace internal
} // namespace sequence_manager
} // namespace base
......@@ -235,6 +235,9 @@ class BASE_EXPORT TaskQueueImpl {
// constructed due to not having TaskQueue.
void SetQueueEnabledForTest(bool enabled);
// TODO(alexclarke): Remove when possible.
void ClearSequenceManagerForTesting();
protected:
void SetDelayedWakeUpForTesting(Optional<DelayedWakeUp> wake_up);
......
......@@ -4,6 +4,7 @@
#include "base/task/sequence_manager/work_queue.h"
#include "base/task/sequence_manager/sequence_manager_impl.h"
#include "base/task/sequence_manager/work_queue_sets.h"
namespace base {
......@@ -155,8 +156,13 @@ Task WorkQueue::TakeTaskFromWorkQueue() {
bool WorkQueue::RemoveAllCanceledTasksFromFront() {
DCHECK(work_queue_sets_);
bool task_removed = false;
while (!tasks_.empty() &&
(!tasks_.front().task || tasks_.front().task.IsCancelled())) {
const SequenceManagerImpl* sequence_manager = task_queue_->sequence_manager();
// TODO(alexclarke): Use IsCancelled once we've understood the bug.
// See http://crbug.com/798554
while (
!tasks_.empty() &&
(!tasks_.front().task ||
sequence_manager->SetCrashKeysAndCheckIsTaskCancelled(tasks_.front()))) {
tasks_.pop_front();
task_removed = true;
}
......
......@@ -9,6 +9,7 @@
#include "base/bind.h"
#include "base/task/sequence_manager/real_time_domain.h"
#include "base/task/sequence_manager/sequence_manager_impl.h"
#include "base/task/sequence_manager/task_queue_impl.h"
#include "base/task/sequence_manager/work_queue_sets.h"
#include "testing/gmock/include/gmock/gmock.h"
......@@ -34,8 +35,10 @@ struct Cancelable {
class WorkQueueTest : public testing::Test {
public:
void SetUp() override {
dummy_sequence_manager_ = SequenceManagerImpl::CreateUnbound(nullptr);
time_domain_.reset(new RealTimeDomain());
task_queue_ = std::make_unique<TaskQueueImpl>(nullptr, time_domain_.get(),
task_queue_ = std::make_unique<TaskQueueImpl>(dummy_sequence_manager_.get(),
time_domain_.get(),
TaskQueue::Spec("test"));
work_queue_.reset(new WorkQueue(task_queue_.get(), "test",
......@@ -44,7 +47,11 @@ class WorkQueueTest : public testing::Test {
work_queue_sets_->AddQueue(work_queue_.get(), 0);
}
void TearDown() override { work_queue_sets_->RemoveQueue(work_queue_.get()); }
void TearDown() override {
work_queue_sets_->RemoveQueue(work_queue_.get());
task_queue_->ClearSequenceManagerForTesting();
}
protected:
Task FakeCancelableTaskWithEnqueueOrder(int enqueue_order,
......@@ -71,6 +78,7 @@ class WorkQueueTest : public testing::Test {
return fake_task;
}
std::unique_ptr<SequenceManagerImpl> dummy_sequence_manager_;
std::unique_ptr<RealTimeDomain> time_domain_;
std::unique_ptr<TaskQueueImpl> task_queue_;
std::unique_ptr<WorkQueue> work_queue_;
......
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