[PATCH v2 0/1] MR3022: kernelbase: In AllocConsole(), dispose existing unix console.
Unix consoles (created from initial process) are inherited by child processes for various reasons, like keeping std handles bound to that unix console. This differs from Windows as the default for a GUI is not to have console attached. If a GUI programs asks for a console, it will succeed on Windows but fail under Wine as the Unix console is still present. So, allow AllocConsole() to succeed when called from a GUI program and tied to a Unix console. (don't do it for CUI as they are already attached to a console). This fixes Scrap Mechanic when run with '-dev' option. (based on suggestion from Zhiyi Zhang) Signed-off-by: Eric Pouech <epouech(a)codeweavers.com> -- v2: kernelbase: Only inherit consoles for CUI processes. https://gitlab.winehq.org/wine/wine/-/merge_requests/3022
From: Jacek Caban <jacek(a)codeweavers.com> Signed-off-by: Eric Pouech <epouech(a)codeweavers.com> --- dlls/kernel32/tests/console.c | 2 +- dlls/kernelbase/console.c | 11 ++++++++--- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/dlls/kernel32/tests/console.c b/dlls/kernel32/tests/console.c index 24c1e5c69fd..4e164c11b3d 100644 --- a/dlls/kernel32/tests/console.c +++ b/dlls/kernel32/tests/console.c @@ -4951,7 +4951,7 @@ static void test_CreateProcessCUI(void) }, with_console_tests[] = { -/* 0*/ {FALSE, 0, NULL_STD, 0, TRUE}, +/* 0*/ {FALSE, 0, NULL_STD, 0}, {FALSE, DETACHED_PROCESS, NULL_STD, 0}, {FALSE, CREATE_NEW_CONSOLE, NULL_STD, 0}, {FALSE, CREATE_NO_WINDOW, NULL_STD, 0}, diff --git a/dlls/kernelbase/console.c b/dlls/kernelbase/console.c index 831e358c692..7745ee7ee14 100644 --- a/dlls/kernelbase/console.c +++ b/dlls/kernelbase/console.c @@ -2344,14 +2344,19 @@ void init_console( void ) init_console_std_handles( FALSE ); } } + else if (RtlImageNtHeader( GetModuleHandleW( NULL ))->OptionalHeader.Subsystem != + IMAGE_SUBSYSTEM_WINDOWS_CUI) + { + /* don't try to close pseudo handles */ + if ((LONG_PTR)params->ConsoleHandle > 0) CloseHandle(params->ConsoleHandle); + params->ConsoleHandle = NULL; + } else if (params->ConsoleHandle == CONSOLE_HANDLE_ALLOC || params->ConsoleHandle == CONSOLE_HANDLE_ALLOC_NO_WINDOW) { BOOL no_window = params->ConsoleHandle == CONSOLE_HANDLE_ALLOC_NO_WINDOW; - HMODULE mod = GetModuleHandleW( NULL ); params->ConsoleHandle = NULL; - if (RtlImageNtHeader( mod )->OptionalHeader.Subsystem == IMAGE_SUBSYSTEM_WINDOWS_CUI) - alloc_console( no_window ); + alloc_console( no_window ); } else if (params->ConsoleHandle && params->ConsoleHandle != CONSOLE_HANDLE_SHELL_NO_WINDOW) create_console_connection( params->ConsoleHandle ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3022
amended Jacek's proposal: - close passed console handle - adapt tests -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3022#note_36429
Looking closer at this, maybe we should just do that in the parent process. What do you think about something like this? https://gitlab.winehq.org/jacek/wine/-/commit/02faf858eb13d0110cd45cc906614d... -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3022#note_36477
On Wed Jun 21 12:09:40 2023 +0000, Jacek Caban wrote:
Looking closer at this, maybe we should just do that in the parent process. What do you think about something like this? https://gitlab.winehq.org/jacek/wine/-/commit/02faf858eb13d0110cd45cc906614d... yes for moving it into the parent!
but I wonder if this shouldn't rather be done in kernelbase (instead of ntdll) - adding also the creation of the ad hoc console handles when requested from CreateProces flags, and simply having ntdll pass the console handle around (this would also get rid of the pseudo consoles CONSOLE_ALLOC*) - this would look more symmetrical (everything in kernelbase) and ntdll transparent to it (except for unix console inheritance of course) -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3022#note_36481
On Wed Jun 21 12:09:39 2023 +0000, eric pouech wrote:
yes for moving it into the parent! but I wonder if this shouldn't rather be done in kernelbase (instead of ntdll) - adding also the creation of the ad hoc console handles when requested from CreateProces flags, and simply having ntdll pass the console handle around (this would also get rid of the pseudo consoles CONSOLE_ALLOC*) - this would look more symmetrical (everything in kernelbase) and ntdll transparent to it (except for unix console inheritance of course) We don't know if the child will be CUI until its executable is mapped and that's done in ntdll.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/3022#note_36493
On Wed Jun 21 12:55:05 2023 +0000, Jacek Caban wrote:
We don't know if the child will be CUI until its executable is mapped and that's done in ntdll. what's next step? you post your patch and I close this MR?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/3022#note_36585
On Thu Jun 22 15:13:44 2023 +0000, eric pouech wrote:
what's next step? you post your patch and I close this MR? Sounds good to me, I created !3145.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/3022#note_36642
This merge request was closed by eric pouech. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3022
closing superseded by MR!3145 -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3022#note_36653
participants (3)
-
eric pouech (＠epo) -
Jacek Caban
-
Jacek Caban (＠jacek)