Commit 2ca27fec authored by Anton Bikineev's avatar Anton Bikineev Committed by Commit Bot

heap: Unregister cancelable task immediately after execution

Theoretically, if the scheduler for some reason doesn't destroy the
callback right after its execution,
CancelableTaskScheduler::CancelAndWait may stall. I'm not sure if this
is the reason why performance tests fail with timeouts on win-perf, but
it may be related to Windows scheduler specifics.

Bug: 987574
Change-Id: Id3bf951c78bf37aae66ce0657e113ddefe9918b1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1731930
Commit-Queue: Anton Bikineev <bikineev@chromium.org>
Reviewed-by: default avatarKentaro Hara <haraken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#683514}
parent 2917593c
...@@ -22,10 +22,8 @@ class CancelableTaskScheduler::TaskData { ...@@ -22,10 +22,8 @@ class CancelableTaskScheduler::TaskData {
~TaskData() { ~TaskData() {
// The task runner is responsible for unregistering the task in case the // The task runner is responsible for unregistering the task in case the
// task hasn't been cancelled. The first check returns true if the task // task hasn't been cancelled.
// hasn't been executed, the second returns true if it has. if (TryCancel()) {
Status previous;
if (TryRun(&previous) || previous == kRunning) {
scheduler_->UnregisterAndSignal(this); scheduler_->UnregisterAndSignal(this);
} }
} }
...@@ -33,6 +31,7 @@ class CancelableTaskScheduler::TaskData { ...@@ -33,6 +31,7 @@ class CancelableTaskScheduler::TaskData {
void Run() { void Run() {
if (TryRun()) { if (TryRun()) {
std::move(task_).Run(); std::move(task_).Run();
scheduler_->UnregisterAndSignal(this);
} }
} }
...@@ -51,17 +50,11 @@ class CancelableTaskScheduler::TaskData { ...@@ -51,17 +50,11 @@ class CancelableTaskScheduler::TaskData {
// |kRunning|: The task is currently running and cannot be canceled anymore. // |kRunning|: The task is currently running and cannot be canceled anymore.
enum Status : uint8_t { kWaiting, kCancelled, kRunning }; enum Status : uint8_t { kWaiting, kCancelled, kRunning };
friend size_t RemoveCancelledTasks(WTF::HashSet<TaskData*>*); bool TryRun() {
bool TryRun(Status* previous = nullptr) {
Status expected = kWaiting; Status expected = kWaiting;
const bool success = status_.compare_exchange_strong( return status_.compare_exchange_strong(expected, kRunning,
expected, kRunning, std::memory_order_acq_rel, std::memory_order_acq_rel,
std::memory_order_acquire); std::memory_order_acquire);
if (previous) {
*previous = expected;
}
return success;
} }
Task task_; Task task_;
...@@ -98,8 +91,8 @@ size_t CancelableTaskScheduler::CancelAndWait() { ...@@ -98,8 +91,8 @@ size_t CancelableTaskScheduler::CancelAndWait() {
std::unique_ptr<CancelableTaskScheduler::TaskData> std::unique_ptr<CancelableTaskScheduler::TaskData>
CancelableTaskScheduler::Register(Task task) { CancelableTaskScheduler::Register(Task task) {
base::AutoLock lock(lock_);
auto task_data = std::make_unique<TaskData>(std::move(task), this); auto task_data = std::make_unique<TaskData>(std::move(task), this);
base::AutoLock lock(lock_);
tasks_.insert(task_data.get()); tasks_.insert(task_data.get());
return task_data; return task_data;
} }
......
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