Repository navigation
Prevent idle AppKit sidebar layout livelock - #8532
Merged
Merged
Conversation
📝 WalkthroughWalkthroughThe sidebar workspace table now defers and coalesces apply and viewport mutations through a main-actor scheduler. Tests verify deferred updates and that the newest apply snapshot is used. ChangesSidebar workspace table mutation scheduling
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Controller as SidebarWorkspaceTableController
participant Scheduler as SidebarWorkspaceTableMutationScheduler
participant RunLoop as RunLoop.main
participant Table as NSTableView
Controller->>Scheduler: stageApply(...)
Scheduler->>RunLoop: schedule deferred flush
Controller->>Scheduler: stageViewportChange()
RunLoop->>Scheduler: flushPendingMutations()
Scheduler->>Controller: flushApply(...)
Controller->>Table: reconcile rows and table state
Scheduler->>Controller: flushViewportChange()
Controller->>Table: update viewport-dependent state
Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 24 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (24 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
Greptile SummaryThis PR fixes an idle CPU livelock caused by
Confidence Score: 5/5Safe to merge — the fix surgically removes the reentrant mutation boundary without changing any observable behavior for row diffing, height reconciliation, scrolling, hover, or drag. The scheduler is small and well-bounded: it holds at most one staged apply and one viewport flag, clears state before calling callbacks so re-staged mutations during a flush correctly enqueue a new cycle, and uses RunLoop.main.perform(inModes: [.common]) — the AppKit-native deferred scheduling mechanism — rather than a sleep or timer. Actor isolation is correct throughout. Existing tests were correctly updated to await the new deferred boundary, and the new coalescing test covers both the zero-row-before-flush and latest-snapshot-after-flush invariants. No new debug seams were added to production source. No files require special attention. Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant SwiftUI as SwiftUI updateNSView
participant Controller as SidebarWorkspaceTableController
participant Scheduler as SidebarWorkspaceTableMutationScheduler
participant RunLoop as RunLoop.main
participant Table as NSTableView
SwiftUI->>Controller: apply(rows:actions:...)
Controller->>Scheduler: stageApply(input)
Scheduler->>Scheduler: "pendingApply = input"
Scheduler->>RunLoop: perform inModes common
SwiftUI->>Controller: apply(rows:actions:...) coalesced
Controller->>Scheduler: stageApply(input2)
Scheduler->>Scheduler: "pendingApply = input2"
Note over SwiftUI,Scheduler: originating callback returns
RunLoop->>Scheduler: flushPendingMutations
Scheduler->>Scheduler: capture and clear state
Scheduler->>Controller: flushApply(input2)
Controller->>Table: reloadData or moveRow
Table-->>Controller: boundsDidChange
Controller->>Scheduler: stageViewportChange
Scheduler->>RunLoop: new perform cycle
Scheduler->>Controller: flushViewportChange
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant SwiftUI as SwiftUI updateNSView
participant Controller as SidebarWorkspaceTableController
participant Scheduler as SidebarWorkspaceTableMutationScheduler
participant RunLoop as RunLoop.main
participant Table as NSTableView
SwiftUI->>Controller: apply(rows:actions:...)
Controller->>Scheduler: stageApply(input)
Scheduler->>Scheduler: "pendingApply = input"
Scheduler->>RunLoop: perform inModes common
SwiftUI->>Controller: apply(rows:actions:...) coalesced
Controller->>Scheduler: stageApply(input2)
Scheduler->>Scheduler: "pendingApply = input2"
Note over SwiftUI,Scheduler: originating callback returns
RunLoop->>Scheduler: flushPendingMutations
Scheduler->>Scheduler: capture and clear state
Scheduler->>Controller: flushApply(input2)
Controller->>Table: reloadData or moveRow
Table-->>Controller: boundsDidChange
Controller->>Scheduler: stageViewportChange
Scheduler->>RunLoop: new perform cycle
Scheduler->>Controller: flushViewportChange
Reviews (2): Last reviewed commit: "test: await sidebar mutation callback bo..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cmuxTests/SidebarWorkspaceTableTests.swift`:
- Around line 341-344: Replace the fixed 0.02-second deadline in
flushStagedTableMutations with a deterministic completion signal or explicitly
controlled test scheduler. Ensure the method drains staged mutations only after
the deferred callback has completed, without relying on real wall-clock timing
or scheduler load.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: efab10e4-94dd-49b4-b09b-846f8e3ef4ec
📒 Files selected for processing (4)
Sources/Sidebar/AppKitList/SidebarWorkspaceTableApplyInput.swiftSources/Sidebar/AppKitList/SidebarWorkspaceTableMutationScheduler.swiftcmux.xcodeproj/project.pbxprojcmuxTests/SidebarWorkspaceTableTests.swift
💤 Files with no reviewable changes (1)
- Sources/Sidebar/AppKitList/SidebarWorkspaceTableMutationScheduler.swift
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
1 participant
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.
Closes #8525
Root cause
v0.64.20 shipped the native AppKit workspace table from #8270 (
b7bd90103c) and made it the fallback/default in #8433 (e35b74407a). That exposed the new bridge to users who had remained on the SwiftUI sidebar in v0.64.19.SidebarWorkspaceTableView.updateNSViewsynchronously called the controller'sapply. That path can reload/move rows and reconcile row heights. Those table mutations emit bounds/layout callbacks while the originating SwiftUI representable update is still resolving; the callbacks can synchronously perform more viewport and row-height work, producing a SwiftUI/AppKit layout transaction that does not converge while the app is otherwise idle.Fix
SidebarWorkspaceTableMutationScheduler.NSTableViewinside representable/layout callbacks.This removes the reentrant mutation edge for the whole table controller instead of throttling one symptom.
Regression proof
The test and fix are separate commits:
edd08429affails because the table already has two rows before the callback boundary: https://github.com/manaflow-ai/cmux/actions/runs/297796902226400ffaf51introduces the deferred/coalesced mutation boundary.10c6df9b48has both scoped tests passing in a 12-test execution: https://github.com/manaflow-ai/cmux/actions/runs/29781424661The final suite's two other failures are stale assertions already contradicted by current implementation:
.fullWidthversus.plain, and an old-width cache value expected to be removed. Neither covers the changed scheduling path.The exact tagged app build succeeded: https://github.com/manaflow-ai/cmux/actions/runs/29782552764. Its reload-build commit has final PR HEAD as its sole parent and only adds a provisioning log; the app debug dylib SHA-256 is
c99674c11e6cbd7fe5b880adc68e167ccb50b233cdd77fccc140fa3c5f075fa4.Runtime verification
Normal idle on macOS 26.5.1, 10 continuous minutes after settle:
PONG.reloadDataor row-height samples.Normal idle on macOS 15.7.4, 10 continuous minutes after settle:
Restored scale on macOS 26.5.1:
reloadData, row-height, or layout-subtree frames.PONG, topology remained 128/250/250, and no crash/hang/DiagnosticReport or reentrant/layout-loop warning appeared.Artifacts and raw metrics are under
artifacts/issue-8525-idle-high-cpu-crash/in the verification worktree.Verification limitations
launchddomain (OSLaunchdErrorDomain 125), so the tagged app could not create a PID/socket, and its VNC endpoint only offered unsupported Apple-auth security modes. The successful macOS 15.7.4 normal 10-minute run remains valid; no scaled result is claimed for that OS.sentry-cliis present locally but has no authenticated session.Static and localization checks
Package.resolvedpolicy, andgit diff --checkpassed.