renderer: an off-main present never commits while it holds the Core Animation lock (embedder-mode deadlock from #70)

Merged
LA
2d ago
+124-31

Summary

  • Fixes a deadlock that #70 added. In GHOSTTY_HOST_PRESENT_EMBEDDER mode, IOSurfaceLayer.setSurfaceOffMain (the GPU completion thread) took [CATransaction lock] before begin. Zig runs defers last-first, so the thread committed its transaction while it held the lock.
  • cmux frame owner refactor S4 films, 2026-11-03, with the S4 switch on and the film recorder's load: main hung in CA::Transaction::commit > commit_transaction, waiting for a lock. The Metal completion thread hung in bufferCompleted > setSurfaceOffMain, in its commit, while it held the CA lock. The renderer threads waited in SwapChain.nextFrame. The sample without the recorder did not hang. The default mode (GHOSTTY_HOST_PRESENT_RENDERER) never hung under the same load: it already did begin, lock, checks, unlock, commit.
  • Fix: both modes use that order. The embedder-mode gate stays: under the lock, a frame at the shown size goes off main only when the embedder reported its last change committed (main_pending clear). Otherwise it goes through main (setSurface).
  • Invariant (comment in setSurfaceOffMain, ghostty.h and NEXT.md): no thread commits a Core Animation transaction, or flushes, while it holds [CATransaction lock].
  • ghostty.h call-site rule for ghostty_surface_host_frame(present: true): present and set the bounds in one lock span, or in one Core Animation layout or display pass of the commit. An open transaction alone is not enough. Unlock before any commit or flush. Never call ghostty_surface_draw under the lock.
  • Lock order after the fix: the completion thread takes fresh.mutex in publishFresh and releases it before it takes the CA lock. Under the CA lock it does only the shown-size and sequence checks and the contents set. Main takes the CA lock, then fresh.mutex (takeFresh). fresh.mutex is a leaf on both threads, so no cycle. ghostty_surface_host_frame never waits. Main does wait for a free frame, and so for the completion thread, in a first-frame display pass and in ghostty_surface_draw, as before this PR (ghostty.h says so). cmux on macOS never calls ghostty_surface_draw.
  • The old order stopped an off-main commit from shipping a change that main made after the checks. Now main can run a lock span between the unlock and the off-main commit. cmux relies on Core Animation to encode a commit under the CA lock, so that a commit sees all of an embedder lock span or none of it. Main's sample shows commit_transaction waiting for that lock, but one frame does not prove that it holds the lock for all of its layer work. The S4 films judge it (two terminals in one span, with echo in a third). The gate (main_pending) and the mode are still read under the lock, so the checks for this surface stay correct.

Testing

  • New test: host present: an off-main present never commits while it holds the Core Animation lock (embedder), and the same for (renderer) as a control. It replaces +[CATransaction lock], +unlock and +commit with counting versions on the metaclass. It runs one setSurfaceOffMain on a spawned thread and expects one commit with no commit under the lock. It uses no GPU and no timing.
  • Red: test commit b0c7e291a, run https://github.com/manaflow-ai/ghostty-next/actions/runs/41694823112 on scratch/off-main-lock-red-2, which only narrows the filter to host present: . The embedder test fails with expected 0, found 1. The renderer control passes (102 of 103).
  • Green: fix 6a7472263, run https://github.com/manaflow-ai/ghostty-next/actions/runs/41694827381 on scratch/off-main-lock-green-2: ghostty-test 241/241, lib-vt 475/475.
  • 4338c404e and 3163f4b73 change only comments, ghostty.h and NEXT.md.
  • The workflow's test filter list does not change in this PR. The scratch branches are never merged.

Observed / Not run

  • Reviews at 3163f4b73: Codex SHIP and Claude SHIP (comments on this PR). The header text fix from the Claude review goes in a follow-up fork PR.
  • Observed: the red and green runs above, and the stacks from the cmux S4 sample (cmux STATUS 9732).
  • Not run yet: the cmux display test TerminalFrameOwnerMainThreadTests (main must keep turning under the recorder load) on a cmux build pinned to this fix. It goes in the cmux pin PR (#33956), with its red step on the old pin.

Changelog

none