microsoft / terminal

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

Terminal cannot be closed when the defterm client closes too quickly (and creates a window with no tabs) #15936

Open floppyhammer opened 1 year ago

floppyhammer commented 1 year ago

[!IMPORTANT]
📌 Pinned comment: https://github.com/microsoft/terminal/issues/15936#issuecomment-1710747811 We're pretty sure this comes down to two effects:

  • The client app closes before the Terminal can accept the handoff and create a tab for it.
  • The Terminal can't be closed when it has no tabs open.

We should really fix both of these at the same time.


Windows Terminal version

1.17.12191.0

Windows build number

25941.1000

Other Software

No response

Steps to reproduce

There's no routine to easily reproduce it. It happens occasionally when I use some command line tools (like npm) that will invoke another terminal/cmd console. When the process is done, the invoked terminal will not close successfully (only the tab is closed, which results in an empty terminal with no tab), and you are unable to close it manually (because there's no tab).

Expected Behavior

A terminal losing any tabs should not exist.

Actual Behavior

terminal

lhecker commented 1 year ago

There's no routine to easily reproduce it. It happens occasionally when I use some command line tools (like npm) that will invoke another terminal/cmd console.

I don't think anyone on the team has encountered this before. Are there some steps we could follow to attempt to reproduce it? (For instance if you list the tools and commands that cause the issue for you.)

tusharsnx commented 1 year ago

I happened to hit this bug many times. The strange terminal window (without any tabs on it) is invoked with -Embedding arg in the background by something, not sure what but ~I've a feeling~ (I've lost the feeling 😄) that it's VS.

image

tusharsnx commented 1 year ago

More Info:

It seems to be invoked by ~wininit.exe~ svchost.exe?

Screenshot 2023-09-08 002937

image

tusharsnx commented 1 year ago

This is quite funny, but we have a repro right in our test suite 😂 (So I was correct, VS was related in this, maybe indirectly)

Step to reproduce:

video demo: https://youtu.be/lNJOSjXioOY

zadjii-msft commented 1 year ago

My theory is that if the client application exits fast enough, we start a defterm window, but we never get it to the point of making an actual tab.

I see this sometimes (but usually only when I have a horribly busted defterm install).

I wonder if this repros after the process model changes in 1.18 EDIT: #16234 would imply that yes, this repros on 1.18

floppyhammer commented 1 year ago

My repro case:

  1. Run npm run dev to start a server.
  2. Force close it by pressing ctrl+c once, and no need to confirm the ensued prompt Terminate batch job (Y/N)?.
  3. Normally, a terminal window pops up and closes itself quickly. Sometimes there's a second window following, but it will stay and it contains no tab.
DanielHabenicht commented 1 year ago

I can also reproduce with:

For me it looks even more odd, as the windows don't even have a fill: image

tusharsnx commented 1 year ago

@DanielHabenicht I suppose you're Win10, and the fill we can see in other repros is Mica which isn't available on Win10. Therefore, no fill? 🤔

DanielHabenicht commented 1 year ago

Yes, I am on windows 10. You can see the version in the issue linked above.

danielniccoli commented 1 year ago

Here is another repro. Steps inside: https://github.com/microsoft/terminal/issues/16281

I noticed that sometimes when this happens, in addition to Windows Terminal launching without a tab (the reported bug) it does also start the classic shell that then runs the actual PowerShell command/script.

Edit: By the way, this is the script I'm running.

