microsoft / terminal

The new Windows Terminal and the original Windows console host, all in the same place!
MIT License
95.74k stars 8.33k forks source link

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

Open alabuzhev opened 3 years ago

alabuzhev commented 3 years ago

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;

}



2. Go to the Terminal settings, `+ Add a new profile`, `+ New empty profile`, and set `Command line` to the compiled exe.
3. 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.
DHowett commented 3 years ago

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 commented 3 years ago

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.

mhilbrunner commented 2 years ago

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: https://github.com/godotengine/godot/issues/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 commented 2 years ago

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 commented 2 years ago

¹ 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.

xPaw commented 1 year ago

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.