[PATCH 0/3] MR8136: Fix interaction between FreeConsole() and CRT std streams.
The Aim of this MR is to fix https://bugs.winehq.org/show_bug.cgi?id=10941 In a nutshell: - CUI app starts, and CRT inits its std streams from console, - app calls FreeConsole() then AllocConsole() and expects the CRT I/O functions to be able to output to newly created console. It fails: - when run with wineconsole, as the inherited std consoles are closed in FreeConsole(). Added tests, but current implementation of kernelbase is correct. So, the fix is to force wineconsole to pass unbound console handles with standard inheritance (to they are not closed), - when run from unix console, the std handles were also closed. There's no unbound console in that case. The fix is not to close these handles in FreeConsole(). - the other cases from the bug entry work because cmd was created with unbound console handles. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8136
From: Eric Pouech <epouech(a)codeweavers.com> Signed-off-by: Eric Pouech <epouech(a)codeweavers.com> --- dlls/kernel32/tests/console.c | 118 ++++++++++++++++++++++++++++++++++ 1 file changed, 118 insertions(+) diff --git a/dlls/kernel32/tests/console.c b/dlls/kernel32/tests/console.c index 398b818776a..744482e40c1 100644 --- a/dlls/kernel32/tests/console.c +++ b/dlls/kernel32/tests/console.c @@ -5616,6 +5616,117 @@ static void test_CtrlHandlerSubsystem(void) DeleteFileA(cuiexec); } +#define CFC_VALID_STD_HANDLE 0x01 +#define CFC_CLOSED_BY_FREECONSOLE 0x02 +#define CFC_SAME_OBJECTS 0x04 + +static void test_child_free_console(void) +{ + HANDLE std, std_clone = NULL; + unsigned exit_code = 0; + DWORD dw; + + std = GetStdHandle(STD_ERROR_HANDLE); + if (GetHandleInformation(std, &dw)) + exit_code |= CFC_VALID_STD_HANDLE; + if (std && std != INVALID_HANDLE_VALUE && + !DuplicateHandle(GetCurrentProcess(), std, GetCurrentProcess(), &std_clone, 0, FALSE, DUPLICATE_SAME_ACCESS)) + exit_code |= 0xff; + FreeConsole(); + if ((exit_code & CFC_VALID_STD_HANDLE) && !GetHandleInformation(std, &dw)) + { + exit_code |= CFC_CLOSED_BY_FREECONSOLE; + SetStdHandle(STD_ERROR_HANDLE, NULL); /* so that we can grab the new std handle set by AllocConsole() */ + } + AllocConsole(); /* need a new console so that the unbound handles point to something */ + + if (std && std != INVALID_HANDLE_VALUE && !NtCompareObjects(std_clone, GetStdHandle(STD_ERROR_HANDLE))) + exit_code |= CFC_SAME_OBJECTS; + + CloseHandle(std_clone); + ExitProcess(exit_code); +} + +static void test_FreeConsoleStd(void) +{ + char buf[MAX_PATH], **argv; + HANDLE saved_std; + unsigned i; + BOOL ret; + STARTUPINFOA si = {sizeof(si)}; + PROCESS_INFORMATION info; + DWORD status; + + static const struct + { + DWORD cp_flags; + enum {with_global, with_startupinfo, with_none} with; + unsigned expected_bits; + } + tests[] = + { + {0, with_global, CFC_VALID_STD_HANDLE}, + {0, with_startupinfo, CFC_VALID_STD_HANDLE | CFC_SAME_OBJECTS}, + {CREATE_NEW_CONSOLE, with_global, CFC_VALID_STD_HANDLE | CFC_CLOSED_BY_FREECONSOLE}, + {CREATE_NEW_CONSOLE, with_startupinfo, CFC_VALID_STD_HANDLE | CFC_SAME_OBJECTS}, + {CREATE_NEW_CONSOLE, with_none, CFC_VALID_STD_HANDLE | CFC_CLOSED_BY_FREECONSOLE}, + {DETACHED_PROCESS, with_global, 0}, + {DETACHED_PROCESS, with_startupinfo, CFC_VALID_STD_HANDLE | CFC_SAME_OBJECTS}, + {DETACHED_PROCESS, with_none, 0}, + }; + + if (!pVerifyConsoleIoHandle) + { + win_skip("Can't run test\n"); + return; + } + si.hStdInput = si.hStdOutput = INVALID_HANDLE_VALUE; + winetest_get_mainargs(&argv); + for (i = 0; i < ARRAY_SIZE(tests); i++) + { + winetest_push_context("test[%u]", i); + sprintf(buf, "\"%s\" console free_console", argv[0]); + switch (tests[i].with) + { + case with_global: + si.dwFlags &= ~STARTF_USESTDHANDLES; + saved_std = GetStdHandle(STD_ERROR_HANDLE); + SetStdHandle(STD_ERROR_HANDLE, create_unbound_handle(TRUE, TRUE)); + break; + case with_startupinfo: + si.dwFlags |= STARTF_USESTDHANDLES; + si.hStdError = create_unbound_handle(TRUE, TRUE); + break; + case with_none: + si.dwFlags &= ~STARTF_USESTDHANDLES; + break; + } + + ret = CreateProcessA(NULL, buf, NULL, NULL, TRUE, tests[i].cp_flags, NULL, NULL, &si, &info); + ok(ret, "got error %lu\n", GetLastError()); + status = WaitForSingleObject(info.hProcess, INFINITE); + ok(status == WAIT_OBJECT_0, "Unexpected wait status\n"); + ret = GetExitCodeProcess(info.hProcess, &status); + ok(status == tests[i].expected_bits, "Expected %x but got %lx\n", tests[i].expected_bits, status); + switch (tests[i].with) + { + case with_global: + CloseHandle(GetStdHandle(STD_ERROR_HANDLE)); + SetStdHandle(STD_ERROR_HANDLE, saved_std); + break; + case with_startupinfo: + CloseHandle(si.hStdError); + break; + case with_none: + break; + } + + CloseHandle(info.hProcess); + CloseHandle(info.hThread); + winetest_pop_context(); + } +} + START_TEST(console) { HANDLE hConIn, hConOut, revert_output = NULL, unbound_output; @@ -5717,6 +5828,12 @@ START_TEST(console) return; } + if (argc >= 3 && !strcmp(argv[2], "free_console")) + { + test_child_free_console(); + return; + } + test_current = argc >= 3 && !strcmp(argv[2], "--current"); using_pseudo_console = argc >= 3 && !strcmp(argv[2], "--pseudo-console"); @@ -5906,6 +6023,7 @@ START_TEST(console) test_console_as_root_directory(); if (!test_current) { + test_FreeConsoleStd(); test_pseudo_console(); test_AttachConsole(hConOut); test_AllocConsole(); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8136
From: Eric Pouech <epouech(a)codeweavers.com> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=10941 Signed-off-by: Eric Pouech <epouech(a)codeweavers.com> --- programs/wineconsole/wineconsole.c | 41 +++++++++++++++++------------- 1 file changed, 24 insertions(+), 17 deletions(-) diff --git a/programs/wineconsole/wineconsole.c b/programs/wineconsole/wineconsole.c index aeeee11aa86..ab3e5cdade8 100644 --- a/programs/wineconsole/wineconsole.c +++ b/programs/wineconsole/wineconsole.c @@ -25,6 +25,7 @@ #include "windef.h" #include "winbase.h" #include "wincon.h" +#include "winternl.h" #include "wineconsole_res.h" @@ -32,6 +33,18 @@ WINE_DEFAULT_DEBUG_CHANNEL(console); +static BOOL setup_target_console(void) +{ + if (!FreeConsole()) return FALSE; + /* Zero out std handles so that AllocConsole() sets the newly allocated handles as std, + * and will be inherited by child process. + */ + SetStdHandle(STD_INPUT_HANDLE, NULL); + SetStdHandle(STD_OUTPUT_HANDLE, NULL); + SetStdHandle(STD_ERROR_HANDLE, NULL); + return AllocConsole(); +} + int WINAPI wWinMain( HINSTANCE inst, HINSTANCE prev, WCHAR *cmdline, INT show ) { STARTUPINFOW startup = { sizeof(startup) }; @@ -43,9 +56,14 @@ int WINAPI wWinMain( HINSTANCE inst, HINSTANCE prev, WCHAR *cmdline, INT show ) if (!*cmd) cmd = default_cmd; - if (!CreateProcessW( NULL, cmd, NULL, NULL, FALSE, CREATE_NEW_CONSOLE, NULL, NULL, &startup, &info )) + if (!setup_target_console()) + { + ERR( "failed to allocate console: %lu\n", GetLastError() ); + return 1; + } + + if (!CreateProcessW( NULL, cmd, NULL, NULL, FALSE, 0, NULL, NULL, &startup, &info )) { - HANDLE hStdInput, hStdOutput; WCHAR format[256], *buf; INPUT_RECORD ir; DWORD len; @@ -53,30 +71,19 @@ int WINAPI wWinMain( HINSTANCE inst, HINSTANCE prev, WCHAR *cmdline, INT show ) exit_code = GetLastError(); WARN( "CreateProcess '%ls' failed: %lu\n", cmd, exit_code ); - /* create a new console to display error messages in it */ - FreeConsole(); /* make sure we're not connected to any console */ - if (!AllocConsole()) - { - ERR( "failed to allocate console: %lu\n", GetLastError() ); - return 1; - } - - hStdInput = CreateFileW( L"CONIN$", GENERIC_READ | GENERIC_WRITE, 0, NULL, - OPEN_EXISTING, 0, 0 ); - hStdOutput = CreateFileW( L"CONOUT$", GENERIC_READ | GENERIC_WRITE, 0, NULL, - OPEN_EXISTING, 0, 0 ); - LoadStringW( GetModuleHandleW( NULL ), IDS_CMD_LAUNCH_FAILED, format, ARRAY_SIZE(format) ); len = wcslen( format ) + wcslen( cmd ); if ((buf = malloc( len * sizeof(WCHAR) ))) { swprintf( buf, len, format, cmd ); - WriteConsoleW( hStdOutput, buf, wcslen(buf), &len, NULL); - while (ReadConsoleInputW( hStdInput, &ir, 1, &len ) && ir.EventType == MOUSE_EVENT); + WriteConsoleW( startup.hStdOutput, buf, wcslen(buf), &len, NULL); + while (ReadConsoleInputW( startup.hStdInput, &ir, 1, &len ) && ir.EventType == MOUSE_EVENT); } return exit_code; } + /* detach from created console */ + FreeConsole(); CloseHandle( info.hThread ); WaitForSingleObject( info.hProcess, INFINITE ); return GetExitCodeProcess( info.hProcess, &exit_code ) ? exit_code : GetLastError(); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8136
From: Eric Pouech <epouech(a)codeweavers.com> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=10941 Signed-off-by: Eric Pouech <epouech(a)codeweavers.com> --- dlls/kernelbase/console.c | 1 + 1 file changed, 1 insertion(+) diff --git a/dlls/kernelbase/console.c b/dlls/kernelbase/console.c index 8c90eb321df..9d5377800b6 100644 --- a/dlls/kernelbase/console.c +++ b/dlls/kernelbase/console.c @@ -2372,6 +2372,7 @@ void init_console( void ) if (params->ConsoleHandle && create_console_connection( params->ConsoleHandle )) { init_console_std_handles( FALSE ); + console_flags = 0; } } else if (params->ConsoleHandle == CONSOLE_HANDLE_ALLOC || -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8136
participants (2)
-
Eric Pouech -
eric pouech (＠epo)