Start-Sleep 1  # Try to work around https://github.com/microsoft/terminal/issues/15936
if((Test-Path "C:\code") -and $env:OneDriveCommercial -and (Test-Path $env:OneDriveCommercial)) {
    # With monitoring
    # robocopy "C:\code" (Join-Path $env:OneDriveCommercial code_backup) /MIR /TIMFIX /MON:1 /MOT:1 /DCOPY:DAT /XD venv .venv node_modules __pycache__ /NFL /NDL /R:10 /W:1
    # Without monitoring
    $p = Start-Process robocopy -ArgumentList "C:\code `"$(Join-Path $env:OneDriveCommercial code_backup)`" /MIR /TIMFIX /DCOPY:DAT /XD venv .venv node_modules __pycache__ /NFL /NDL /R:10 /W:1" -NoNewWindow -Wait -PassThru
    Start-Sleep 1  # Try to work around https://github.com/microsoft/terminal/issues/15936
    exit $p.ExitCode
} else {
    Start-Sleep 1  # Try to work around https://github.com/microsoft/terminal/issues/15936
    exit 1
}
PaprikaBoyTheOneAndOnly commented 8 months ago

Hi, are there any updates on this?

zadjii-msft commented 8 months ago

I'm investigating this currently. I could have swore I used to have a minimal repro for this, but I'm having a hard time currently 😕

zadjii-msft commented 8 months ago

If someone who's seeing this consistently could Capture a Debug ETL Trace and share it here, that would help immensely. I'm guessing there's one of two things going on:

If we for whatever reason see a ReceiveTerminalHandoff_Success, then that would be surprising to me.


notes First things first: * Conhost starts up. It finds that it's got a delegation **console**, and it starts that. (we can assume this works, because a terminal does get started) * OpenConsole starts up, as the target of a defcon handoff. * `ConsoleEstablishHandoff`: * It finds the target terminal to _also_ delegate to. * Does quite a bit of pipe manipulation, to get ready to call the terminal. (we can assume this works, because a terminal does get started) * We open a handle to the target client process. We hang onto that handle until after the end of `ConsoleEstablishHandoff`. * Calls `CoCreateInstance(g.delegationPair.terminal,...`. (this instantiates terminal, below.) * gets our connection info: Icon, lnk, settings. * **⚠️ yikes** there's a `RETURN_IF_NTSTATUS_FAILED(ConsoleInitializeConnectInfo(...))` in here. If that does fail, we've started terminal, but failed to do a handoff. * We call `handoff->EstablishPtyHandoff` * This will get handled in `CTerminalHandoff::EstablishPtyHandoff`, which is broken down below. _now, over in the terminal, our registered default terminal handoff target. The following occurs when the terminal is launched as a COM server_ These paths to `_StartInboundListener` take care of both glomming and non glomming. * In the glomming scenario, the monarch tells the MRU window to `ExecuteCommandline`, and that will start the inbound listener * in the non-glomming, `TerminalWindow::Initialize` sets the inbound listener. In both cases, the inbound listener is started immediately if the page already laid out, or we set a flag and wait till the first layout to accept the handoff. * `TerminalWindow::Initialize`, if `_appArgs.IsHandoffListener` OR `TerminalWindow::ExecuteCommandline`, again, if `_appArgs.IsHandoffListener` * `TerminalPage::SetInboundListener` * sets `_shouldStartInboundListener=true` * If we were already laid out the first time, immediately calls `_StartInboundListener` * `TerminalPage::_OnFirstLayout` called when the page first lays out * process startup actions * call `_StartInboundListener` to possibly start accepting handoff So here, we're already laid out, and we know this window wants to accept the handoff. * `TerminalPage::_StartInboundListener` if `_shouldStartInboundListener`... * registers the `ConptyConnection::NewConnection` callback * calls `ConptyConnection::StartInboundListener` * `CTerminalHandoff::s_StartListening` passes the `ConptyConnection::NewHandoff` function as the callback to: * `CoRegisterClassObject(CTerminalHandoff, us, as a local server, single-use)` * returns _later, when conpty is ready...._ * `CTerminalHandoff::EstablishPtyHandoff` * THROWS if it fails, for ANY reason * **⚠️ Throwing would also just never call the callback, nor signal an error.** * STOPS listening * duplicates handles * calls it's registered callback * `ConptyConnection::NewHandoff` raises the handoff, which is handled in * `TerminalPage::_OnNewConnection` accepts the connection from conpty * possibly `co_await`s to get onto the window thread. * Creates the tab `_CreateNewTabFromPane(MakePane(..., existingConnection))`
zadjii-msft commented 2 months ago

Leonard: if you use the debug version, set a breakpoint during handoff, or just put a Sleep(10000) on the Terminal side, then the conhost will fail out and the close events will fire before the close events get set up

alzhahir commented 2 months ago

Hi, I think I'm facing a similar issue and was able to reproduce it consistently. Not sure if it's related to this bug specifically, but I hope it helps somehow. For the purposes here, I am using Terminal v1.21.2361.0.

  1. Close all Terminal windows.
  2. Open a Terminal window. (Window 1)
  3. Use a command* to launch a second Terminal window, (Window 2) with the command exiting automatically.
  4. Use mouse to hold on Window 2 as the command exits. (i.e dragging the window)
  5. Press the close button on Window 2 and Window 1 gets closed instead.
  6. We are now left with Window 2.
    • Pressing the close button again won't do anything.
    • Launching a new tab on Window 2 works fine but will also not close the window when all tabs are closed, or the close button is pressed.
    • To exit out of this "buggy" state, you would need to end the Terminal process or launch a new Terminal window. (either from external methods, or from within the bugged window itself)

*In my case, I initially used the command wt pwsh -wd "C:\Users" -c {Write-Host "test";} but since I was not sure if this was a PowerShell issue or Terminal issue, I also tested the command wt ping github.com and it worked as well.

Several observations: