@@ -78,7 +78,10 @@ static void PlatformWorkerThread(void* data) { |
78 | 78 | fflush(stderr); |
79 | 79 | } |
80 | 80 | entry->task->Run(); |
81 | | - pending_worker_tasks->Lock().NotifyOfCompletion(); |
| 81 | + // See NodePlatform::DrainTasks(). |
| 82 | + if (entry->is_outstanding()) { |
| 83 | + pending_worker_tasks->Lock().NotifyOfOutstandingCompletion(); |
| 84 | + } |
82 | 85 | } |
83 | 86 | } |
84 | 87 |
|
@@ -209,8 +212,10 @@ class WorkerThreadsTaskRunner::DelayedTaskScheduler { |
209 | 212 | static void RunTask(uv_timer_t* timer) { |
210 | 213 | DelayedTaskScheduler* scheduler = |
211 | 214 | ContainerOf(&DelayedTaskScheduler::loop_, timer->loop); |
212 | | - scheduler->pending_worker_tasks_->Lock().Push( |
213 | | - scheduler->TakeTimerTask(timer)); |
| 215 | + auto entry = scheduler->TakeTimerTask(timer); |
| 216 | + bool is_outstanding = entry->is_outstanding(); |
| 217 | + scheduler->pending_worker_tasks_->Lock().Push(std::move(entry), |
| 218 | + is_outstanding); |
214 | 219 | } |
215 | 220 |
|
216 | 221 | std::unique_ptr<TaskQueueEntry> TakeTimerTask(uv_timer_t* timer) { |
@@ -277,7 +282,8 @@ void WorkerThreadsTaskRunner::PostTask(v8::TaskPriority priority, |
277 | 282 | std::unique_ptr<v8::Task> task, |
278 | 283 | const v8::SourceLocation& location) { |
279 | 284 | auto entry = std::make_unique<TaskQueueEntry>(std::move(task), priority); |
280 | | - pending_worker_tasks_.Lock().Push(std::move(entry)); |
| 285 | + bool is_outstanding = entry->is_outstanding(); |
| 286 | + pending_worker_tasks_.Lock().Push(std::move(entry), is_outstanding); |
281 | 287 | } |
282 | 288 |
|
283 | 289 | void WorkerThreadsTaskRunner::PostDelayedTask( |
@@ -574,7 +580,25 @@ void NodePlatform::DrainTasks(Isolate* isolate) { |
574 | 580 | if (!per_isolate) return; |
575 | 581 |
|
576 | 582 | do { |
577 | | - // Worker tasks aren't associated with an Isolate. |
| 583 | + // FIXME(54918): we should not be blocking on the worker tasks on the |
| 584 | + // main thread in one go. Doing so leads to two problems: |
| 585 | + // 1. If any of the worker tasks post another foreground task and wait |
| 586 | + // for it to complete, and that foreground task is posted right after |
| 587 | + // we flush the foreground task queue and before the foreground thread |
| 588 | + // goes into sleep, we'll never be able to wake up to execute that |
| 589 | + // foreground task and in turn the worker task will never complete, and |
| 590 | + // we have a deadlock. |
| 591 | + // 2. Worker tasks can be posted from any thread, not necessarily associated |
| 592 | + // with the current isolate, and we can be blocking on a worker task that |
| 593 | + // is associated with a completely unrelated isolate in the event loop. |
| 594 | + // This is suboptimal. |
| 595 | + // |
| 596 | + // However, not blocking on the worker tasks at all can lead to loss of some |
| 597 | + // critical user-blocking worker tasks e.g. wasm async compilation tasks, |
| 598 | + // which should block the main thread until they are completed, as the |
| 599 | + // documentation suggets. As a compromise, we currently only block on |
| 600 | + // user-blocking tasks to reduce the chance of deadlocks while making sure |
| 601 | + // that criticl user-blocking tasks are not lost. |
578 | 602 | worker_thread_task_runner_->BlockingDrain(); |
579 | 603 | } while (per_isolate->FlushForegroundTasksInternal()); |
580 | 604 | } |
@@ -748,16 +772,22 @@ v8::PageAllocator* NodePlatform::GetPageAllocator() { |
748 | 772 |
|
749 | 773 | template <class T> |
750 | 774 | TaskQueue<T>::TaskQueue() |
751 | | - : lock_(), tasks_available_(), tasks_drained_(), |
752 | | - outstanding_tasks_(0), stopped_(false), task_queue_() { } |
| 775 | + : lock_(), |
| 776 | + tasks_available_(), |
| 777 | + outstanding_tasks_drained_(), |
| 778 | + outstanding_tasks_(0), |
| 779 | + stopped_(false), |
| 780 | + task_queue_() {} |
753 | 781 |
|
754 | 782 | template <class T> |
755 | 783 | TaskQueue<T>::Locked::Locked(TaskQueue* queue) |
756 | 784 | : queue_(queue), lock_(queue->lock_) {} |
757 | 785 |
|
758 | 786 | template <class T> |
759 | | -void TaskQueue<T>::Locked::Push(std::unique_ptr<T> task) { |
760 | | - queue_->outstanding_tasks_++; |
| 787 | +void TaskQueue<T>::Locked::Push(std::unique_ptr<T> task, bool outstanding) { |
| 788 | + if (outstanding) { |
| 789 | + queue_->outstanding_tasks_++; |
| 790 | + } |
761 | 791 | queue_->task_queue_.push(std::move(task)); |
762 | 792 | queue_->tasks_available_.Signal(lock_); |
763 | 793 | } |
@@ -788,16 +818,16 @@ std::unique_ptr<T> TaskQueue<T>::Locked::BlockingPop() { |
788 | 818 | } |
789 | 819 |
|
790 | 820 | template <class T> |
791 | | -void TaskQueue<T>::Locked::NotifyOfCompletion() { |
| 821 | +void TaskQueue<T>::Locked::NotifyOfOutstandingCompletion() { |
792 | 822 | if (--queue_->outstanding_tasks_ == 0) { |
793 | | - queue_->tasks_drained_.Broadcast(lock_); |
| 823 | + queue_->outstanding_tasks_drained_.Broadcast(lock_); |
794 | 824 | } |
795 | 825 | } |
796 | 826 |
|
797 | 827 | template <class T> |
798 | 828 | void TaskQueue<T>::Locked::BlockingDrain() { |
799 | 829 | while (queue_->outstanding_tasks_ > 0) { |
800 | | - queue_->tasks_drained_.Wait(lock_); |
| 830 | + queue_->outstanding_tasks_drained_.Wait(lock_); |
801 | 831 | } |
802 | 832 | } |
803 | 833 |
|
|