[PATCH v2 0/2] MR9500: In CreateProcess(), set std handle when passing a pseudo-console.
This MR tests and fix the std handles in child process when using CreateProcess with extended information and passing a pseudo-console handle.
From the added tests, and other manual testing (see linked bugzilla entry), it turns out native overrides the (bound) std handles from the parent console with handles on the passed pseudo-console for the child process. But happily inherits other kind of handles.
As side notes: - resetting the std handle in RTL_USER_PROCESS_PARAMETERS being console handles could be optimized: potentially, native could just test if the passed handles are bound to the passed console and reset them if not. - hijacked an unused bit in ConsoleFlags to discriminate when to recreate the std handles in child (didn't check-out how native does it though). -- v2: kernelbase: Create std handles from passed pseudo-console. kernel32/tests: Test std handles in CreateProcess with pseudo console. https://gitlab.winehq.org/wine/wine/-/merge_requests/9500
From: Eric Pouech <epouech(a)codeweavers.com> Signed-off-by: Eric Pouech <epouech(a)codeweavers.com> --- dlls/kernel32/tests/process.c | 110 +++++++++++++++++++++++++++++++--- 1 file changed, 102 insertions(+), 8 deletions(-) diff --git a/dlls/kernel32/tests/process.c b/dlls/kernel32/tests/process.c index 948b9bfd709..51733bc5f5d 100644 --- a/dlls/kernel32/tests/process.c +++ b/dlls/kernel32/tests/process.c @@ -3420,6 +3420,42 @@ static BOOL build_startupinfo( STARTUPINFOA *startup, unsigned args, HANDLE hstd return needs_close; } +static BOOL build_startupinfoex( STARTUPINFOEXA *startup, unsigned args, HANDLE hstd[4], HPCON *pseudo_console ) +{ + HANDLE output_read, input_write; + HANDLE output_write, input_read; + SIZE_T attr_size; + COORD size = {80, 32}; + HRESULT hres; + BOOL ret, needs_close; + + ret = CreatePipe(&input_read, &input_write, NULL, 0); + ok(ret, "Couldn't create pipe\n"); + ret = CreatePipe(&output_read, &output_write, NULL, 0); + ok(ret, "Couldn't create pipe\n"); + + hres = CreatePseudoConsole(size, input_read, output_write, 0, pseudo_console); + ok(hres == S_OK, "CreatePseudoConsole failed: %08lx\n", hres); + + memset(startup, 0, sizeof(*startup)); + needs_close = build_startupinfo(&startup->StartupInfo, args, hstd); + startup->StartupInfo.cb = sizeof(*startup); + + InitializeProcThreadAttributeList(NULL, 1, 0, &attr_size); + startup->lpAttributeList = HeapAlloc(GetProcessHeap(), 0, attr_size); + InitializeProcThreadAttributeList(startup->lpAttributeList, 1, 0, &attr_size); + UpdateProcThreadAttribute(startup->lpAttributeList, 0, PROC_THREAD_ATTRIBUTE_PSEUDOCONSOLE, *pseudo_console, + sizeof(*pseudo_console), NULL, NULL); + + hstd[2] = output_read; + hstd[3] = input_write; + + CloseHandle(output_write); + CloseHandle(input_read); + + return needs_close; +} + struct std_handle_test { /* input */ @@ -3515,6 +3551,41 @@ static void test_StdHandleInheritance(void) /* all others handles type behave as H_DISK */ {ARG_STD | ARG_CP_INHERIT | ARG_HANDLE_INHERIT | H_DISK, HATTR_NULL}, {ARG_STARTUPINFO | ARG_CP_INHERIT | ARG_HANDLE_INHERIT | H_DISK, HATTR_TYPE | HATTR_INHERIT | FILE_TYPE_DISK}, + }, + pseudoconsole_cui[] = + { + /* all others handles type behave as H_DISK */ + {ARG_STARTUPINFO | ARG_CP_INHERIT | ARG_HANDLE_INHERIT | H_DISK, HATTR_TYPE | HATTR_INHERIT | FILE_TYPE_DISK}, + {ARG_STD | ARG_CP_INHERIT | ARG_HANDLE_INHERIT | H_DISK, HATTR_TYPE | HATTR_INHERIT | FILE_TYPE_DISK}, + + /* all others handles type behave as H_DISK */ + {ARG_STARTUPINFO | ARG_HANDLE_INHERIT | H_DISK, HATTR_TYPE | HATTR_INHERIT | FILE_TYPE_CHAR}, + {ARG_STD | ARG_HANDLE_INHERIT | H_DISK, HATTR_TYPE | HATTR_INHERIT | FILE_TYPE_DISK}, + + /* all others handles type behave as H_DISK (except H_CONSOLE) */ + {ARG_STARTUPINFO | H_DISK, HATTR_TYPE | HATTR_INHERIT | FILE_TYPE_CHAR}, + {ARG_STD | H_DISK, HATTR_TYPE | FILE_TYPE_DISK}, + + /* all others handles type behave as H_DISK (except H_CONSOLE) */ + {ARG_STARTUPINFO | ARG_HANDLE_PROTECT | H_DISK, HATTR_TYPE | HATTR_INHERIT | FILE_TYPE_CHAR}, + {ARG_STD | ARG_HANDLE_PROTECT | H_DISK, HATTR_TYPE | HATTR_PROTECT | FILE_TYPE_DISK}, + + /* all others handles type behave as H_DISK */ + {ARG_STARTUPINFO | ARG_CP_INHERIT | H_DISK, HATTR_DANGLING, .is_broken = HATTR_TYPE | FILE_TYPE_UNKNOWN}, + {ARG_STD | ARG_CP_INHERIT | H_DISK, HATTR_DANGLING}, + + {ARG_STARTUPINFO | H_DEVIL, HATTR_TYPE | HATTR_INHERIT | FILE_TYPE_CHAR}, + {ARG_STD | H_DEVIL, HATTR_TYPE | HATTR_INHERIT | FILE_TYPE_CHAR}, + {ARG_STARTUPINFO | H_INVALID, HATTR_TYPE | HATTR_INHERIT | FILE_TYPE_CHAR}, + {ARG_STD | H_INVALID, HATTR_TYPE | HATTR_INHERIT | FILE_TYPE_CHAR}, + {ARG_STARTUPINFO | H_NULL, HATTR_TYPE | HATTR_INHERIT | FILE_TYPE_CHAR}, + {ARG_STD | H_NULL, HATTR_TYPE | HATTR_INHERIT | FILE_TYPE_CHAR}, + + /* looks like parent handle is not inherited */ + {ARG_STARTUPINFO | H_CONSOLE, HATTR_TYPE | HATTR_INHERIT | FILE_TYPE_CHAR}, + {ARG_STD | H_CONSOLE, HATTR_TYPE | HATTR_INHERIT | FILE_TYPE_CHAR}, + {ARG_STARTUPINFO | ARG_HANDLE_PROTECT | H_CONSOLE, HATTR_TYPE | HATTR_INHERIT | FILE_TYPE_CHAR}, + {ARG_STD | ARG_HANDLE_PROTECT | H_CONSOLE, HATTR_TYPE | HATTR_INHERIT | FILE_TYPE_CHAR}, }; static const struct { @@ -3527,10 +3598,13 @@ static void test_StdHandleInheritance(void) tests[] = { #define X(d, cg, s) {(d), (cg), s, ARRAY_SIZE(s), #s} - X(0, TRUE, nothing_cui), - X(0, FALSE, nothing_gui), - X(DETACHED_PROCESS, TRUE, detached_cui), - X(DETACHED_PROCESS, FALSE, detached_gui), + X(0, TRUE, nothing_cui), + X(0, FALSE, nothing_gui), + X(DETACHED_PROCESS, TRUE, detached_cui), + X(DETACHED_PROCESS, FALSE, detached_gui), + X(EXTENDED_STARTUPINFO_PRESENT, TRUE, pseudoconsole_cui), + X(EXTENDED_STARTUPINFO_PRESENT | DETACHED_PROCESS, TRUE, detached_cui), + /* not testing gui + pseudo console, behaves like without pseudo console */ #undef X }; @@ -3554,16 +3628,20 @@ static void test_StdHandleInheritance(void) for (i = 0; i < tests[j].count; i++) { - STARTUPINFOA startup; - HANDLE hstd[2] = {}; + STARTUPINFOEXA startup; + HPCON pseudo_console; + HANDLE hstd[4] = {}; BOOL needs_close; winetest_push_context("%s[%u] ", tests[j].descr, i); - needs_close = build_startupinfo( &startup, std_tests[i].args, hstd ); + if (tests[j].cp_flags & EXTENDED_STARTUPINFO_PRESENT) + needs_close = build_startupinfoex( &startup, std_tests[i].args, hstd, &pseudo_console ); + else + needs_close = build_startupinfo( &startup.StartupInfo, std_tests[i].args, hstd ); ret = check_run_child(tests[j].use_cui ? cuiexec : guiexec, tests[j].cp_flags, !!(std_tests[i].args & ARG_CP_INHERIT), - &startup); + &startup.StartupInfo); ok(ret, "Couldn't run child\n"); reload_child_info(resfile); @@ -3591,16 +3669,25 @@ static void test_StdHandleInheritance(void) { unsigned startup_expected = (std_tests[i].args & ARG_STD) ? HATTR_INVALID : std_tests[i].expected; + todo_wine_if(j == 4 && (i == 2 || i == 4 || i == 6 || i == 10 || i == 12 || i == 14 || i == 16 || i == 18)) + { okChildHexInt("StartupInfoA", "hStdInputEncode", startup_expected, std_tests[i].is_broken); okChildHexInt("StartupInfoA", "hStdOutputEncode", startup_expected, std_tests[i].is_broken); + } startup_expected = (std_tests[i].args & ARG_STD) ? HATTR_UNTOUCHED : std_tests[i].expected; + todo_wine_if(j == 4 && (i == 2 || i == 4 || i == 6 || i == 10 || i == 12 || i == 14 || i == 16 || i == 18)) + { okChildHexInt("StartupInfoW", "hStdInputEncode", startup_expected, std_tests[i].is_broken); okChildHexInt("StartupInfoW", "hStdOutputEncode", startup_expected, std_tests[i].is_broken); + } + todo_wine_if(j == 4 && (i == 2 || i == 4 || i == 6 || (i >= 10 && i <= 19))) + { okChildHexInt("TEB", "hStdInputEncode", std_tests[i].expected, std_tests[i].is_broken); okChildHexInt("TEB", "hStdOutputEncode", std_tests[i].expected, std_tests[i].is_broken); + } } release_memory(); @@ -3612,6 +3699,13 @@ static void test_StdHandleInheritance(void) SetHandleInformation(hstd[1], HANDLE_FLAG_PROTECT_FROM_CLOSE, 0); CloseHandle(hstd[1]); } + if (tests[j].cp_flags & EXTENDED_STARTUPINFO_PRESENT) + { + HeapFree(GetProcessHeap(), 0, startup.lpAttributeList); + ClosePseudoConsole(pseudo_console); + CloseHandle(hstd[2]); + CloseHandle(hstd[3]); + } winetest_pop_context(); } } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9500
From: Eric Pouech <epouech(a)codeweavers.com> Partial fix for Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=58967 Signed-off-by: Eric Pouech <epouech(a)codeweavers.com> --- dlls/kernel32/tests/process.c | 9 --------- dlls/kernelbase/console.c | 7 +++++++ dlls/kernelbase/process.c | 12 ++++++++++++ 3 files changed, 19 insertions(+), 9 deletions(-) diff --git a/dlls/kernel32/tests/process.c b/dlls/kernel32/tests/process.c index 51733bc5f5d..56b152a7f31 100644 --- a/dlls/kernel32/tests/process.c +++ b/dlls/kernel32/tests/process.c @@ -3669,25 +3669,16 @@ static void test_StdHandleInheritance(void) { unsigned startup_expected = (std_tests[i].args & ARG_STD) ? HATTR_INVALID : std_tests[i].expected; - todo_wine_if(j == 4 && (i == 2 || i == 4 || i == 6 || i == 10 || i == 12 || i == 14 || i == 16 || i == 18)) - { okChildHexInt("StartupInfoA", "hStdInputEncode", startup_expected, std_tests[i].is_broken); okChildHexInt("StartupInfoA", "hStdOutputEncode", startup_expected, std_tests[i].is_broken); - } startup_expected = (std_tests[i].args & ARG_STD) ? HATTR_UNTOUCHED : std_tests[i].expected; - todo_wine_if(j == 4 && (i == 2 || i == 4 || i == 6 || i == 10 || i == 12 || i == 14 || i == 16 || i == 18)) - { okChildHexInt("StartupInfoW", "hStdInputEncode", startup_expected, std_tests[i].is_broken); okChildHexInt("StartupInfoW", "hStdOutputEncode", startup_expected, std_tests[i].is_broken); - } - todo_wine_if(j == 4 && (i == 2 || i == 4 || i == 6 || (i >= 10 && i <= 19))) - { okChildHexInt("TEB", "hStdInputEncode", std_tests[i].expected, std_tests[i].is_broken); okChildHexInt("TEB", "hStdOutputEncode", std_tests[i].expected, std_tests[i].is_broken); - } } release_memory(); diff --git a/dlls/kernelbase/console.c b/dlls/kernelbase/console.c index 83bec6c9878..4a0f96076b5 100644 --- a/dlls/kernelbase/console.c +++ b/dlls/kernelbase/console.c @@ -2390,5 +2390,12 @@ void init_console( void ) alloc_console( no_window ); } else if (params->ConsoleHandle && params->ConsoleHandle != CONSOLE_HANDLE_SHELL_NO_WINDOW) + { create_console_connection( params->ConsoleHandle ); + if (params->ConsoleFlags & 2) + { + init_console_std_handles( FALSE ); + params->ConsoleFlags &= ~2; + } + } } diff --git a/dlls/kernelbase/process.c b/dlls/kernelbase/process.c index d89b2f1405a..92c9fd511d8 100644 --- a/dlls/kernelbase/process.c +++ b/dlls/kernelbase/process.c @@ -606,6 +606,18 @@ BOOL WINAPI DECLSPEC_HOTPATCH CreateProcessInternalW( HANDLE token, const WCHAR TRACE( "PROC_THREAD_ATTRIBUTE_PSEUDOCONSOLE %p reference %p\n", console, console->reference ); params->ConsoleHandle = console->reference; + if (!(flags & DETACHED_PROCESS)) + { + params->ConsoleFlags |= 2; + /* don't inherit standard handles bound to parent console (but inherit the others) */ + if (!(startup_info->dwFlags & STARTF_USESTDHANDLES)) + { + DWORD mode; + if (GetConsoleMode(params->hStdInput, &mode)) params->hStdInput = NULL; + if (GetConsoleMode(params->hStdOutput, &mode)) params->hStdOutput = NULL; + if (GetConsoleMode(params->hStdError, &mode)) params->hStdError = NULL; + } + } break; } case PROC_THREAD_ATTRIBUTE_JOB_LIST: -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9500
On Tue Nov 18 08:08:24 2025 +0000, eric pouech wrote:
changed this line in [version 2 of the diff](/wine/wine/-/merge_requests/9500/diffs?diff_id=225069&start_sha=61c19b037e6abcca665b55c59c256d84d1288af6#5039e127257ddb528e569299a8dd9c5f204f5083_3426_3425) V2 pushed
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/9500#note_122727
Jacek Caban (@jacek) commented about dlls/kernelbase/process.c:
TRACE( "PROC_THREAD_ATTRIBUTE_PSEUDOCONSOLE %p reference %p\n", console, console->reference ); params->ConsoleHandle = console->reference; + if (!(flags & DETACHED_PROCESS)) + { + params->ConsoleFlags |= 2; + /* don't inherit standard handles bound to parent console (but inherit the others) */ + if (!(startup_info->dwFlags & STARTF_USESTDHANDLES)) + { + DWORD mode; + if (GetConsoleMode(params->hStdInput, &mode)) params->hStdInput = NULL; + if (GetConsoleMode(params->hStdOutput, &mode)) params->hStdOutput = NULL; + if (GetConsoleMode(params->hStdError, &mode)) params->hStdError = NULL;
For non-console handles this will set last error. I’m not sure if that’s a problem, but https://gitlab.winehq.org/jacek/wine/-/commit/32e39c819be6a2a2a0c03047175cb8... also passes the tests. It’s not exactly the same, though, what do you think? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9500#note_122762
participants (3)
-
Eric Pouech -
eric pouech (＠epo)
-
Jacek Caban (＠jacek)