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.

Sign up for GitHub

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

Jump to bottom

If the default shell can't be launched and closeOnExit=true, Terminal buys the farm #2563

Closed
KAMiKAZOW opened this issue Aug 27, 2019 · 7 comments · Fixed by #3623
Closed

If the default shell can't be launched and closeOnExit=true, Terminal buys the farm #2563

KAMiKAZOW opened this issue Aug 27, 2019 · 7 comments · Fixed by #3623
Assignees
@DHowett-MSFT
Labels
Area-TerminalConnection Issues pertaining to the terminal<->backend connection interface Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. Needs-Tag-Fix Doesn't match tag requirements Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Milestone
Terminal v0.8

Comments

@KAMiKAZOW
Copy link

Environment

Windows build number:  10.0.18362.295
Windows Terminal version: 0.4.2382.0

Steps to reproduce

  • Install a WSL distribution (openSUSE Leap 15.0 in my case).
  • Move it to top spot and make it default.
  • Uninstall old WSL distribution and install the upgraded one (openSUSE Leap 15.1 in my case).

Expected behavior

Remove uninstalled WSL distributions. Add all installed WSL distributions unless explicitly disabled in WT.

Actual behavior

WT closes right away. I'm somehow expected to know the UUID change (wsl.exe --list does not show the UUIDs).

Another bug report claims that behavior for non-existent profiles would be handled gracefully in 0.4. Can't confirm that.
@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 Aug 27, 2019
@DHowett-MSFT
Copy link
Contributor

You're not actually expected to know the UUID. The distributions are launched by name alone. Check out the name after wsl.exe -d in your profile configuration.

@DHowett-MSFT
Copy link
Contributor

(You can just delete the guid from the new profile, and WT will automatically generate one for you)

@KAMiKAZOW
Copy link
Author

WT still closes right on launch.

@JBanks
Copy link
Contributor

JBanks commented Sep 1, 2019

We should add in a fail-safe for the event that the default profile has been corrupted. The current settings directory is quite difficult to find for new users, and could discourage experimentation. Perhaps a safe-mode if the past execution exited immediately?

@DHowett-MSFT
Copy link
Contributor

This is now the master bug for "my default shell doesn't launch and it makes me sad because terminal exits instantly"

@DHowett-MSFT DHowett-MSFT changed the title Changing WSL distributions leads to crash If the default shell can't be launched and closeOnExit=true, Terminal buys the farm Sep 5, 2019
@DHowett-MSFT DHowett-MSFT added Area-TerminalConnection Issues pertaining to the terminal<->backend connection interface Issue-Bug It either shouldn't be doing this or needs an investigation. Product-Terminal The new Windows Terminal. and removed Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting labels Sep 5, 2019
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label Sep 5, 2019
@DHowett-MSFT
Copy link
Contributor

This will be helped by #2039 and #2091 (or an evolution thereof).

@DHowett-MSFT DHowett-MSFT self-assigned this Sep 5, 2019
@DHowett-MSFT DHowett-MSFT added this to the Terminal v1.0 milestone Sep 5, 2019
This was referenced Sep 12, 2019
Merged
Closed
@DHowett-MSFT DHowett-MSFT mentioned this issue Sep 25, 2019
Closed
@DHowett-MSFT DHowett-MSFT mentioned this issue Oct 14, 2019
Closed
@DHowett-MSFT DHowett-MSFT mentioned this issue Oct 23, 2019
Closed
DHowett-MSFT pushed a commit that referenced this issue Nov 6, 2019
@DHowett
Replace ConhostConnection with ConptyConnection
932775a 
This commit deletes ConhostConnection and replaces it with
ConptyConnection. The ConptyConnection uses CreatePseudoConsole and
depends on winconpty to override the one from kernel32.

* winconpty must be packagebale, so I've added GetPackagingOutputs.
   * To validate this, I added conpty.dll to the MSIX regression script.
* I moved the code from conpty-universal that deals with environment
  strings into the types library.

This puts us in a way better place to implement #2563, as we can now
separately detect a failure to launch a pseudoconsole, a failure to
CreateProcess, and a failure of the launched process.

Fixes #1131.
@DHowett-MSFT DHowett-MSFT mentioned this issue Nov 6, 2019
Merged
5 tasks
DHowett-MSFT pushed a commit that referenced this issue Nov 6, 2019
@DHowett
Replace ConhostConnection with ConptyConnection
8bde4c2 
This commit deletes ConhostConnection and replaces it with
ConptyConnection. The ConptyConnection uses CreatePseudoConsole and
depends on winconpty to override the one from kernel32.

* winconpty must be packagebale, so I've added GetPackagingOutputs.
   * To validate this, I added conpty.dll to the MSIX regression script.
* I moved the code from conpty-universal that deals with environment
  strings into the types library.

