Skip to content

fix(host): free Ghostty surfaces on terminal teardown - #138

Merged
bvolpato merged 2 commits into
am-will:mainfrom
bvolpato:bvolpato/free-terminal-surfaces
Aug 9, 2026
Merged

fix(host): free Ghostty surfaces on terminal teardown#138
bvolpato merged 2 commits into
am-will:mainfrom
bvolpato:bvolpato/free-terminal-surfaces

Conversation

@bvolpato

@bvolpato bvolpato commented Aug 6, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • shut down Ghostty surfaces explicitly when terminal tabs, panes, workspaces, or app close
  • drain pane tab ownership and break GTK callback, signal, and controller reference cycles
  • keep callback widgets and clipboard userdata alive until Ghostty worker shutdown completes
  • prevent retired terminal widgets from creating replacement surfaces during late realize events

Fixes #137.

Root cause

SURFACE_MAP and terminal callbacks retained GTK pane state, while cleanup depended on widget destruction. Closed panes and workspaces could therefore keep terminal roots reachable, preventing ghostty_surface_free from running and leaving PTY shells plus three Ghostty threads per surface alive.

Testing

  • ./scripts/check.sh (306 Rust tests and 14 packaging checks passed)
  • LIMUX_SMOKE_PROFILE=debug ./scripts/xvfb-smoke-test.sh
  • live Weston regression creates 10 three-terminal workspaces, exits one shell, closes each workspace, and verifies host child processes return from 4 to 1 every cycle
  • existing smoke coverage verifies split/reparent, surface health, terminal I/O, session restore, and clean process-group shutdown
  • shellcheck scripts/xvfb-smoke-test.sh
  • git diff --check

@bvolpato
bvolpato force-pushed the bvolpato/free-terminal-surfaces branch from 83620f1 to 5f09214 Compare August 9, 2026 01:39
@bvolpato

bvolpato commented Aug 9, 2026

Copy link
Copy Markdown
Collaborator Author

Fresh review of exact head 5f09214 found one remaining app-shutdown path that bypasses the new teardown.

P1: window.rs quit_app() calls Application::quit() directly, while pane retirement is only wired through the window close-request handler. Gio quit exits through application shutdown without guaranteeing close-request or widget destruction; an isolated GTK probe confirmed neither signal runs. Super+Q can therefore bypass TerminalHandle::shutdown() and the PR does not yet cover every claimed app-close path.

Route quit_app() through window.close(), or share one idempotent teardown helper between close-request and application quit. Add focused smoke coverage for the configured quit shortcut or direct quit path.

All other changed paths remain green, including the 10-cycle workspace teardown regression. Keeping this open until the final quit path is covered.

@bvolpato
bvolpato force-pushed the bvolpato/free-terminal-surfaces branch from 5f09214 to 6142be8 Compare August 9, 2026 23:25
@bvolpato

bvolpato commented Aug 9, 2026

Copy link
Copy Markdown
Collaborator Author

Addressed in 6142be8. The quit action now clones the application window and calls window.close(), so the existing close-request path saves session state, stops queued saves, retires every pane, and clears host control state before exit. Cloning happens before close() to avoid holding a RefCell borrow during synchronous close-request callbacks.

Validation passed on rebased main: ./scripts/check.sh, focused host check/clippy/tests, ShellCheck, diff check, and the full live GTK smoke including 10 multi-pane teardown cycles.

@bvolpato
bvolpato merged commit b4271a6 into am-will:main Aug 9, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Closing terminal tabs leaks Ghostty surfaces and PTY processes

1 participant