Browse Source

deps: backport bca8409 from upstream V8

Original commit message:

  Make CancelableTask ids unique
  They were only limited to 32 bit when using the internal Hashmap. Since
  this has changed alreay some time ago, we can switch to 64 bit ids and
  check that we never overflow.

  Bug:
  Change-Id: Ia6c6d02d6b5e555c6941185a79427dc4aa2a1d62
  Reviewed-on: https://chromium-review.googlesource.com/598229
  Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
  Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
  Cr-Commit-Position: refs/heads/master@{#47085}

PR-URL: https://github.com/nodejs/node/pull/14001
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
canary-base
Matt Loring 8 years ago
committed by Anna Henningsen
parent
commit
64162a1041
No known key found for this signature in database GPG Key ID: D8B9F5AEAE84E4CF
  1. 13
      deps/v8/src/cancelable-task.cc
  2. 18
      deps/v8/src/cancelable-task.h
  3. 3
      deps/v8/src/heap/item-parallel-job.h
  4. 2
      deps/v8/src/heap/mark-compact.h
  5. 4
      deps/v8/test/unittests/cancelable-tasks-unittest.cc

13
deps/v8/src/cancelable-task.cc

@ -29,18 +29,17 @@ Cancelable::~Cancelable() {
CancelableTaskManager::CancelableTaskManager() CancelableTaskManager::CancelableTaskManager()
: task_id_counter_(0), canceled_(false) {} : task_id_counter_(0), canceled_(false) {}
uint32_t CancelableTaskManager::Register(Cancelable* task) { CancelableTaskManager::Id CancelableTaskManager::Register(Cancelable* task) {
base::LockGuard<base::Mutex> guard(&mutex_); base::LockGuard<base::Mutex> guard(&mutex_);
uint32_t id = ++task_id_counter_; CancelableTaskManager::Id id = ++task_id_counter_;
// The loop below is just used when task_id_counter_ overflows. // Id overflows are not supported.
while (cancelable_tasks_.count(id) > 0) ++id; CHECK_NE(0, id);
CHECK(!canceled_); CHECK(!canceled_);
cancelable_tasks_[id] = task; cancelable_tasks_[id] = task;
return id; return id;
} }
void CancelableTaskManager::RemoveFinishedTask(CancelableTaskManager::Id id) {
void CancelableTaskManager::RemoveFinishedTask(uint32_t id) {
base::LockGuard<base::Mutex> guard(&mutex_); base::LockGuard<base::Mutex> guard(&mutex_);
size_t removed = cancelable_tasks_.erase(id); size_t removed = cancelable_tasks_.erase(id);
USE(removed); USE(removed);
@ -49,7 +48,7 @@ void CancelableTaskManager::RemoveFinishedTask(uint32_t id) {
} }
CancelableTaskManager::TryAbortResult CancelableTaskManager::TryAbort( CancelableTaskManager::TryAbortResult CancelableTaskManager::TryAbort(
uint32_t id) { CancelableTaskManager::Id id) {
base::LockGuard<base::Mutex> guard(&mutex_); base::LockGuard<base::Mutex> guard(&mutex_);
auto entry = cancelable_tasks_.find(id); auto entry = cancelable_tasks_.find(id);
if (entry != cancelable_tasks_.end()) { if (entry != cancelable_tasks_.end()) {

18
deps/v8/src/cancelable-task.h

@ -5,7 +5,7 @@
#ifndef V8_CANCELABLE_TASK_H_ #ifndef V8_CANCELABLE_TASK_H_
#define V8_CANCELABLE_TASK_H_ #define V8_CANCELABLE_TASK_H_
#include <map> #include <unordered_map>
#include "include/v8-platform.h" #include "include/v8-platform.h"
#include "src/base/atomic-utils.h" #include "src/base/atomic-utils.h"
@ -24,12 +24,14 @@ class Isolate;
// from any fore- and background task/thread. // from any fore- and background task/thread.
class V8_EXPORT_PRIVATE CancelableTaskManager { class V8_EXPORT_PRIVATE CancelableTaskManager {
public: public:
using Id = uint64_t;
CancelableTaskManager(); CancelableTaskManager();
// Registers a new cancelable {task}. Returns the unique {id} of the task that // Registers a new cancelable {task}. Returns the unique {id} of the task that
// can be used to try to abort a task by calling {Abort}. // can be used to try to abort a task by calling {Abort}.
// Must not be called after CancelAndWait. // Must not be called after CancelAndWait.
uint32_t Register(Cancelable* task); Id Register(Cancelable* task);
// Try to abort running a task identified by {id}. The possible outcomes are: // Try to abort running a task identified by {id}. The possible outcomes are:
// (1) The task is already finished running or was canceled before and // (1) The task is already finished running or was canceled before and
@ -39,7 +41,7 @@ class V8_EXPORT_PRIVATE CancelableTaskManager {
// removed. // removed.
// //
enum TryAbortResult { kTaskRemoved, kTaskRunning, kTaskAborted }; enum TryAbortResult { kTaskRemoved, kTaskRunning, kTaskAborted };
TryAbortResult TryAbort(uint32_t id); TryAbortResult TryAbort(Id id);
// Cancels all remaining registered tasks and waits for tasks that are // Cancels all remaining registered tasks and waits for tasks that are
// already running. This disallows subsequent Register calls. // already running. This disallows subsequent Register calls.
@ -59,13 +61,13 @@ class V8_EXPORT_PRIVATE CancelableTaskManager {
private: private:
// Only called by {Cancelable} destructor. The task is done with executing, // Only called by {Cancelable} destructor. The task is done with executing,
// but needs to be removed. // but needs to be removed.
void RemoveFinishedTask(uint32_t id); void RemoveFinishedTask(Id id);
// To mitigate the ABA problem, the api refers to tasks through an id. // To mitigate the ABA problem, the api refers to tasks through an id.
uint32_t task_id_counter_; Id task_id_counter_;
// A set of cancelable tasks that are currently registered. // A set of cancelable tasks that are currently registered.
std::map<uint32_t, Cancelable*> cancelable_tasks_; std::unordered_map<Id, Cancelable*> cancelable_tasks_;
// Mutex and condition variable enabling concurrent register and removing, as // Mutex and condition variable enabling concurrent register and removing, as
// well as waiting for background tasks on {CancelAndWait}. // well as waiting for background tasks on {CancelAndWait}.
@ -89,7 +91,7 @@ class V8_EXPORT_PRIVATE Cancelable {
// a platform. This step transfers ownership to the platform, which destroys // a platform. This step transfers ownership to the platform, which destroys
// the task after running it. Since the exact time is not known, we cannot // the task after running it. Since the exact time is not known, we cannot
// access the object after handing it to a platform. // access the object after handing it to a platform.
uint32_t id() { return id_; } CancelableTaskManager::Id id() { return id_; }
protected: protected:
bool TryRun() { return status_.TrySetValue(kWaiting, kRunning); } bool TryRun() { return status_.TrySetValue(kWaiting, kRunning); }
@ -120,7 +122,7 @@ class V8_EXPORT_PRIVATE Cancelable {
CancelableTaskManager* parent_; CancelableTaskManager* parent_;
base::AtomicValue<Status> status_; base::AtomicValue<Status> status_;
uint32_t id_; 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

3
deps/v8/src/heap/item-parallel-job.h

@ -133,7 +133,8 @@ class ItemParallelJob {
const size_t num_tasks = tasks_.size(); const size_t num_tasks = tasks_.size();
const size_t num_items = items_.size(); const size_t num_items = items_.size();
const size_t items_per_task = (num_items + num_tasks - 1) / num_tasks; const size_t items_per_task = (num_items + num_tasks - 1) / num_tasks;
uint32_t* task_ids = new uint32_t[num_tasks]; CancelableTaskManager::Id* task_ids =
new CancelableTaskManager::Id[num_tasks];
size_t start_index = 0; size_t start_index = 0;
Task* main_task = nullptr; Task* main_task = nullptr;
Task* task = nullptr; Task* task = nullptr;

2
deps/v8/src/heap/mark-compact.h

@ -471,7 +471,7 @@ class MarkCompactCollector final : public MarkCompactCollectorBase {
Heap* const heap_; Heap* const heap_;
int num_tasks_; int num_tasks_;
uint32_t task_ids_[kMaxSweeperTasks]; CancelableTaskManager::Id task_ids_[kMaxSweeperTasks];
base::Semaphore pending_sweeper_tasks_semaphore_; base::Semaphore pending_sweeper_tasks_semaphore_;
base::Mutex mutex_; base::Mutex mutex_;
SweptList swept_list_[kAllocationSpaces]; SweptList swept_list_[kAllocationSpaces];

4
deps/v8/test/unittests/cancelable-tasks-unittest.cc

@ -180,7 +180,7 @@ TEST(CancelableTask, RemoveBeforeCancelAndWait) {
ResultType result1 = 0; ResultType result1 = 0;
TestTask* task1 = new TestTask(&manager, &result1, TestTask::kCheckNotRun); TestTask* task1 = new TestTask(&manager, &result1, TestTask::kCheckNotRun);
ThreadedRunner runner1(task1); ThreadedRunner runner1(task1);
uint32_t id = task1->id(); CancelableTaskManager::Id id = task1->id();
EXPECT_EQ(id, 1u); EXPECT_EQ(id, 1u);
EXPECT_TRUE(manager.TryAbort(id)); EXPECT_TRUE(manager.TryAbort(id));
runner1.Start(); runner1.Start();
@ -195,7 +195,7 @@ TEST(CancelableTask, RemoveAfterCancelAndWait) {
ResultType result1 = 0; ResultType result1 = 0;
TestTask* task1 = new TestTask(&manager, &result1); TestTask* task1 = new TestTask(&manager, &result1);
ThreadedRunner runner1(task1); ThreadedRunner runner1(task1);
uint32_t id = task1->id(); CancelableTaskManager::Id id = task1->id();
EXPECT_EQ(id, 1u); EXPECT_EQ(id, 1u);
runner1.Start(); runner1.Start();
runner1.Join(); runner1.Join();

Loading…
Cancel
Save