diff --git a/node.gyp b/node.gyp index 1f9d34b200966c..da4165b2330df3 100644 --- a/node.gyp +++ b/node.gyp @@ -925,6 +925,7 @@ 'test/cctest/test_base64.cc', 'test/cctest/test_node_postmortem_metadata.cc', 'test/cctest/test_environment.cc', + 'test/cctest/test_platform.cc', 'test/cctest/test_util.cc', 'test/cctest/test_url.cc' ], diff --git a/src/inspector_agent.cc b/src/inspector_agent.cc index ff15b4f6f414a6..2020ffa6e3cb82 100644 --- a/src/inspector_agent.cc +++ b/src/inspector_agent.cc @@ -314,7 +314,7 @@ class NodeInspectorClient : public V8InspectorClient { terminated_ = false; running_nested_loop_ = true; while (!terminated_ && channel_->waitForFrontendMessage()) { - platform_->FlushForegroundTasks(env_->isolate()); + while (platform_->FlushForegroundTasks(env_->isolate())) {} } terminated_ = false; running_nested_loop_ = false; diff --git a/src/node_platform.cc b/src/node_platform.cc index b2d73072c8b346..2123a9cbe9c51b 100644 --- a/src/node_platform.cc +++ b/src/node_platform.cc @@ -99,7 +99,7 @@ void PerIsolatePlatformData::PostDelayedTask( } PerIsolatePlatformData::~PerIsolatePlatformData() { - FlushForegroundTasksInternal(); + while (FlushForegroundTasksInternal()) {} CancelPendingDelayedTasks(); uv_close(reinterpret_cast(flush_tasks_), @@ -227,7 +227,13 @@ bool PerIsolatePlatformData::FlushForegroundTasksInternal() { }); }); } - while (std::unique_ptr task = foreground_tasks_.Pop()) { + // Move all foreground tasks into a separate queue and flush that queue. + // This way tasks that are posted while flushing the queue will be run on the + // next call of FlushForegroundTasksInternal. + std::queue> tasks = foreground_tasks_.PopAll(); + while (!tasks.empty()) { + std::unique_ptr task = std::move(tasks.front()); + tasks.pop(); did_work = true; RunForegroundTask(std::move(task)); } @@ -258,8 +264,8 @@ void NodePlatform::CallDelayedOnForegroundThread(Isolate* isolate, std::unique_ptr(task), delay_in_seconds); } -void NodePlatform::FlushForegroundTasks(v8::Isolate* isolate) { - ForIsolate(isolate)->FlushForegroundTasksInternal(); +bool NodePlatform::FlushForegroundTasks(v8::Isolate* isolate) { + return ForIsolate(isolate)->FlushForegroundTasksInternal(); } void NodePlatform::CancelPendingDelayedTasks(v8::Isolate* isolate) { @@ -352,4 +358,12 @@ void TaskQueue::Stop() { tasks_available_.Broadcast(scoped_lock); } +template +std::queue> TaskQueue::PopAll() { + Mutex::ScopedLock scoped_lock(lock_); + std::queue> result; + result.swap(task_queue_); + return result; +} + } // namespace node diff --git a/src/node_platform.h b/src/node_platform.h index b7546057871a1d..8f6ff89f491fe3 100644 --- a/src/node_platform.h +++ b/src/node_platform.h @@ -26,6 +26,7 @@ class TaskQueue { void Push(std::unique_ptr task); std::unique_ptr Pop(); std::unique_ptr BlockingPop(); + std::queue> PopAll(); void NotifyOfCompletion(); void BlockingDrain(); void Stop(); @@ -65,7 +66,9 @@ class PerIsolatePlatformData : void ref(); int unref(); - // Returns true iff work was dispatched or executed. + // Returns true if work was dispatched or executed. New tasks that are + // posted during flushing of the queue are postponed until the next + // flushing. bool FlushForegroundTasksInternal(); void CancelPendingDelayedTasks(); @@ -130,7 +133,10 @@ class NodePlatform : public MultiIsolatePlatform { double CurrentClockTimeMillis() override; v8::TracingController* GetTracingController() override; - void FlushForegroundTasks(v8::Isolate* isolate); + // Returns true if work was dispatched or executed. New tasks that are + // posted during flushing of the queue are postponed until the next + // flushing. + bool FlushForegroundTasks(v8::Isolate* isolate); void RegisterIsolate(IsolateData* isolate_data, uv_loop_t* loop) override; void UnregisterIsolate(IsolateData* isolate_data) override; diff --git a/test/cctest/test_platform.cc b/test/cctest/test_platform.cc new file mode 100644 index 00000000000000..876547480b7032 --- /dev/null +++ b/test/cctest/test_platform.cc @@ -0,0 +1,55 @@ +#include "node_internals.h" +#include "libplatform/libplatform.h" + +#include +#include "gtest/gtest.h" +#include "node_test_fixture.h" + +// This task increments the given run counter and reposts itself until the +// repost counter reaches zero. +class RepostingTask : public v8::Task { + public: + explicit RepostingTask(int repost_count, + int* run_count, + v8::Isolate* isolate, + node::NodePlatform* platform) + : repost_count_(repost_count), + run_count_(run_count), + isolate_(isolate), + platform_(platform) {} + + // v8::Task implementation + void Run() final { + ++*run_count_; + if (repost_count_ > 0) { + --repost_count_; + platform_->CallOnForegroundThread(isolate_, + new RepostingTask(repost_count_, run_count_, isolate_, platform_)); + } + } + + private: + int repost_count_; + int* run_count_; + v8::Isolate* isolate_; + node::NodePlatform* platform_; +}; + +class PlatformTest : public EnvironmentTestFixture {}; + +TEST_F(PlatformTest, SkipNewTasksInFlushForegroundTasks) { + v8::Isolate::Scope isolate_scope(isolate_); + const v8::HandleScope handle_scope(isolate_); + const Argv argv; + Env env {handle_scope, argv}; + int run_count = 0; + platform->CallOnForegroundThread( + isolate_, new RepostingTask(2, &run_count, isolate_, platform.get())); + EXPECT_TRUE(platform->FlushForegroundTasks(isolate_)); + EXPECT_EQ(1, run_count); + EXPECT_TRUE(platform->FlushForegroundTasks(isolate_)); + EXPECT_EQ(2, run_count); + EXPECT_TRUE(platform->FlushForegroundTasks(isolate_)); + EXPECT_EQ(3, run_count); + EXPECT_FALSE(platform->FlushForegroundTasks(isolate_)); +}