[WIP] src: only block on user blocking worker tasks #58047
Conversation
Previously, we block on all worker tasks in the main thread, which can cause a deadlock if the worker task post another foreground task and wait for it to complete. This patch changes it to only block on worker tasks that have kUserBlocking priority to avoid the deadlock. In addition this updates the task queue to be a priority queue and order the task based on priority, so that higher priority tasks get executed first, as the documentation suggests.
nodejs-github-bot
added
c++
|
This seems to fix #54918 - I started two stress test jobs before opening this PR:
And the CI seems to be happy about it: https://ci.nodejs.org/job/node-test-commit/79439/ I am being somewhat lazy here to achieve the "only block on user blocking worker tasks" but only counting them as outstanding tasks. Need another look to make sure I am not throwing other tasks into the queue in the delayed scheduler. Also I am just speculating that worker tasks that would post another foreground task shouldn't be kUserBlocking, or at least I haven't found any so far (the ones that are causing the deadlocks that I can reproduce are just kUserVisible GC tasks). May need to confirm this. |
|
I confirm that this patch fixes the issue on my machine. |
| } else { | ||
| fprintf(stderr, " <no location>\n"); | ||
| } | ||
| // DumpNativeBacktrace(stderr); |
There was a problem hiding this comment.
We probably don't want to keep those
| // DumpNativeBacktrace(stderr); |
src: only block on user blocking worker tasks
Previously, we block on all worker tasks in the main thread,
which can cause a deadlock if the worker task post another
foreground task and wait for it to complete. This patch changes
it to only block on worker tasks that have kUserBlocking priority
to avoid the deadlock.
In addition this updates the task queue to be a priority queue
and order the task based on priority, so that higher priority
tasks get executed first, as the documentation suggests.
Refs: #47452
Fixes: #54918