Skip to content

macOS: free surface synchronously in deinit on main thread - #13364

Merged
mitchellh merged 1 commit into
ghostty-org:mainfrom
bo2themax:push-2e928e5b
Jul 26, 2026
Merged

macOS: free surface synchronously in deinit on main thread#13364
mitchellh merged 1 commit into
ghostty-org:mainfrom
bo2themax:push-2e928e5b

Conversation

@bo2themax

@bo2themax bo2themax commented Jul 17, 2026

Copy link
Copy Markdown
Member

Since the renderer thread now emits scrollbar events on almost every frame, there's always a .scrollbar message for the dying surface in the app mailbox.

The OS runtime seems to schedule appTick and ghostty_surface_free differently across macOS pre-26, 26 and 27.

On macOS 26.x, ghostty_app_free happens after App.scrollbar(_:target:v:), leaving surface.userdata pointing at a freed SurfaceView.

When deinit runs on the main thread, free the surface synchronously instead of detaching to a task. This fixes both crashes mentioned in #9512 and #13359.

AI Disclosure

I used Claude to analyze the backtrace, but the code is written and tested by myself.

@bo2themax
bo2themax requested a review from a team as a code owner July 17, 2026 10:12
@ghostty-bot ghostty-bot Bot added the os/macos label Jul 17, 2026
@jparise

jparise commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Seems related to #9760, too.

I'm curious if the approach here covers all of the complications I discovered while working on that one.

@mitchellh mitchellh left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not confident in this change. If I'm understanding properly, the problem is sometimes ghostty_app_free happens BEFORE all the surface frees? That's fundamentally it? If so, this doesn't seem right. If not, I don't understand what this is doing. It doesn't seem like a surface should have any business freeing the entire app (which owns N surfaces).

I need more info.

@bo2themax

bo2themax commented Jul 17, 2026

Copy link
Copy Markdown
Member Author

I'm not confident in this change. If I'm understanding properly, the problem is sometimes ghostty_app_free happens BEFORE all the surface frees? That's fundamentally it? If so, this doesn't seem right. If not, I don't understand what this is doing. It doesn't seem like a surface should have any business freeing the entire app (which owns N surfaces).

I need more info.

Sh*t, you're right, I used ghostty_surface_free in VM, but typed it wrong🫪, and still pasted in every comment...

And for the context for undo-timeout=0 here are the sequence difference between 26.5.1 and 27 Beta 3 (using 0784f6b)

I assume the second scrollbar_dirty => .scrollbar arrives early when draining the mailbox.

image image

For the previous undo crash mentioned in #9512, I reproduced it again the backtrace is exactly the same as undo-timeout=0, so I reverted it.

// Since the renderer thread now emits scrollbar events
// on almost every frame, there's always a `.scrollbar` message
// for the dying surface in the app mailbox.
//

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even if this ends up being the right approach, I don't think we should frame the rationale like this. This is a point-in-time description of the current state rather than a lifecycle contract.

That's a (the?) reason why #9760 is as involved as it is: it avoids making assumptions about the mailbox, etc. and instead makes the contract "signal and wait for the apprt to finish" before we finalize the termination of the (macOS) app itself.

I acknowledge that the issue you're going after here (#13359, #13359) is in the tab+surface close path rather than the whole app termination path, but there are similarities.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make sense to me! Do you have a suggestion how should we phrase this? I don't think I have better understanding of the whole lifecycle including the renderer and the mailbox🤔

@jparise jparise left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought about it a bit more, and I think this approach will work and address the root issue.

I don't really like how it solidifies some assumptions around the macOS app's threads and lifecycle alongside the embedded apprt's though.

Comment thread macos/Sources/Features/Terminal/TerminalController.swift Outdated
Comment thread macos/Sources/Ghostty/Ghostty.Surface.swift Outdated
@mitchellh
mitchellh merged commit 1eecfe0 into ghostty-org:main Jul 26, 2026
97 checks passed
@github-actions github-actions Bot added this to the 1.4.0 milestone Jul 26, 2026
mitchellh added a commit that referenced this pull request Jul 26, 2026
@bo2themax
bo2themax deleted the push-2e928e5b branch July 27, 2026 05:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants