microsoft / microsoft/terminal

CreatePseudoConsole doesn't handle sockets correctly

Open
#4,359 6 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Area-Quality Help Wanted Issue-Bug Priority-1 Product-Conpty
Dominant language
C++
Stars
105k
Forks
9.6k
Avg merge
3d 17h
Merged PRs (30d)
29

Description

The documentation of CreatePseudoConsole explicitly documents that CreatePseudoConsole can be used with sockets:

https://docs.microsoft.com/en-us/windows/console/creating-a-pseudoconsole-session#preparing-the-communication-channels

If we look at the implementation of CreatePseudoConsole, we can see it calls DuplicateHandle on the handles passed to it:

https://github.com/microsoft/terminal/blob/2712e41cad2abcfe627934e41e713ea132a659de/src/winconpty/winconpty.cpp#L348-L349

However, the documentation of DuplicateHandle explicitly mentions that DuplicateHandle should not be used to duplicate socket handles:

https://docs.microsoft.com/en-us/windows/win32/api/handleapi/nf-handleapi-duplicatehandle#remarks

Doesn't this make the CreatePseudoConsole implementation incorrect when using sockets?

I haven't actually confirmed whether sockets work or not, I'm currently trying to get CreatePseudoConsole to work with sockets but it's returning Access is Denied. While looking at the source code to figure out why it's returning Access is Denied, I stumbled upon this possible issue.

EDIT: Nvm the Access is Denied issue, I incorrectly assumed CreatePseudoConsole would return 0 on failure. The DuplicateHandle issue still stands however.

EDIT2: Removed edit because it was a programming error by me.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in src/winconpty/winconpty.cpp at the DuplicateHandle calls around lines 348-349, then compare that behavior with the CreatePseudoConsole and DuplicateHandle documentation linked in the issue. Reproduce CreatePseudoConsole with socket handles and determine whether the implementation or documentation is incorrect; done means the socket behavior is confirmed and the issue is resolved or clearly documented.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
networking, operating-systems
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
32/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.