Skip to content

Keep wait_for_work from spinning forever on a task that wakes itself - #5865

Open
mstampfli wants to merge 1 commit into
DioxusLabs:mainfrom
mstampfli:fix/wait-for-work-self-waking-task
Open

mstampfli wants to merge 1 commit into
DioxusLabs:mainfrom
mstampfli:fix/wait-for-work-self-waking-task

Conversation

@mstampfli

Copy link
Copy Markdown

Problem

VirtualDom::wait_for_work never returns Pending while a spawned task keeps waking itself, so the executor driving it never runs again. The thread spins at 100% CPU without making a syscall and the UI stops updating for good.

The synchronous drains poll tasks until the queue is empty:

  • poll_tasks: while !self.has_dirty_scopes() { let Some(task) = self.pop_task() ... }
  • drain_remaining_effects: the same loop again for tasks
  • render_immediate_with_writer: while let Some(work) = self.pop_work()

A task that wakes its own waker before returning Pending is queued again by queue_events right after its poll, so pop_task hands the same task back forever. Even with a bounded pass, wait_for_work would go back into wait_for_event next, whose channel has already delivered that task's wakeup, so it would never be woken.

This is easy to hit with tokio. Once a task has used its cooperative budget, the next tokio resource it polls (an interval, a channel, consume_budget) returns Pending and wakes the task. On a thread that is not one of the runtime's workers (for example Runtime::block_on on a multi-thread runtime) that wake is immediate. The budget is only refilled once the executor gets control back, which never happens, so a plain loop { interval.tick().await; ... } task freezes the app. I hit this in a production terminal app built on dioxus: its UI froze while a spinner task sat in exactly this loop.

Fix

  • A synchronous pass over the task queue polls each task at most 32 times (TaskPass in scheduler.rs). A task that comes up again after that is set aside and queued again when the pass ends. poll_tasks, drain_remaining_effects and render_immediate_with_writer all use it.
  • When a pass leaves tasks queued, wait_for_work yields to the executor with the existing yield_now instead of waiting on the channel, then runs the next pass.

Short chains of immediate wakeups still settle within one call, so render_immediate still converges the way nested_suspense_resolves_client expects (a first version that polled each task only once per pass broke that test). The bound of 32 matches the batch size the suspense loops already use before yielding. Setting a task aside instead of ending the pass means a busy task in a parent scope cannot starve tasks in child scopes.

The suspense loops (wait_for_suspense_work, render_suspense_immediate) already yield every 32 items and are unchanged.

Tests

Five tests in packages/core/tests/task.rs. Each drives the VirtualDom on its own thread with a deadline, because the bug is a hang:

  • wait_for_work_yields_between_polls_of_a_self_waking_task: the task keeps being polled while wait_for_work is pending
  • synchronous_drains_return_with_a_self_waking_task: process_events and render_immediate return, and each call polls the task again
  • effect_draining_returns_with_a_self_waking_task: an effect that starts such a task
  • self_waking_task_does_not_starve_a_child_task
  • exhausted_tokio_coop_budget_does_not_freeze_the_virtual_dom: the tokio case above, via tokio::task::coop::consume_budget under Runtime::block_on

The first four fail on current main with "the VirtualDom did not hand control back within 10s". The effect test was added afterwards to cover the effect drain on its own; it hangs the same way when only that drain uses the unbounded pop_task, as it does on main. Each part of the fix is covered: removing the wait_for_work yield, lifting the bound, reverting any one of the three drains to the unbounded pop, or dropping the set-aside tasks instead of queueing them again each makes at least one of these tests fail.

cargo test -p dioxus-core passes (211 passed, 0 failed, 7 ignored). cargo fmt --check, typos and cargo clippy -p dioxus-core --no-deps --tests --all-features --all-targets -- -D warnings are clean. (Without --no-deps, clippy 1.97 stops earlier on two existing redundant reference in format! argument lints in dioxus-core-macro, which this PR does not touch.)

The synchronous drains (poll_tasks, drain_remaining_effects and
render_immediate_with_writer) polled tasks until the queue was empty. A
task that wakes its own waker before returning Pending is queued again
right after its poll, so the same task came back forever and
wait_for_work never returned Pending to its executor. With tokio this
happens to any task looping over a tokio resource once its cooperative
budget runs out off the runtime's worker threads, because the budget is
only refilled when the executor gets control back.

A pass now polls each task at most 32 times (TaskPass) and queues any
task set aside at that bound for the next pass. wait_for_work yields to
the executor with yield_now between passes when tasks remain queued,
instead of waiting on a channel that has already delivered their
wakeups. Short chains of immediate wakeups still settle within one
render_immediate call.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant