starred/terminal
1
0
Fork 0
mirror of https://github.com/microsoft/terminal.git synced 2026-02-03 21:25:34 +00:00
Code Issues 19.0k Packages Projects Releases 20 Wiki Activity

When using PROC_THREAD_ATTRIBUTE_PSEUDOCONSOLE, what 'size' parameter should be passed to UpdateProcThreadAttribute? #9307

New Issue
Closed
opened 2026-01-31 01:51:19 +00:00 by claunia · 3 comments
claunia commented 2026-01-31 01:51:19 +00:00
Owner
Copy Link

Originally created by @njsmith on GitHub (Jun 28, 2020).

Originally assigned to: @DHowett on GitHub.

In all the examples I see for PROC_THREAD_ATTRIBUTE_PSEUDOCONSOLE, when calling UpdateProcThreadAttribute, it passes the lpValue parameters as an HPCON value, and the cbSize parameter as sizeof(HPCON). For example:

aa1ed0a19c/samples/ConPTY/EchoCon/EchoCon/EchoCon.cpp (L147-L157)

But... this doesn't make any sense to me. HPCON is a pointer to a PseudoConsole object. And I thought that when calling UpdateProcThreadAttribute, the cbSize parameter was supposed to be set to the size of the pointed-to object. For example, the docs say:

lpValue
A pointer to the attribute value. This value should persist until the attribute is destroyed using the DeleteProcThreadAttributeList function.

cbSize
The size of the attribute value specified by the lpValue parameter.

And the examples on that page agree – e.g. when demonstrating PROC_THREAD_ATTRIBUTE_PROTECTION_LEVEL, they pass &ProtectionLevel and sizeof(ProtectionLevel).

But in the ConPTY examples and docs, we don't pass the size of the pointed-to pseudoconsole object. Instead, we're setting cbSize to the size of a pointer, so 4 or 8 depending on whether this is a 32- or 64-bit build.

Shouldn't we be passing sizeof(*HPCON) as the size parameter? Or is there something I'm misunderstanding? Or if this parameter is being ignored, should we just pass 0 instead to make it less confusing?

Originally created by @njsmith on GitHub (Jun 28, 2020). Originally assigned to: @DHowett on GitHub. In all the examples I see for `PROC_THREAD_ATTRIBUTE_PSEUDOCONSOLE`, when calling `UpdateProcThreadAttribute`, it passes the `lpValue` parameters as an `HPCON` value, and the `cbSize` parameter as `sizeof(HPCON)`. For example: https://github.com/microsoft/terminal/blob/aa1ed0a19c479ce6d7c0087ab4bca95f87e83584/samples/ConPTY/EchoCon/EchoCon/EchoCon.cpp#L147-L157 But... this doesn't make any sense to me. `HPCON` is a pointer to a `PseudoConsole` object. And I thought that when calling `UpdateProcThreadAttribute`, the `cbSize` parameter was supposed to be set to the size of the pointed-to object. For example, the [docs say](https://docs.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-updateprocthreadattribute): > `lpValue` > A pointer to the attribute value. This value should persist until the attribute is destroyed using the `DeleteProcThreadAttributeList` function. > > `cbSize` > The size of the attribute value specified by the `lpValue` parameter. And the examples on that page agree – e.g. when demonstrating `PROC_THREAD_ATTRIBUTE_PROTECTION_LEVEL`, they pass `&ProtectionLevel` and `sizeof(ProtectionLevel)`. But in the ConPTY examples and docs, we don't pass the size of the pointed-to pseudoconsole object. Instead, we're setting `cbSize` to the size of a pointer, so 4 or 8 depending on whether this is a 32- or 64-bit build. Shouldn't we be passing `sizeof(*HPCON)` as the size parameter? Or is there something I'm misunderstanding? Or if this parameter is being ignored, should we just pass 0 instead to make it less confusing?
claunia added the Issue-QuestionNeeds-Tag-FixResolution-Answered labels 2026-01-31 01:51:19 +00:00
claunia commented 2026-01-31 01:51:20 +00:00
Author
Owner
Copy Link

@DHowett commented on GitHub (Jun 28, 2020):

This is very possibly something that we messed up when authoring the attribute. I’ll look into the implementation in CreateProcess on Monday.

Thanks for bringing this up!

@DHowett commented on GitHub (Jun 28, 2020): This is very possibly something that we messed up when authoring the attribute. I’ll look into the implementation in `CreateProcess` on Monday. Thanks for bringing this up!
claunia commented 2026-01-31 01:51:20 +00:00
Author
Owner
Copy Link

@eryksun commented on GitHub (Jun 29, 2020):

