Fix use-after-free in ghostty_surface_refresh after sleep/wake

[lawrencecchen]

Summary

• Add nil guard in forceRefresh() to prevent dereferencing freed surface pointer after forceRefreshSurface() triggers layout that frees the surface
• Split else if chains in Workspace.swift into separate if statements so requestBackgroundSurfaceStartIfNeeded() runs when the surface is freed during refresh (recovery path)
• Add releaseSurfaceForTesting() helper (#if DEBUG) and regression test

Closes #432

Test plan

• Sleep 30s, wake, verify no crash and terminals redraw
• Sleep 5min, wake, verify no crash
• Close lid, reopen, verify no crash
• Run unit tests: xcodebuild test -only-testing:cmuxTests

Summary by CodeRabbit

• Bug Fixes

  • Improved surface refresh handling to ensure correct references are maintained during terminal topology changes, preventing potential interactions with freed surfaces.
  • Enhanced background surface startup logic for more consistent behavior.

• Tests

  • Added regression test to verify refresh behavior maintains stability when runtime surfaces are released.

[vercel]

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

Project	Deployment	Actions	Updated (UTC)
cmux	Building	Preview, Comment	Feb 27, 2026 7:20am

[coderabbitai]

📝 Walkthrough

Walkthrough

This pull request fixes a use-after-free crash in forceRefresh() by guarding against nil surfaces, decouples background surface startup logic in Workspace.swift, and adds a regression test to verify the fix prevents further crashes during geometry reconciliation after surface release.

Changes

Cohort / File(s)	Summary
Terminal Surface Safety Sources/GhosttyTerminalView.swift	Added nil guard in forceRefresh() using locally captured currentSurface to prevent use-after-free crashes; introduced DEBUG-only logging for refresh events; added releaseSurfaceForTesting() helper methods for test-driven surface release simulation.
Background Surface Logic Sources/Workspace.swift	Decoupled background surface startup from the refresh path by separating conditional checks; ensures surface initialization occurs independently when nil, removing prior dependency on preceding branches.
Regression Test cmuxTests/CmuxWebViewKeyEquivalentTests.swift	Added DEBUG-gated test testForceRefreshNoopsAfterSurfaceReleaseDuringGeometryReconcile verifying that forceRefresh() safely handles already-released surfaces during geometry reconciliation.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Poem

🐰 A surface once freed was a crash-prone snare,
But nil guards and logic now settle with care,
From geometry's wake, no more memory strife,
The refresh stands guard o'er the surface's life! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

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

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the primary fix: adding a nil guard in forceRefresh() to prevent use-after-free crash after sleep/wake events.
Linked Issues check	✅ Passed	All coding objectives from issue #432 are met: nil guard added in forceRefresh(), control flow adjusted in Workspace.swift for recovery logic, DEBUG test helper and regression test provided.
Out of Scope Changes check	✅ Passed	All changes directly address issue #432 with no unrelated modifications: forceRefresh nil guard, Workspace.swift control flow fix, test infrastructure, and regression test.

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

✨ Finishing Touches

• 📝 Generate docstrings (stacked PR)
• 📝 Generate docstrings (commit on current branch)

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch issue-432-surface-use-after-free

---

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

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

Sources/GhosttyTerminalView.swift (1)

2020-2031: ⚠️ Potential issue | 🟡 Minor

DEBUG logging should use dlog() instead of direct file writes.

The coding guidelines specify that all debug events must go to a unified log using the dlog() free function. This refresh debug logging writes directly to /tmp/cmux-refresh-debug.log instead of using dlog(). As per coding guidelines, all dlog() call sites must be wrapped in #if DEBUG / #endif conditional compilation blocks.

♻️ Refactor to use dlog()

         `#if` DEBUG
-        let ts = ISO8601DateFormatter().string(from: Date())
-        let line = "[\(ts)] forceRefresh: \(id) \(viewState)\n"
-        let logPath = "/tmp/cmux-refresh-debug.log"
-        if let handle = FileHandle(forWritingAtPath: logPath) {
-            handle.seekToEndOfFile()
-            handle.write(line.data(using: .utf8)!)
-            handle.closeFile()
-        } else {
-            FileManager.default.createFile(atPath: logPath, contents: line.data(using: .utf8))
-        }
+        dlog("surface.forceRefresh surface=\(id.uuidString.prefix(5)) \(viewState)")
         `#endif`

As per coding guidelines: "All debug events (keys, mouse, focus, splits, tabs) must go to a unified log in DEBUG builds using the dlog() free function. All dlog() call sites must be wrapped in #if DEBUG / #endif conditional compilation blocks."

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@Sources/GhosttyTerminalView.swift` around lines 2020 - 2031, Replace the
manual file-write debug block with a call to the dlog() free function inside the
existing `#if` DEBUG/#endif: construct the same log string (including ISO8601
timestamp, id and viewState) and call dlog("[\(ts)] forceRefresh: \(id)
\(viewState)"); remove the FileHandle/FileManager create/write/close code;
ensure the timestamp generation (ISO8601DateFormatter) and message formatting
remain the same so the output matches the previous content.

🧹 Nitpick comments (1)

Sources/Workspace.swift (1)

3142-3147: Consider matching restart gating with the main reconcile path.

Non-blocking: to avoid unnecessary restart attempts during transient detach/reparent, consider gating this restart with attachment/usable-bounds checks (same pattern as Line 3087).

Proposed consistency patch

                 panel.hostedView.reconcileGeometryNow()
                 if panel.surface.surface != nil {
                     panel.surface.forceRefresh()
                 }
-                if panel.surface.surface == nil {
+                let isAttached = panel.hostedView.window != nil && panel.hostedView.superview != nil
+                let hasUsableBounds = panel.hostedView.bounds.width > 1 && panel.hostedView.bounds.height > 1
+                if panel.surface.surface == nil, isAttached, hasUsableBounds {
                     panel.surface.requestBackgroundSurfaceStartIfNeeded()
                 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@Sources/Workspace.swift` around lines 3142 - 3147, The restart attempt
(panel.surface.requestBackgroundSurfaceStartIfNeeded()) should be gated the same
way the main reconcile path gates restarts to avoid transient detach/reparent
churn: wrap the requestBackgroundSurfaceStartIfNeeded() call in the same
attachment and usable-bounds condition used in the reconcile block (i.e., copy
the attachment/usableBounds check from the reconcile code path) so you only call
requestBackgroundSurfaceStartIfNeeded() when the panel is attached and has
usable bounds; leave panel.surface.forceRefresh() behavior unchanged.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@Sources/GhosttyTerminalView.swift`:
- Around line 2020-2031: Replace the manual file-write debug block with a call
to the dlog() free function inside the existing `#if` DEBUG/#endif: construct the
same log string (including ISO8601 timestamp, id and viewState) and call
dlog("[\(ts)] forceRefresh: \(id) \(viewState)"); remove the
FileHandle/FileManager create/write/close code; ensure the timestamp generation
(ISO8601DateFormatter) and message formatting remain the same so the output
matches the previous content.

---

Nitpick comments:
In `@Sources/Workspace.swift`:
- Around line 3142-3147: The restart attempt
(panel.surface.requestBackgroundSurfaceStartIfNeeded()) should be gated the same
way the main reconcile path gates restarts to avoid transient detach/reparent
churn: wrap the requestBackgroundSurfaceStartIfNeeded() call in the same
attachment and usable-bounds condition used in the reconcile block (i.e., copy
the attachment/usableBounds check from the reconcile code path) so you only call
requestBackgroundSurfaceStartIfNeeded() when the panel is attached and has
usable bounds; leave panel.surface.forceRefresh() behavior unchanged.

---

ℹ️ Review info

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e20d692 and d448b2a.

📒 Files selected for processing (3)

• Sources/GhosttyTerminalView.swift
• Sources/Workspace.swift
• cmuxTests/CmuxWebViewKeyEquivalentTests.swift

[greptile-apps]

Greptile Summary

Fixes a use-after-free crash during sleep/wake by adding a nil guard in forceRefresh() that captures the surface pointer before view.forceRefreshSurface() triggers AppKit layout changes that can free the surface. The Workspace changes ensure the recovery path (requestBackgroundSurfaceStartIfNeeded()) runs independently even when the surface is freed during refresh, and includes a regression test to prevent future occurrences.

• Captured surface pointer as currentSurface before potentially-freeing operations in GhosttyTerminalView.swift:2038
• Changed else if to separate if statements in Workspace.swift so recovery path executes when surface is nil
• Added releaseSurfaceForTesting() debug helper and regression test to verify graceful handling of freed surfaces

Confidence Score: 5/5

• Safe to merge - this PR fixes a critical use-after-free crash with minimal, well-tested changes
• The fix correctly addresses the use-after-free by capturing the surface pointer before potentially-freeing operations and re-checking after. The recovery path changes ensure proper handling when the surface is freed, and the regression test provides coverage
• No files require special attention

Important Files Changed

Filename	Overview
Sources/GhosttyTerminalView.swift	Adds nil guard in forceRefresh() to prevent use-after-free and test helper for regression testing
Sources/Workspace.swift	Splits else if chains into separate if statements to ensure recovery path runs when surface is freed
cmuxTests/CmuxWebViewKeyEquivalentTests.swift	Adds regression test verifying forceRefresh() handles freed surface without crashing

_{Last reviewed commit: d448b2a}