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

Terminal tab terminates along with the initial process even if there are other active tasks in that console #10501

Open
alabuzhev opened this issue Jun 23, 2021 · 6 comments
Labels
Area-TerminalConnection Issues pertaining to the terminal<->backend connection interface Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-2 A description (P2) Product-Terminal The new Windows Terminal.
Milestone

Comments

@alabuzhev
Copy link
Contributor

Windows Terminal version (or Windows build number)

1.9.1445.0

Other Software

No response

Steps to reproduce

  1. Compile the following simple program:
#include <windows.h>

int main()
{
	STARTUPINFOW si{ sizeof(si) };
	PROCESS_INFORMATION pi{};

	if (!CreateProcessW(L"c:\\windows\\system32\\cmd.exe", {}, {}, {}, false, 0, {}, {}, &si, &pi))
		return 1;

	CloseHandle(pi.hThread);
	CloseHandle(pi.hProcess);

	return 0;
}
  1. Go to the Terminal settings, + Add a new profile, + New empty profile, and set Command line to the compiled exe.
  2. Try to open that profile.

Expected Behavior

The tab should remain active after the initial process exits since there are other active tasks still attached to that particular console.

Actual Behavior

It closes immediately.

Additional details

The problem was initially observed in a more complex scenario, where an app performs a self-update by launching the updater helper process in the same console and exiting. The helper then does its magic, restarts the original program and exits, providing a smooth user experience.

Needless to say that it works and always worked in conhost.

@ghost ghost added Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Needs-Tag-Fix Doesn't match tag requirements labels Jun 23, 2021
@DHowett
Copy link
Member

DHowett commented Jun 23, 2021

So, this was an intentional design choice that I'm willing to revisit. In the majority of cases, we haven't seen too much trouble from tracking only the root process ... but I do understand that there's a reason folks want to exchange who is the session leader in a given console session.

I suspect that the most correct way to handle this while still keeping exit code reporting (and therefore the "graceful" close-on-exit behavior of closing the tab when the root process exits with code 0) would be to either expose process state through ConPTY's signal channel or have WT track the process tree spawned by the root process¹.

¹ we may need to do this anyway--it would be nice to present a "do you want to close this tab?" confirmation dialog that tells you what's running...

@alabuzhev
Copy link
Contributor Author

track the process tree spawned by the root process

Not the whole tree though: children could be spawned with CREATE_NEW_CONSOLE, DETACHED_PROCESS, or could just be GUI processes. If they're not attached to the root's console there's no point in waiting for them or including them in a confirmation dialog.

The list that GetConsoleProcessList returns is probably reliable.

@zadjii-msft zadjii-msft added the Product-Terminal The new Windows Terminal. label Jul 20, 2021
@mhilbrunner
Copy link

mhilbrunner commented Dec 15, 2021

Hey! On the Godot Game Engine, we're currently getting a lot of user reports of Godot not working on Windows 11 due to the Windows Terminal.

This is likely the root cause.

Godot launches a "project manager" process, where you can browser and select your game projects, and once you select one, it exits and starts the actual editor for that project in a new process, reusing the same console.

In Windows 11 with Windows Terminal as default console, the Windows Terminal just exits if you select a project to open in the project manager.

We've patched it in the current version by using CREATE_NEW_CONSOLE, but that doesn't change the fact that all previous released versions (e.g. on Steam) suddenly don't work anymore on Windows 11 and exit after trying to open something in the project manager, without any error message or indication for users.

If you want to have a look, this is our issue tracking this: godotengine/godot#54076

Note that the issue is closed as we used CREATE_NEW_CONSOLE as a workaround, but it would be very valuable to us and our community for the old behaviour to work (and for all previous Godot Engine versions to work again).

@eryksun
Copy link

eryksun commented Dec 15, 2021

ISTM that only the console host process, "OpenConsole.exe", would need to be tracked for the lifetime of a tab. It's responsible for managing the console session and will exit when there are no more attached processes. For tracking in general (e.g. the session exit status), the pseudoconsole signal channel can report the pid of processes when they attach and detach from the session (e.g. AttachConsole, FreeConsole) and also the current root process whenever it changes.

Though I do think there's a need to be able to kill a console session via the root process. As mentioned on another issue, pseudoconsole sessions could be improved to support a graceful exit via taskkill.exe /pid <root_process_pid>, which the classic console has supported for over 20 years. Graceful termination is implemented by posting WM_CLOSE to the top-level windows of a process. This relies on getting a window's owner via GetWindowThreadProcessId(), which is intentionally designed to return the effective owner of a console window. The classic console maintains its root process as the effective owner. Two features would need to be implemented for the hidden window of a pseudconsole session. Its window procedure would have to end the session when the window is closed. Also, the window class would have to be registered with the system as a console window class (apparently this is implemented by a private NtUser system call), which requires maintaining the effective owner as window data.

@zadjii-msft
Copy link
Member

¹ we may need to do this anyway--it would be nice to present a "do you want to close this tab?" confirmation dialog that tells you what's running...

Yanking triage and sticking it on the backlog for this reason. We're tracking that in #6549, so whenever we get to that, we should be able to enable this as well.

@zadjii-msft zadjii-msft added Issue-Bug It either shouldn't be doing this or needs an investigation. Area-TerminalConnection Issues pertaining to the terminal<->backend connection interface Priority-2 A description (P2) labels Aug 15, 2022
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label Aug 15, 2022
@zadjii-msft zadjii-msft added this to the Backlog milestone Aug 15, 2022
@zadjii-msft zadjii-msft removed the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label Aug 15, 2022
@xPaw
Copy link

xPaw commented Dec 29, 2022

I ran into this issue as well where a process updated itself by spawning a new process and exiting the parent. This works outside of Terminal and works on other operating systems.

It's rather unexpected that this behaviour changed in terminal.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-TerminalConnection Issues pertaining to the terminal<->backend connection interface Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-2 A description (P2) Product-Terminal The new Windows Terminal.
Projects
None yet
Development

No branches or pull requests

6 participants