HPCON is a void * typedef. Apparently it points to an array of 3 handles: the signal pipe, the "\Device\ConDrv\Reference" file, and the conhost.exe process. It seems likely to me that sizeof(HPCON) works because PROC_THREAD_ATTRIBUTE_PSEUDOCONSOLE is only passing the pointer, and the code that handles the attribute probably only cares about the "\Device\ConDrv\Reference" file that references the console session. The "Reference" handle is all a client process needs to connect to the console session. The pseudoconsole attribute probably just replaces the normal inheritance of that handle from the parent process.

@eryksun commented on GitHub (Jun 29, 2020): `HPCON` is a `void *` typedef. Apparently it points to an array of 3 handles: the signal pipe, the "\Device\ConDrv\Reference" file, and the conhost.exe process. It seems likely to me that `sizeof(HPCON)` works because `PROC_THREAD_ATTRIBUTE_PSEUDOCONSOLE` is only passing the pointer, and the code that handles the attribute probably only cares about the "\Device\ConDrv\Reference" file that references the console session. The "Reference" handle is all a client process needs to connect to the console session. The pseudoconsole attribute probably just replaces the normal inheritance of that handle from the parent process.
claunia commented 2026-01-31 01:51:20 +00:00
Author
Owner
Copy Link

@DHowett commented on GitHub (Jun 29, 2020):

So, yes, Eryk is correct here. All we're doing is unpacking the first member from the HPCON, so that's all we care about. We definitely didn't do this right, but it is semantically valid and it is now a compatibility concern to change ☹️

It should have been &hpc, sizeof(hpc).

@DHowett commented on GitHub (Jun 29, 2020): So, yes, Eryk is correct here. All we're doing is unpacking the first member from the HPCON, so that's all we care about. We _definitely_ didn't do this right, but it is semantically valid and it is now a compatibility concern to change ☹️ It should have been `&hpc, sizeof(hpc)`.
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
⛺ Reserved
A11yCO
A11yMAS
A11ySev1
A11ySev2
A11ySev3
A11yTTValidated
A11yUsable
A11yVoiceAccess
A11yWCAG
Area-Accessibility
Area-AtlasEngine
Area-AzureShell
Area-Build
Area-Build
Area-Chat
Area-CmdPal
Area-CodeHealth
Area-Commandline
Area-CookedRead
Area-DefApp
Area-Extensibility
Area-Fonts
Area-GroupPolicy
Area-i18n
Area-Input
Area-Interaction
Area-Interop
Area-Localization
Area-Output
Area-Performance
Area-Portable
Area-Quality
Area-Remoting
Area-Rendering
Area-Schema
Area-Server
Area-Settings
Area-SettingsUI
Area-ShellExtension
Area-ShellExtension
Area-ShellExtension
Area-Suggestions
Area-Suggestions
Area-TerminalConnection
Area-TerminalControl
Area-Theming
Area-UserInterface
Area-VT
Area-Windowing
Area-WPFControl
AutoMerge
Blocking-Ingestion
Culprit-Centennial
Culprit-WinUI
Disability-All
Disability-Blind
Disability-LowVision
Disability-Mobility
External-Blocked-WinUI3
Fixed
Gathering-Data
good first issue
HCL-E+D
HCL-WindowsTerminal
Help Wanted
Impact-Compatibility
Impact-Compliance
Impact-Correctness
Impact-Visual
In-PR
InclusionBacklog
InclusionBacklog-Windows TerminalWin32
InclusionCommitted-202206
Issue-Bug
Issue-Docs
Issue-Feature
Issue-Feature
Issue-Question
Issue-Samples
Issue-Scenario
Issue-Task
Needs-Attention
Needs-Author-Feedback
Needs-Bisect
Needs-Discussion
Needs-Repro
Needs-Tag-Fix
Needs-Tag-Fix
Needs-Triage
No-Recent-Activity
Priority-0
Priority-1
Priority-2
Priority-3
Product-Cmd.exe
Product-Colortool
Product-Colortool
Product-Colortool
Product-Conhost
Product-Conpty
Product-Meta
Product-Powershell
Product-Terminal
Product-WSL
pull-request
Mirrored from GitHub Pull Request
 Resolution-Answered
Resolution-By-Design
Resolution-Duplicate
Resolution-External
Resolution-Fix-Available
Resolution-Fix-Committed
Resolution-No-Repro
Resolution-Won't-Fix
Severity-Blocking
Severity-Crash
Severity-DataLoss
spam
this-will-be-a-breaking-change
Tracking-External
WindowsTerminal_Win32
Work-Item
zAskModeBug
zInbox-Bug
No Label Issue-Question Needs-Tag-Fix Resolution-Answered
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
claunia
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: starred/terminal#9307
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?