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

Create a "conpty" framework package to bundle up the PseudoConsole APIs and conhost #1130

Closed
3 of 4 tasks
DHowett-MSFT opened this issue Jun 4, 2019 · 8 comments
Closed
3 of 4 tasks
Labels
Area-Interop Communication between processes 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-Conpty For console issues specifically related to conpty
Milestone

Comments

@DHowett-MSFT
Copy link
Contributor

DHowett-MSFT commented Jun 4, 2019

This deliverable tracks pulling out the source for the PseudoConsole API set.

Rationale

Right now, we're using conhost.exe directly as a pseudoconsole host. If we ship it as a framework package, we can switch to the "official" PseudoConsole APIs (CreatePseudoConsole, ClosePseudoConsole, etc.) and offer conpty as an independently-upgradable package on which WT and third party applications can depend. This will allow us to more fully undock from the OS release cycle.

Requirements

  • Document/open HPCON
  • Translate winconpty.c (which currently lives in kernelbase) from C to C++
    • produce conpty.dll, which hosts the three pseudoconsole APIs
  • Produce a framework package

see also: #3577

@DHowett-MSFT DHowett-MSFT added Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. Area-Interop Communication between processes Product-Conpty For console issues specifically related to conpty labels Jun 4, 2019
@DHowett-MSFT DHowett-MSFT added this to the Terminal Backlog milestone Jun 4, 2019
@ghost ghost added the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label Jun 4, 2019
@DHowett-MSFT DHowett-MSFT removed the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label Jun 4, 2019
@zadjii-msft
Copy link
Member

As discussed with @Tyriar, we should probably also just make a nuget package for the ConPTY APIs in lieu of a proper framework package. Framework packages will only work for packaged applications, and not for something like VSCode.

DHowett-MSFT pushed a commit that referenced this issue Sep 4, 2019
This pull request introduces a copy of the code from kernel32.dll that
implements CreatePseudoConsole, ClosePseudoConsole and
ResizePseudoConsole. Apart from some light modifications to fit into the
infrastructure in this project and support launching OpenConsole.exe, it
is intended to be 1:1 with the code that ships in Windows.

Any guideline violations in this code are likely intentional. Since this
was built into kernel32, it uses the STL only _very sparingly._

Consumers of this library must make sure that conpty.lib lives earlier
in the link line than onecoreuap_apiset, onecoreuap, onecore_apiset,
onecore or kernel32.

Refs #1130.
@davidhewitt
Copy link

Random question, is there a way that winconpty could be packaged as a static library which contains the conhost server internally? It would be nice if applications could be built using winconpty and can then be shipped without needing to have OpenConsole.exe / conhost.exe packaged beside them.

@DHowett-MSFT
Copy link
Contributor Author

Not easily: conhost isn’t architected in such a way as to allow multiple instances of it to be hosted in the same address space. It’s riddled with singletons and global state. You’d only be able to host a single console session with a static pseudoconsole library 😄

@davidhewitt
Copy link

It’s riddled with singletons and global state.

Yikes! Amazingly that would still be enough for the use case I was thinking (Alacritty). It would only ever host a single console process per Alacritty process.

Such a library would greatly help Alacritty use ConPTY with the ability to control the version of ConPTY that it's paired with. For example, I've been wondering about submitting a PR to implement #262. Using overlapped I/O would help to unify our code with the Win7 legacy code (WinPTY) which also uses overlapped I/O. However, as things are, we'd probably want to keep supporting the blocking ConPTY I/O implementation until we were sure all Windows 10 versions with blocking I/O are more or less out-of-use. If we could control the ConPTY version, ideally by a static lib rather than packaging with a conhost.exe, we could entirely remove the blocking I/O implementation in Alacritty.

For safety, such a static library could add a check to ensure only one console process is spawned, right?

If I were to author one, would you be willing to review a PR which adds a way to build conhost as a static lib?

@DHowett-MSFT
Copy link
Contributor Author

DHowett-MSFT commented Dec 28, 2019

I appreciate the offer, but I wouldn’t be willing to ship that as a solution. Sorry! There’s too many caveats, and moving forward I’d prefer a solution that takes us through true console host multi-instancing.

From an architectural standpoint, it seems more robust to run the console host as a separate process if only for the insulation it affords the pty user from crashes.

Also, see #3568: a static library solution may make it difficult for an x86 application running on x64 to properly host consoles, as the conhost needs to match the system architecture.

@DHowett-MSFT
Copy link
Contributor Author

DHowett-MSFT commented Dec 28, 2019

I was idly investigating shipping conhost as a dll and using something like rundll to host it so it didn’t read so immediately as a separate process, and could be another entry point on winconpty.dll, but that still doesn’t help us with #3568...

@davidhewitt
Copy link

Understood, thanks for investigating!

wez added a commit to wezterm/wezterm that referenced this issue Apr 6, 2020
This commit allows loading the console functions from `conpty.dll`
instead of `kernel32.dll` which means that we can update and
track newer features than have been deployed to Windows.

In practical terms this means that we can now unlock mouse input
reporting in eg: VIM running under WSL.
refs: microsoft/terminal#376

We're jumping the gun on this issue, which is tracking making
a proper supportable way to deploy this sort of update:
refs: microsoft/terminal#1130

For now it seems easier just for us to bundle our own copy of
these bits.

This includes a speculative change to include those in our
Windows downloads also.

The binaries were built from
microsoft/terminal@4f8acb4
@zadjii-msft zadjii-msft modified the milestones: Terminal Backlog, Backlog Jan 4, 2022
@DHowett DHowett closed this as completed Apr 27, 2022
@ghost ghost added the Needs-Tag-Fix Doesn't match tag requirements label Apr 27, 2022
@DHowett
Copy link
Member

DHowett commented Apr 27, 2022

This is obsoleted by #12980!

ghost pushed a commit that referenced this issue Apr 27, 2022
This pull request introduces a packaging phase that emits
Microsoft.Windows.Console.ConPTY, a nuget package that contains the
pseudoconsole API as well as the requisite copies of conhost.

* winconpty learned to load a version of OpenConsole.exe specific to the
  processor architecture on its hosting machine
* the package, as well as its contents, is signed properly and is nearly
  ready for distribution via nuget.org
* the API in conpty-static.h has been adjusted to expose
  CreatePseudoConsoleAsUser and stamp out the correct DLL import/export
  annotations.
* getting .NET to play right was somewhat challenging, but I tested this
  against .NET 6.0 and it seemed to work properly; it shipped conpty.dll
  in the right places, and it shipped OpenConsole.exe next to the
  published application.

In the future, we could provide an interop assembly for C# consumers;
that is, unfortunately, out of scope today.

Closes #3577
Closes #3568
Obsoletes #1130
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Interop Communication between processes 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-Conpty For console issues specifically related to conpty
Projects
None yet
Development

No branches or pull requests

4 participants