Skip to content
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

fix: exit shell process on terminal close #14823

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

kittaakos
Copy link
Contributor

What it does

Check exitStatus.code instead of checking the falsyness of the exitStatus. The latter is always truthy with the exit reason support.

Ref: #12293

Closes: #13346

How to test

From #13346 (comment):

Steps to Reproduce:

  1. Open two Terminal tabs.
  2. In one them them, start a long-running process, such as the yes command.
  3. In the other tab, use ps -ef or top to see the process running.
  4. Close the Terminal with the long-running process.
  5. Observe that the process is still running somewhere in the background.
fix_theia_11997.mov

Follow-ups

Breaking changes

  • This PR introduces breaking changes and requires careful review. If yes, the breaking changes section in the changelog has been updated.

Attribution

Review checklist

Reminder for reviewers

Check `exitStatus.code` instead of checking the falsyness of the
`exitStatus`. The latter is always truthy with the exit reason
support.

Ref: eclipse-theia#12293

Closes: eclipse-theia#13346
@msujew msujew requested a review from rschnekenbu February 3, 2025 15:25
@kittaakos
Copy link
Contributor Author

kittaakos commented Feb 3, 2025

This PR should also close #12715. Closing the terminal for the long-running tasks sends the SIGHUP signal to the task process and terminates it.

OP of #12715 expects the following:

  • Run workspace tasks as a live service like server ( "react-native start" -> metro bundler using 8081 port)
  • Close the task terminal but keep the task running.

This is not the correct expectation. Closing the terminal window of the task must terminate the underlying process.

.vscode/tasks.json:

{
  "version": "2.0.0",
  "tasks": [
    {
      "label": "Run HTTP Server",
      "command": "node index.js",
      "type": "shell"
    }
  ]
}

index.js:

const http = require('http');
const port = 4567;
const hostname = '127.0.0.1';
const server = http.createServer(function (req, res) {
    res.writeHead(200, { 'Content-Type': 'text/plain' });
    res.end('Hello World!\n');
});
server.listen(port, hostname, () => {
    console.log(`Node.js web server at ${hostname}:${port} is running..`);
});

Open the browser on 127.0.0.1:4567 and see that it works when the task runs. When the terminal window is closed, the task terminates:

fixes_12715.mov

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Waiting on reviewers
Development

Successfully merging this pull request may close these issues.

Closing a Terminal tab doesn't terminate the running process
1 participant