fix(macos): run GatewayConnectionManager health check loop off @MainActor#26082
Conversation
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
![vex-assistant-bot[bot]](https://avatars.githubusercontent.com/in/3399303?s=60&v=4){kind=small}
There was a problem hiding this comment.
✦ Vex Review — PR #26082 (LUM-916)
Good approach — moving the 15s health check loop to Task.detached(.utility) with explicit MainActor.run hops for state reads (isUpdateInProgress, shouldReconnect) is clean and correct. The guard !Task.isCancelled after sleep is a good addition.
Same two concerns as PR #26081:
-
Overlap with #26081 (LUM-914): That PR offloads the inner
performHealthCheck()work. If both merge, you get nested detached tasks. I'd suggest either: (a) merge #26082 first (outer loop off-main), then #26081 becomes unnecessary sinceperformHealthCheck()is already called from a non-main context, or (b) merge #26081 first and skip #26082. The outer loop'sTask.sleepisn't expensive enough on@MainActorto justify detaching by itself — it's the HTTP work inside that causes hangs. -
Rebase needed: PR #25496 (merged today) migrated this class to
@Observable. Both PRs should rebase to verify no semantic conflicts.
Recommendation: If you want both fixes, merge #26082 first (broader fix), then evaluate if #26081's inner detach is still needed.
The 15-second health check loop previously ran as a Task { @mainactor ... }
so the while-loop, Task.sleep scheduling, and error handling all occupied
the main actor. Under memory pressure this periodic work contributes to
app hangs (LUM-914 recorded a ≥2000ms hang attributed to this loop on a
device with ~131MB free RAM).
Change the loop to a detached task at .utility priority and hop to
@mainactor only for the state reads (isUpdateInProgress,
healthCheckInterval, shouldReconnect) and the update-timeout cleanup
that mutates @published properties. performHealthCheck() itself stays
@mainactor so all @published writes and cached-assistant reads remain on
main — no new data races, no changes to resolveConnection() or the rest
of the network stack that triggered the earlier revert of #21695.
Refs: WWDC25 Embracing Swift concurrency
(https://developer.apple.com/videos/play/wwdc2025/268/),
Task.detached(priority:), MainActor.run.
Closes LUM-916
Co-Authored-By: tkheyfets <timur@vellum.ai>
Co-Authored-By: tkheyfets <timur@vellum.ai>
devin-ai-integration
Bot
force-pushed
the
devin/lum-916-move-gatewayconnectionmanager-health-check-off-mainactor-to
branch
from
b747fa1 to
7b4cfad
Compare
|
Rebased on On the overlap with #26081: leaving coordination to the author of that PR, but agree that once this one merges the inner detach becomes redundant. |
tkheyfets
deleted the
devin/lum-916-move-gatewayconnectionmanager-health-check-off-mainactor-to
branch
1 participant
Moves the 15 s health check loop to a detached
.utilitytask so itsTask.sleepscheduling and between-check work no longer occupy@MainActor; onlyperformHealthCheck()and the update-timeout cleanup (which mutate@Publishedstate) hop back to main viaMainActor.run. This removes periodic main-actor pressure that was contributing to app hangs under memory pressure without changingresolveConnection()or the rest of the network stack that triggered the earlier revert of #21695.Closes LUM-916