Fix browser WebView teardown memory leak (#2962)

[austinywang]

Summary

• release portal-hosted browser WKWebViews immediately when a browser panel closes
• remove per-panel WebKit handlers and callbacks during browser panel teardown
• add a regression test covering immediate detach on close

Testing

• Not run locally per repo policy; CI should run the regression.

Closes #2962

---

Note

Medium Risk
Changes WebKit lifecycle and portal teardown on a critical close path; behavior is guarded and covered by new tests but mistakes could cause crashes or lingering WebKit callbacks.

Overview
Fixes a memory leak on browser panel close (#2962) by tearing down the live WKWebView immediately instead of leaving it portal-hosted with delegates and script handlers still attached.

close() is now one-shot (isClosed guard). It runs releaseWebViewForTeardown on the old web view—clear first responder, portal detach/discard, stop loading, nil delegates, strip user scripts, teardownReactGrabMessageHandler / teardownMediaPlaybackMessageHandler, clear CmuxWebView context-menu callbacks, remove from superview—then installClosedPlaceholderWebView (inert replacement, no first responder). Panel state is reset more aggressively (download/loading UI, portal leases, ReactGrab, etc.).

New teardown*MessageHandler helpers unregister WebKit handlers and clear panel-owned handler references (media playback also resets tracking).

Tests assert portal detach and handler removal on close, and that close() is safe if handlers were already torn down.

^{Reviewed by Cursor Bugbot for commit 6dae266. Bugbot is set up for automated code reviews on this repo. Configure here.}

---

Summary by cubic

Fixes a memory leak when closing browser panels by tearing down the portal-hosted WKWebView immediately and installing a non-interactive placeholder. Adds one-shot close guarding and idempotent script handler teardown to avoid late callbacks and leaks. Closes #2962.

• Bug Fixes
  • Release the closed web view immediately: clear first responder, detach/discard portal, stop loading, remove user scripts, tear down ReactGrab/media handlers, nil navigation/UI/download delegates, clear CmuxWebView callbacks, disable first-responder, remove from superview.
  • Make close one-shot with isClosed; install a placeholder; reset state (cancel favicon/loading work, observers/cancellables, delegates, downloads/progress/loading flags, shouldRenderWebView, portal leases/locks, ReactGrab and media playback tracking).
  • Add guarded teardown helpers for script handlers (no-op if already removed) and tests for immediate detach and idempotent teardown.

^{Written for commit 6dae266. Summary will update on new commits.}

Summary by CodeRabbit

• Bug Fixes

  • Browser panel now properly closes and releases resources without interference from stale cleanup operations, improving stability and preventing potential memory issues.

• Tests

  • Added test coverage for browser panel closure behavior.

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
cmux	Ready	Preview, Comment	Jun 6, 2026 11:00am
cmux-staging	Building	Preview, Comment	Jun 6, 2026 11:00am

[coderabbitai]

Looking for one thing? Review this PR in Change Stack to search files, summaries, diffs, and code without losing your place.

📝 Walkthrough

Walkthrough

BrowserPanel.close() is refactored to enforce idempotent one-shot teardown by introducing a private isClosed guard flag and delegating webview teardown to two new private helpers. These helpers fully detach the current WKWebView from portal registry and replace it with an inert placeholder. The refactored close() method also performs expanded state cleanup and calls ReactGrab message handler teardown. A regression test validates immediate detachment of portal-hosted webviews.

Changes

BrowserPanel Teardown Hardening

Layer / File(s)	Summary
Idempotent close guard and teardown helpers Sources/Panels/BrowserPanel.swift	Adds private isClosed flag to prevent repeated teardown, and introduces releaseWebViewForTeardown(_:reason:) and installClosedPlaceholderWebView() helper methods that handle first-responder clearing, portal detach/discard, delegate/script cleanup, and placeholder webview installation.
Refactored close() method with expanded cleanup Sources/Panels/BrowserPanel.swift	Updates close() to guard on isClosed, delegate webview teardown to helpers, and expand state cleanup to cover navigation/UI/download delegates, observers/cancellables, timers/tasks, flags, portal host tracking, and ReactGrab state.
ReactGrab message handler teardown Sources/Panels/ReactGrab.swift	Introduces teardownReactGrabMessageHandler(for:) extension method to remove the registered script message handler and clear the stored handler reference during panel close.
Portal webview detachment regression test cmuxTests/BrowserPanelTests.swift	Adds testCloseDetachesPortalHostedWebViewImmediately test that verifies the original WKWebView is immediately detached from its portal host superview when close() is called.

Possibly related PRs

• manaflow-ai/cmux#4243: Also refactors BrowserPanel.close() flow by marking lifecycle state and refreshing lifecycle metadata during close.
• manaflow-ai/cmux#4244: Also hardens BrowserPanel webview teardown by adding hidden-webview discard and detach/delegate-clearing that runs during close.
• manaflow-ai/cmux#4345: Also refactors BrowserPanel teardown path by adding background-preload host creation and explicit release during panel close.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 A close() once called, now stands its ground,
No teardown ghosts shall come around!
With guards and helpers, crisp and clean,
The WebView's fate is finally seen. ✨

🚥 Pre-merge checks | ✅ 18 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (18 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely summarizes the main change: fixing a browser WebView teardown memory leak, with a reference to the issue number.
Linked Issues check	✅ Passed	The PR directly addresses the memory leak reported in issue #2962 by implementing immediate WKWebView release, portal detachment, handler cleanup, and a regression test.
Out of Scope Changes check	✅ Passed	All changes are directly related to the objective of fixing the WebView teardown memory leak: refactoring close() logic, adding teardown helpers, and including a regression test.
Cmux Swift Actor Isolation	✅ Passed	BrowserPanel changes maintain actor isolation: class is @MainActor, new private helpers properly scoped, teardownReactGrabMessageHandler inherits isolation, tests exempt.
Cmux Swift Blocking Runtime	✅ Passed	No blocking primitives (semaphores, Task.sleep, asyncAfter, main.sync, locks) introduced. All new code synchronous: isClosed guard, teardown helpers, portal cleanup.
Cmux No Hacky Sleeps	✅ Passed	PR modifies only Swift files (.swift). The runtime-no-hacky-sleeps rule explicitly excludes Swift code, which is covered by swift-blocking-runtime.md instead.
Cmux Algorithmic Complexity	✅ Passed	Uses only small UI collections and single-instance cleanup. Responder chain loop bounded to 64 hops. No nested scans or unbounded iterations on scalable collections.
Cmux Swift Concurrency	✅ Passed	New close/teardown methods use only synchronous cleanup with AppKit/WebKit calls; no DispatchQueue, fire-and-forget Task, or Combine app state patterns detected.
Cmux Swift @Concurrent	✅ Passed	All modified Swift methods are synchronous, @MainActor-bound, and perform UI-bound teardown. No async/await or @concurrent annotations added/modified.
Cmux Swift File And Package Boundaries	✅ Passed	Focused bug fix adding 61 net lines to oversized BrowserPanel.swift (well under 250-line threshold) with clear helper method extraction and no mixed responsibilities.
Cmux Swift Logging	✅ Passed	New BrowserPanel teardown methods and close() modifications contain no print, debugPrint, dump, or NSLog statements violating swift-logging.md rules.
Cmux User-Facing Error Privacy	✅ Passed	All changes are internal (private methods/tests) with no user-facing error messages, alerts, vendor names, credentials, or sensitive information exposed to users.
Cmux Full Internationalization	✅ Passed	No user-facing text introduced. Changes are internal teardown refactoring and test assertions—all exempt from i18n requirements per the rules.
Cmux Swiftui State Layout	✅ Passed	PR does not violate SwiftUI state layout rules—modifies existing ObservableObject lifecycle/teardown, adds helper methods with no new state patterns, and includes AppKit-based test.
Cmux Architecture Rethink	✅ Passed	Small correctness fix with clear ownership: isClosed one-shot guard; synchronous releaseWebViewForTeardown clears delegates and portal. No timing repairs, dispatch delays, or split lifecycle.
Cmux Swift Auxiliary Window Close Shortcuts	✅ Passed	PR refactors WebView teardown logic without adding new windows. Pre-existing cmux.browserBackgroundPreload is in IGNORED_IDENTIFIERS.
Cmux Source Artifacts	✅ Passed	PR only changes three hand-written Swift source/test files (BrowserPanel.swift, ReactGrab.swift, BrowserPanelTests.swift) with no artifacts, caches, generated files, or forbidden directories added.
Description check	✅ Passed	The pull request description follows the required template with complete Summary, Testing, and Checklist sections covering the changes, test verification approach, and issue closure.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch issue-2962-high-memory-tahoe

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[greptile-apps]

Greptile Summary

This PR fixes a memory leak when closing BrowserPanel by fully tearing down the live WKWebView — detaching portal hosting, removing all message handlers and user scripts, clearing CmuxWebView callbacks, nilling WebKit delegates, and removing the view from its superview — then installing an inert placeholder so the panel object can outlive close. Previous threads flagged and verified resolution of: downloadDelegate not being nil'd, the isClosed invariant needing a comment, removeScriptMessageHandler crashing when called before bindWebView, and the redundant first-responder resignation pattern.

• releaseWebViewForTeardown performs an ordered, comprehensive cleanup: first-responder resignation, portal detach/discard, stopLoading, delegate nil-ing, user-script removal, ReactGrab and media-playback handler teardown (both guarded on non-nil), CmuxWebView callback clearing, and removeFromSuperview.
• installClosedPlaceholderWebView creates a new inert CmuxWebView (same profile/store, allowsFirstResponderAcquisition = false, fresh webViewInstanceID UUID so any in-flight callbacks are rejected) and assigns it as the panel's webView.
• Tests cover both the portal-hosted immediate-detach invariant and the idempotent double-teardown path.

Confidence Score: 5/5

Safe to merge — teardown ordering is correct, all handler removals are guarded against unbound panels, and the placeholder swap is coherent with the webViewInstanceID invalidation that rejects in-flight callbacks.

The changes are well-scoped to the close/teardown path, previous-thread issues (downloadDelegate leak, isClosed invariant comment, removeScriptMessageHandler crash) have all been addressed in this revision, and the two regression tests directly exercise the portal-detach and idempotent-teardown cases.

No files require special attention.

Important Files Changed

Filename	Overview
Sources/Panels/BrowserPanel.swift	Adds isClosed one-shot guard, releaseWebViewForTeardown helper (portal detach, delegate/callback clearing, removeFromSuperview), and installClosedPlaceholderWebView; close() now does full teardown including downloadDelegate, portal leases, and loading/download state reset.
Sources/Panels/BrowserPanel+MediaPlayback.swift	Adds teardownMediaPlaybackMessageHandler guarded on handler != nil, calling removeScriptMessageHandler with content-world and resetting tracking in both branches.
Sources/Panels/ReactGrab.swift	Adds teardownReactGrabMessageHandler with guard on reactGrabMessageHandler != nil before removeScriptMessageHandler call.
cmuxTests/BrowserPanelTests.swift	Adds two regression tests: portal-hosted webView detach-on-close (superview and media handler both nil after close), and tolerance of pre-torn-down handlers (no crash on double teardown).

Sequence Diagram

sequenceDiagram
    participant Caller
    participant BrowserPanel
    participant releaseWebViewForTeardown
    participant BrowserWindowPortalRegistry
    participant OldWebView as Old WKWebView
    participant Placeholder as Placeholder CmuxWebView

    Caller->>BrowserPanel: close()
    BrowserPanel->>BrowserPanel: "guard !isClosed → isClosed = true"
    BrowserPanel->>BrowserPanel: unfocus() (resign first responder)
    BrowserPanel->>BrowserPanel: "closedWebView = webView"
    BrowserPanel->>releaseWebViewForTeardown: releaseWebViewForTeardown(closedWebView)
    releaseWebViewForTeardown->>OldWebView: window.makeFirstResponder(nil) if still responder
    releaseWebViewForTeardown->>BrowserWindowPortalRegistry: detach(webView)
    releaseWebViewForTeardown->>BrowserWindowPortalRegistry: discard(webView, preserveCurrentSuperview: false)
    releaseWebViewForTeardown->>OldWebView: stopLoading()
    releaseWebViewForTeardown->>OldWebView: "navigationDelegate = nil"
    releaseWebViewForTeardown->>OldWebView: "uiDelegate = nil"
    releaseWebViewForTeardown->>OldWebView: removeAllUserScripts()
    releaseWebViewForTeardown->>OldWebView: teardownReactGrabMessageHandler (guarded)
    releaseWebViewForTeardown->>OldWebView: teardownMediaPlaybackMessageHandler (guarded)
    releaseWebViewForTeardown->>OldWebView: clear CmuxWebView callbacks
    releaseWebViewForTeardown->>OldWebView: removeFromSuperview()
    BrowserPanel->>Placeholder: "makeWebView() → allowsFirstResponderAcquisition = false"
    BrowserPanel->>BrowserPanel: "webViewInstanceID = UUID()"
    BrowserPanel->>BrowserPanel: "webView = Placeholder"
    BrowserPanel->>BrowserPanel: nil navigationDelegate, uiDelegate, downloadDelegate
    BrowserPanel->>BrowserPanel: reset loading/download/portal/ReactGrab state

Loading

_{Reviews (6): Last reviewed commit: "fix: guard browser script handler teardo..." | Re-trigger Greptile}

[austinywang]

Reviewed the latest bot feedback after c7a7521. Greptile's latest item is a safe-to-merge summary with "No files require special attention," so no code change is needed. CodeRabbit's latest item is a review-rate-limit/usage notice rather than actionable code feedback; the CodeRabbit check itself is green.

[cursor]

Cursor Bugbot has reviewed your changes and found 1 potential issue.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit a79a0c3. Configure here.}