diff --git a/src/node_platform.cc b/src/node_platform.cc index 197102068b74f4..9c5326e96aa37d 100644 --- a/src/node_platform.cc +++ b/src/node_platform.cc @@ -116,6 +116,10 @@ class WorkerThreadsTaskRunner::DelayedTaskScheduler { std::unique_ptr task, double delay_in_seconds) { auto locked = tasks_.Lock(); + // Once Stop() has begun tearing down the scheduler, the uv_async_send() + // below would hit a closing handle and abort. Drop tasks that arrive + // during shutdown. + if (stopped_) return; auto entry = std::make_unique(std::move(task), priority); auto delayed = std::make_unique( @@ -132,6 +136,9 @@ class WorkerThreadsTaskRunner::DelayedTaskScheduler { void Stop() { auto locked = tasks_.Lock(); + // Flip stopped_ so no new tasks get scheduled while we shut down. + if (stopped_) return; + stopped_ = true; locked.Push(std::make_unique(this)); uv_async_send(&flush_tasks_); } @@ -238,6 +245,7 @@ class WorkerThreadsTaskRunner::DelayedTaskScheduler { // Locally scheduled tasks to be poped into the worker task runner queue. // It is flushed whenever the next closest timer expires. TaskQueue tasks_; + bool stopped_ = false; uv_loop_t loop_; uv_async_t flush_tasks_; std::unordered_set timers_; diff --git a/test/cctest/test_platform.cc b/test/cctest/test_platform.cc index f1c1d52d92c78f..be7b249e1f5bda 100644 --- a/test/cctest/test_platform.cc +++ b/test/cctest/test_platform.cc @@ -38,6 +38,40 @@ class RepostingTask : public v8::Task { node::NodePlatform* platform_; }; +class NoopTask : public v8::Task { + public: + void Run() final {} +}; + +class PostDelayedTaskAfterShutdownStartsTask : public v8::Task { + public: + PostDelayedTaskAfterShutdownStartsTask(node::NodePlatform* platform, + uv_sem_t* task_started, + uv_sem_t* post_task) + : platform_(platform), + task_started_(task_started), + post_task_(post_task) {} + + void Run() final { + uv_sem_post(task_started_); + uv_sem_wait(post_task_); + uv_sleep(50); + platform_->PostDelayedTaskOnWorkerThreadImpl(v8::TaskPriority::kUserVisible, + std::make_unique(), + 1.0, + v8::SourceLocation()); + } + + private: + node::NodePlatform* platform_; + uv_sem_t* task_started_; + uv_sem_t* post_task_; +}; + +static void ShutdownPlatform(void* arg) { + static_cast(arg)->Shutdown(); +} + class PlatformTest : public EnvironmentTestFixture {}; TEST_F(PlatformTest, SkipNewTasksInFlushForegroundTasks) { @@ -60,6 +94,40 @@ TEST_F(PlatformTest, SkipNewTasksInFlushForegroundTasks) { EXPECT_FALSE(platform->FlushForegroundTasks(isolate_)); } +// Regression test: a worker thread posting a delayed task concurrently with +// platform shutdown must not call uv_async_send() on the scheduler's +// flush_tasks_ handle after Stop() has begun closing it. The two semaphores +// and the sleeps pin the ordering so the delayed task is posted only after +// Shutdown() has run, which is the window that used to abort with +// "Assertion failed: !(handle->flags & UV_HANDLE_CLOSING)". The test passes +// when shutdown completes without crashing. +TEST_F(NodeZeroIsolateTestFixture, DelayedWorkerTaskDuringPlatformShutdown) { + node::NodePlatform test_platform(1, tracing_agent->GetTracingController()); + + uv_sem_t task_started; + uv_sem_t post_task; + ASSERT_EQ(0, uv_sem_init(&task_started, 0)); + ASSERT_EQ(0, uv_sem_init(&post_task, 0)); + + test_platform.PostTaskOnWorkerThreadImpl( + v8::TaskPriority::kUserBlocking, + std::make_unique( + &test_platform, &task_started, &post_task), + v8::SourceLocation()); + + uv_sem_wait(&task_started); + + uv_thread_t shutdown_thread; + ASSERT_EQ( + 0, uv_thread_create(&shutdown_thread, ShutdownPlatform, &test_platform)); + uv_sleep(50); + uv_sem_post(&post_task); + ASSERT_EQ(0, uv_thread_join(&shutdown_thread)); + + uv_sem_destroy(&post_task); + uv_sem_destroy(&task_started); +} + // Tests the registration of an abstract `IsolatePlatformDelegate` instance as // opposed to the more common `uv_loop_s*` version of `RegisterIsolate`. TEST_F(NodeZeroIsolateTestFixture, IsolatePlatformDelegateTest) {