Repository navigation
Let go of the scene manager's seals once they have been used - #6100
Merged
bgoncal merged 2 commits intoOct 22, 2026
Merged
Conversation
The promise for the web view controller, and the one for the app coordinator, are made pending at launch and sealed by the first window. The seal's closure holds the promise's box, and the box holds what it was sealed with, so keeping the closure kept the first controller of the process alive for as long as the app ran, once a reset or a server switch had replaced it. On a Mac that was a web view nobody could see, frontend loaded, websocket open, about a percent of a core. Both seals are now replaced by a no-op the moment they are used.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The narrow ownership fix preserves promise behavior and includes regression coverage for both retention paths; tests were not executed here.
No open issues 🎉
What changed in this PR
Fixes a SceneManager retention issue that kept the first frontend controller and coordinator alive after replacement.
Changes:
- Releases both promise seal closures after their first use.
- Adds regression tests verifying replaced objects are deallocated.
| File | Description |
|---|---|
| Tests/App/Scenes/SceneManagerPromise.test.swift | Tests controller and coordinator release after replacement. |
| Sources/App/Scenes/SceneManager.swift | Replaces used seals with no-op closures to release retained values. |
Review effort: Balanced
This was referenced Oct 21, 2026
Merged
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6100 +/- ##
=======================================
Coverage 56.37% 56.38%
=======================================
Files 1256 1256
Lines 83511 83513 +2
=======================================
+ Hits 47078 47085 +7
+ Misses 36433 36428 -5
☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
bgoncal
approved these changes
Oct 22, 2026
bgoncal
pushed a commit
that referenced
this pull request
Oct 22, 2026
This is a big one for performance! Even though the interface is not visible and nothing is actively working, the entire web interface is still being rendered constantly. That's a huge waste of resources not just in the app but for the server as well. This fixes that issue. This one was sensitive enough that I had AI do the research and modified most of the code myself. Only the tests were fully autogenerated. I think this should make a big leap in CPU usage for the app. ## AI Policy - [X] I have read the [Open Home Foundation AI Policy](https://contributors.home-assistant.io/start/ai-policy/). Select exactly one option that describes AI usage in this contribution: - [ ] I have not used AI for this contribution. - [X] AI assistance was used for this contribution. - [ ] AI fully generated the code for this contribution, but I've reviewed and understood it before submitting and will respond without AI during review. ## Summary Reset the frontend once, or switch servers, and SwiftUI throws away the `WebViewController` it was hosting. That should be the end of it. It isn't: on my Mac the discarded controller kept running the whole frontend off screen, websocket open, every state change processed, in its own `WebContent` process at about a percent of a core. Two of them, next to the one on screen, and every connect made all three run `loadActiveURLIfNeeded` (three `loadActiveURLIfNeeded called` lines per connect in the log). One of the two is held by a `SceneManager` promise seal, fixed in #6100. The other is held by something `leaks` couldn't pin down for me. I'd rather not depend on finding every retainer in a codebase that hands the controller to the scene manager, the Mac title bar and the notification manager, so this PR makes a dismantled controller harmless whoever holds it. A dismantled controller now retires. `FrontendView.dismantleUIViewController` calls `retireFrontend()`, which loads `about:blank` the way logging out already does and refuses every load from then on. The connect and activation notifications still reach every controller alive, retired ones included, but they can't bring the server back into this one. It also lets go of the overlay state and reconnect manager it shares with its replacement through the view model, so it can't write into the replacement's screen. ## Change - `WebViewController+Retirement.swift` (new): `retireFrontend()`. Cancels the active URL attempt in flight (past the guards it would still run the frontend's reload script once it wakes up), lets go of the shared overlay state and reconnect manager, cancels the onscreen content publish and resigns and drops the user activity (the blank page's title, or a publish landing late, would otherwise make it current over the replacement's), removes its notification and server observers, and loads the blank page. - `WebViewController.swift`: `isRetired`. - `WebViewController+URLLoading.swift`: `loadActiveURLIfNeeded` returns early when retired, before it refreshes the network information or touches the asset cache. The continuation after the asynchronous cache check re-checks, next to the log-out check it already had, for a retirement that landed while the check was pending. - `WebViewController+ProtocolConformance.swift` and `WebViewController+Navigation.swift`: `load(request:)` and `reload()` refuse when retired. Every way back into the page goes through one of the two: deep links, the kiosk dashboard, a reload after a server version change, `refresh()`, the kiosk auto-reload timer. `goBack()` and `goForward()` refuse as well, the blank page keeps the frontend in the history. - `WebViewController+BlankFrontend.swift`: a content process dying behind a retired frontend is left alone. Without this the second termination would clean the shared asset cache and reload, for a page nobody can see. - `FrontendView.swift`: `dismantleUIViewController`. - `HomeAssistantViewModel.swift`: `resetWebFrontend()` retires the controller it drops, and so does `deinit`, for the server switch and the closing window. The App Labs tab bar's slot hosts that one without a `FrontendView`, so nothing else would. The three standby-loader tasks hold the model weakly now: an implicit `self` before their `guard let self` captured it strongly, `[weak self]` or not, and kept the model and its controller alive past their owner for the length of the sleep. That leak is also what had the minimum-duration lines covered until now, the test models lived long enough for the sleep to run out. `loaderMinimumDuration` is overridable like `loaderWatchdogTimeout`, so a test can wait it out for real. ## Tests `Tests/App/WebView/WebViewControllerRetirementTests.swift` (new, in the synchronized `Tests/App/WebView` folder). A `WKWebView` subclass records what it is asked to load instead of loading it, so a refused load is a fact rather than a page that happened not to arrive in time. - `testDismantlingAFrontendViewRetiresItsController` - `testResettingTheFrontendRetiresTheControllerItDrops` - `testDroppingTheViewModelRetiresTheControllerItStillHolds`: the model goes the moment its owner drops it, and takes the controller down. - `testRetiringTakesThePageDownAndKeepsItDown`: one `about:blank` load, then `load(request:)`, `reload()`, `goBack()`, `goForward()` and a second retire change nothing. - `testARetiredControllerDoesNotLoadTheActiveURL`: no cache check, no task. - `testRetiringWhileTheCacheCheckIsPendingStartsNoLoad`: the handler holds its completion, the controller retires, the completion lands, no task. - `testRetiringCancelsAnActiveURLAttemptInFlight` and `testRetiringCancelsAnOnscreenContentPublishInFlight`. `Tests/App/WebView/WebViewControllerBlankFrontendTests.swift`: `testATerminatedContentProcessIsLeftAloneBehindARetiredFrontend`, two terminations behind a retired frontend count nothing and clean nothing. `Tests/App/WebView/HomeAssistantViewTests.swift`: `testTheLoaderComesDownOnItsOwnOnceItsMinimumDurationHasElapsed`, a `loaded` report before the minimum duration leaves the loader up, and it comes down on its own once the duration has elapsed. - `testRetiringLetsGoOfWhatItSharesWithItsReplacement`: overlay state, reconnect manager and user activity are nil. ## Any other notes One of a series from a CPU investigation of the macOS app. Pairs with "Let go of the scene manager's seals once they have been used", which removes the one retainer of a dismantled controller I could pin down. This one makes any retained controller harmless.
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.
Just a small bit of cleanup. This webview controller is created but never cleaned up so it's constantly using CPU and memory while nothing is using it.
Trivial fix but could make a big difference for lower end machines.
AI Policy
Select exactly one option that describes AI usage in this contribution:
Summary
SceneManagermakes two promises pending at launch, one for theWebViewControllerand one for theAppCoordinator, and stores their seal closures. The first window seals them; every later window replaces the promise with.value(...). The seal closure is never let go of, and it holds the box of the promise it sealed, which holds the value it was sealed with. So the firstWebViewControllerof the process stays alive for as long as the app runs, once a reset or a server switch has replaced it.On the Mac I measured, that was a
WebViewControllerwith no SwiftUI host, its web view still running the full frontend in aWebContentprocess at about a percent of a core, websocket open, reloaded on every connect.leaks --traceon the running app gave the path:SceneManager.webViewControllerSealclosure context, capturedGuaranteebox,sealant.resolved, the controller.Both seals are now replaced by a no-op the moment they have been used.
Tests
Tests/App/Scenes/SceneManagerPromise.test.swift(new;Tests/App/Scenesis a synchronized folder):testTheFirstControllerIsReleasedOnceAnotherReplacesIt: a weak reference to the first controller is nil after a second one replaces it. Fails onmain.testTheFirstCoordinatorIsReleasedOnceAnotherReplacesIt: the same for the coordinator seal.Screenshots
Not a user-facing change.
Link to pull request in Documentation repository
Documentation: home-assistant/companion.home-assistant#
Any other notes
One of a series from a CPU investigation of the macOS app. Pairs with "Take a frontend down once SwiftUI has dismantled its controller": that one stops a dismantled frontend from running, this one stops the first one from being kept alive at all.