This puts us in a way better place to implement #2563, as we can now
separately detect a failure to launch a pseudoconsole, a failure to
CreateProcess, and a failure of the launched process.

Fixes #1131.
DHowett-MSFT pushed a commit that referenced this issue Nov 6, 2019
@DHowett
Replace ConhostConnection with ConptyConnection
01a146f 
This commit deletes ConhostConnection and replaces it with
ConptyConnection. The ConptyConnection uses CreatePseudoConsole and
depends on winconpty to override the one from kernel32.

* winconpty must be packagebale, so I've added GetPackagingOutputs.
   * To validate this, I added conpty.dll to the MSIX regression script.
* I moved the code from conpty-universal that deals with environment
  strings into the types library.

This puts us in a way better place to implement #2563, as we can now
separately detect a failure to launch a pseudoconsole, a failure to
CreateProcess, and a failure of the launched process.

Fixes #1131.
DHowett-MSFT pushed a commit that referenced this issue Nov 6, 2019
@DHowett
Replace ConhostConnection with ConptyConnection (#3461)
357e835 
This commit deletes ConhostConnection and replaces it with
ConptyConnection. The ConptyConnection uses CreatePseudoConsole and
depends on winconpty to override the one from kernel32.

* winconpty must be packageable, so I've added GetPackagingOutputs.
   * To validate this, I added conpty.dll to the MSIX regression script.
* I moved the code from conpty-universal that deals with environment
  strings into the types library.

This puts us in a way better place to implement #2563, as we can now
separately detect a failure to launch a pseudoconsole, a failure to
CreateProcess, and an unexpected termination of the launched process.

Fixes #1131.
DHowett-MSFT pushed a commit that referenced this issue Nov 14, 2019
@DHowett @carlos-zamora
Update doc/specs/#2563 - closeOnExit and TerminalConnection evolution.md
6e7eba2 
Co-Authored-By: Carlos Zamora <[email protected]>
@DHowett-MSFT DHowett-MSFT mentioned this issue Nov 14, 2019
Merged
5 tasks
@DHowett-MSFT DHowett-MSFT mentioned this issue Nov 19, 2019
Merged
5 tasks
@ghost ghost added the In-PR This issue has a related PR label Nov 19, 2019
@DHowett-MSFT DHowett-MSFT added Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. and removed Issue-Bug It either shouldn't be doing this or needs an investigation. labels Nov 20, 2019
@ghost ghost added Needs-Tag-Fix Doesn't match tag requirements and removed In-PR This issue has a related PR labels Nov 25, 2019
DHowett-MSFT pushed a commit that referenced this issue Nov 25, 2019
@DHowett @mcpiroman
Implement ConnectionState and closeOnExit=graceful/always/never (#3623)
901a1e1 
This pull request implements the new
`ITerminalConnection::ConnectionState` interface (enum, event) and
connects it through TerminalControl to Pane, Tab and App as specified in
#2039. It does so to implement `closeOnExit` = `graceful` in addition to
the other two normal CoE types.

It also:

* exposes the singleton `CascadiaSettings` through a function that
  looks it up by using the current Xaml application's `AppLogic`.
  * In so doing, we've broken up the weird runaround where App tells
    TerminalSettings to CloseOnExit and then later another part of App
    _asks TerminalControl_ to tell it what TerminalSettings said App
    told it earlier. `:crazy_eyes:`
* wires up a bunch of connection state points to `AzureConnection`.
  This required moving the Azure connection's state machine to use another
  enum name (oops).
* ships a helper class for managing connection state transitions.
* contains a bunch of template magic.
* introduces `WINRT_CALLBACK`, a oneshot callback like `TYPED_EVENT`.
* replaces a bunch of disparate `_connecting` and `_closing` members
  with just one uberstate.
* updates the JSON schema and defaults to prefer closeOnExit: graceful
* updates all relevant documentation

Specified in #2039
Fixes #2563

Co-authored-by: mcpiroman <[email protected]>
@ghost ghost added the Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. label Nov 25, 2019
@DHowett-MSFT DHowett-MSFT mentioned this issue Nov 25, 2019
Closed
@ghost
Copy link

ghost commented Jan 14, 2020

🎉This issue was addressed in #3623, which has now been successfully released as Windows Terminal Preview v0.8.10091.0.:tada:

Handy links:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@DHowett-MSFT DHowett-MSFT

Labels
Area-TerminalConnection Issues pertaining to the terminal<->backend connection interface Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. Needs-Tag-Fix Doesn't match tag requirements Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Projects
None yet
Milestone
Terminal v0.8
Development

Successfully merging a pull request may close this issue.

Implement ConnectionState and closeOnExit=graceful/always/never microsoft/terminal
3 participants
@JBanks @KAMiKAZOW @DHowett-MSFT