Await asyncs of components deleted before graceful shutdown - #4401
Open
rdlu wants to merge 2 commits into
Open
Conversation
A LiveComponent that leaves the tree while one of its start_async tasks is still running has its task killed mid-flight on a graceful shutdown, instead of being awaited like the asyncs of the LiveView and of live components.
Diff.delete_component/2 drops a component from the state without touching the async tasks it started, which made those tasks invisible to await_asyncs/1 on a graceful shutdown: instead of being awaited, they were killed mid-flight by their link to the channel process. Keep tracking the pids of a removed component's asyncs in the channel state and monitor them, so all_asyncs/1 still sees them and the entries are dropped once the tasks are done.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #4400.
When components are deleted (
delete_components/2inchannel.ex, i.e. on the client'scids_destroyedack), their in-flight async pids are collected via the existingsocket_asyncs/2into a neworphaned_asyncschannel-state map and monitored; a dedicatedhandle_info:DOWNclause prunes entries (and keeps these monitor messages out of userhandle_info/2).all_asyncs/1merges the orphans, so graceful shutdown (await_asyncs/1) awaits them andLiveViewTest.render_async/2waits for them too.Notes for review:
write_component→:error→push_noop); only the shutdown/await bookkeeping changes.render_async/2now also waits for asyncs of removed components (they shareasync_pids). We think that's a fix — previously a test could return while such a task was still writing to the DB — but it is an observable change; happy to narrow it toterminate/2only if preferred.Process.monitor/1is needed for pruning because the monitor set up byAsync.run_async_task/5usesalias: :reply_demonitorand emits no:DOWNon successful completion.test/phoenix_live_view/integrations/start_async_test.exs, using the[:phoenix, :live_component, :destroyed]telemetry event to deterministically wait for the actualdelete_component(the client ackscids_destroyedasynchronously). Verified: new tests red on unpatched main, full suite green after the fix, and the single-file repro from the issue flips exactly as expected against the patched tree.