Skip to content

Fix terminal corruption on unexpected termination by adding signal handlers #10326

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from

Conversation

CodeMan62
Copy link

@CodeMan62 CodeMan62 commented Apr 13, 2025

Description

When the Turborepo TUI is terminated unexpectedly (via SIGINT, SIGTERM, etc.), the terminal can be left in a corrupted state, causing strange characters to appear when hovering over the terminal. This PR addresses this issue by adding proper signal handling in app.rs with platform-specific implementations for both Unix and Windows, implementing an emergency terminal cleanup procedure that safely restores terminal state on abrupt termination, enhancing scrollback management in grid.rs to handle edge cases better, and adding proper dependency management for required signal handling libraries.

Fixes #10210

Testing Instructions

  1. Compiling the project with cargo build to ensure all code changes compile successfully
  2. Running the test suite with cargo test -p turborepo-ui -p turborepo-vt100 to verify all existing tests still pass
  3. Manual testing by launching the TUI and using various signals (SIGINT, SIGTERM) to force termination
  4. Verifying terminal state restoration by checking for visual corruption after forced termination

@CodeMan62 CodeMan62 requested a review from a team as a code owner April 13, 2025 15:04
@turbo-orchestrator turbo-orchestrator bot added the needs: triage New issues get this label. Remove it after triage label Apr 13, 2025
Copy link

vercel bot commented Apr 13, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
examples-basic-web ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 13, 2025 3:04pm
examples-designsystem-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 13, 2025 3:04pm
examples-native-web ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 13, 2025 3:04pm
examples-svelte-web ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 13, 2025 3:04pm
examples-tailwind-web ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 13, 2025 3:04pm
examples-vite-web ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 13, 2025 3:04pm

Copy link

vercel bot commented Apr 13, 2025

@CodeMan62 is attempting to deploy a commit to the Vercel Team on Vercel.

A member of the Team first needs to authorize it.

@anthonyshew
Copy link
Contributor

Just making sure, this is for #10210, correct?

@CodeMan62
Copy link
Author

@anthonyshew yes sorry i forget to mention the issue in description let me update it

@anthonyshew
Copy link
Contributor

No worries! Thank you for the PR. 🙌 We'll review soon.

Copy link
Member

@chris-olszewski chris-olszewski left a comment

Choose a reason for hiding this comment

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

We already register signal handlers in the main binary: https://github.com/vercel/turborepo/blob/main/crates/turborepo-lib/src/commands/run.rs#L12. These handlers send a stop event to the TUI receiver and the TUI then does the terminal cleanup.

We do not want to overwrite these signal handlers as there are other parts of the system that rely on those previous handlers being registered.

In your testing instructions you have

Verifying terminal state restoration by checking for visual corruption after forced termination

Do your have a system where the current release results in artifacts and this PR doesn't when you send SIGINT to turbo?

@CodeMan62
Copy link
Author

We already register signal handlers in the main binary: main/crates/turborepo-lib/src/commands/run.rs#L12. These handlers send a stop event to the TUI receiver and the TUI then does the terminal cleanup.

We do not want to overwrite these signal handlers as there are other parts of the system that rely on those previous handlers being registered.

In your testing instructions you have

Verifying terminal state restoration by checking for visual corruption after forced termination

Do your have a system where the current release results in artifacts and this PR doesn't when you send SIGINT to turbo?

No, I haven't observed any visual artifacts when sending SIGINT to turbo in the current release. You're right - I included those testing instructions preemptively as part of adding the cleanup code, but since we already have proper signal handling and terminal cleanup in the main binary that sends stop events to the TUI receiver, I'll remove both the redundant cleanup code and those testing instructions.

@CodeMan62
Copy link
Author

Hey @chris-olszewski is there anything left or if this seems not to solve the actual problem can you guide me

@chris-olszewski
Copy link
Member

I do not think this strategy solves the problem. #10210 has 2 issues reported in it:

  • The vt100 getting into a bad state from the task output it parses. Fixing this will require finding out what sequence of output causes this.
  • turbo getting interrupted by a signal that isn't caught by one of the signal handlers and results in the program shutting down without getting a chance to clean up the terminal. This can be caused by a SIGKILL which we cannot catch or do not have a handler for. You could test additional signal types and open a PR adding them to our handlers.

@CodeMan62 CodeMan62 closed this Apr 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs: triage New issues get this label. Remove it after triage
Projects
None yet
Development

Successfully merging this pull request may close these issues.

VSCode terminal crashing in specific case with TUI
3 participants