Commit 66c95313 authored by Clemens Hammacher's avatar Clemens Hammacher Committed by Commit Bot

Revert "Use std::atomic in Cancelable"

This reverts commit 61d42c94.

Reason for revert: TSan failures: https://ci.chromium.org/p/v8/builders/luci.v8.ci/V8%20Linux64%20TSAN/23459

Original change's description:
> Use std::atomic in Cancelable
> 
> Avoid deprecated base::AtomicValue, use std::atomic instead.
> Plus minor drive-by cleanups.
> 
> R=​mstarzinger@chromium.org
> 
> Bug: v8:8238
> Change-Id: I47a1f00e26a843b60380c50399eedc49d859830a
> Reviewed-on: https://chromium-review.googlesource.com/c/1326463
> Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
> Reviewed-by: Michael Starzinger <mstarzinger@chromium.org>
> Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#57386}

TBR=ulan@chromium.org,mstarzinger@chromium.org,clemensh@chromium.org

Change-Id: I96f269800eb9c26812050629f7f2c75096f3c858
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: v8:8238
Reviewed-on: https://chromium-review.googlesource.com/c/1329201Reviewed-by: 's avatarClemens Hammacher <clemensh@chromium.org>
Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#57387}
parent 61d42c94
...@@ -10,12 +10,18 @@ ...@@ -10,12 +10,18 @@
namespace v8 { namespace v8 {
namespace internal { namespace internal {
Cancelable::Cancelable(CancelableTaskManager* parent)
: parent_(parent), status_(kWaiting), id_(0), cancel_counter_(0) {
id_ = parent->Register(this);
}
Cancelable::~Cancelable() { Cancelable::~Cancelable() {
// The following check is needed to avoid calling an already terminated // The following check is needed to avoid calling an already terminated
// manager object. This happens when the manager cancels all pending tasks // manager object. This happens when the manager cancels all pending tasks
// in {CancelAndWait} only before destroying the manager object. // in {CancelAndWait} only before destroying the manager object.
Status previous; if (TryRun() || IsRunning()) {
if (TryRun(&previous) || previous == kRunning) {
parent_->RemoveFinishedTask(id_); parent_->RemoveFinishedTask(id_);
} }
} }
......
...@@ -5,10 +5,10 @@ ...@@ -5,10 +5,10 @@
#ifndef V8_CANCELABLE_TASK_H_ #ifndef V8_CANCELABLE_TASK_H_
#define V8_CANCELABLE_TASK_H_ #define V8_CANCELABLE_TASK_H_
#include <atomic>
#include <unordered_map> #include <unordered_map>
#include "include/v8-platform.h" #include "include/v8-platform.h"
#include "src/base/atomic-utils.h"
#include "src/base/macros.h" #include "src/base/macros.h"
#include "src/base/platform/condition-variable.h" #include "src/base/platform/condition-variable.h"
#include "src/globals.h" #include "src/globals.h"
...@@ -86,9 +86,7 @@ class V8_EXPORT_PRIVATE CancelableTaskManager { ...@@ -86,9 +86,7 @@ class V8_EXPORT_PRIVATE CancelableTaskManager {
class V8_EXPORT_PRIVATE Cancelable { class V8_EXPORT_PRIVATE Cancelable {
public: public:
explicit Cancelable(CancelableTaskManager* parent) explicit Cancelable(CancelableTaskManager* parent);
: parent_(parent), id_(parent->Register(this)) {}
virtual ~Cancelable(); virtual ~Cancelable();
// Never invoke after handing over the task to the platform! The reason is // Never invoke after handing over the task to the platform! The reason is
...@@ -99,48 +97,42 @@ class V8_EXPORT_PRIVATE Cancelable { ...@@ -99,48 +97,42 @@ class V8_EXPORT_PRIVATE Cancelable {
CancelableTaskManager::Id id() { return id_; } CancelableTaskManager::Id id() { return id_; }
protected: protected:
bool TryRun() { return status_.TrySetValue(kWaiting, kRunning); }
bool IsRunning() { return status_.Value() == kRunning; }
intptr_t CancelAttempts() { return cancel_counter_; }
private:
// Identifies the state a cancelable task is in: // Identifies the state a cancelable task is in:
// |kWaiting|: The task is scheduled and waiting to be executed. {TryRun} will // |kWaiting|: The task is scheduled and waiting to be executed. {TryRun} will
// succeed. // succeed.
// |kCanceled|: The task has been canceled. {TryRun} will fail. // |kCanceled|: The task has been canceled. {TryRun} will fail.
// |kRunning|: The task is currently running and cannot be canceled anymore. // |kRunning|: The task is currently running and cannot be canceled anymore.
enum Status { kWaiting, kCanceled, kRunning }; enum Status {
kWaiting,
bool TryRun(Status* previous = nullptr) { kCanceled,
return CompareExchangeStatus(kWaiting, kRunning, previous); kRunning,
} };
intptr_t CancelAttempts() { return cancel_counter_; }
private:
friend class CancelableTaskManager;
// Use {CancelableTaskManager} to abort a task that has not yet been // Use {CancelableTaskManager} to abort a task that has not yet been
// executed. // executed.
bool Cancel() { bool Cancel() {
if (CompareExchangeStatus(kWaiting, kCanceled)) { if (status_.TrySetValue(kWaiting, kCanceled)) {
return true; return true;
} }
cancel_counter_++; cancel_counter_++;
return false; return false;
} }
bool CompareExchangeStatus(Status expected, Status desired, CancelableTaskManager* parent_;
Status* previous = nullptr) { base::AtomicValue<Status> status_;
// {compare_exchange_strong} updates {expected}. CancelableTaskManager::Id id_;
bool success = status_.compare_exchange_strong(expected, desired,
std::memory_order_relaxed);
if (previous) *previous = expected;
return success;
}
CancelableTaskManager* const parent_;
std::atomic<Status> status_{kWaiting};
const CancelableTaskManager::Id id_;
// The counter is incremented for failing tries to cancel a task. This can be // The counter is incremented for failing tries to cancel a task. This can be
// used by the task itself as an indication how often external entities tried // used by the task itself as an indication how often external entities tried
// to abort it. // to abort it.
std::atomic<intptr_t> cancel_counter_{0}; std::atomic<intptr_t> cancel_counter_;
friend class CancelableTaskManager;
DISALLOW_COPY_AND_ASSIGN(Cancelable); DISALLOW_COPY_AND_ASSIGN(Cancelable);
}; };
......
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