[PATCH v3 0/5] MR3442: CreateProcess fixes for process group.
This serie: - adds a couple of tests to CreateProcess to cover ctrl-c blocking inheritance flag and process group creation - populates RTL_USER_PROCESS_PARAMETERS.ProcessGroupId - separates new process group creation from ctrl-c inheritance flag -- v3: kernelbase: Correctly handle new process group flag in CreateProcess. kernel32/tests: Extend to new process group tests for CreateProcess. kernelbase,ntdll,server: Separate console and new group flag (CreateProcess). kernel32/tests: Add tests for group and console flags in CreateProcess. kernel32/tests: Only access ProcessGroupId field when present. https://gitlab.winehq.org/wine/wine/-/merge_requests/3442
From: Eric Pouech <epouech(a)codeweavers.com> Signed-off-by: Eric Pouech <epouech(a)codeweavers.com> --- dlls/kernel32/tests/console.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/dlls/kernel32/tests/console.c b/dlls/kernel32/tests/console.c index 4e164c11b3d..931eff6df0b 100644 --- a/dlls/kernel32/tests/console.c +++ b/dlls/kernel32/tests/console.c @@ -5076,7 +5076,10 @@ START_TEST(console) if (IsWindow(GetConsoleWindow())) exit_code |= CP_WITH_WINDOW; if (pGetConsoleProcessList && GetConsoleProcessList(&pcslist, 1) == 1) exit_code |= CP_ALONE; - if (RtlGetCurrentPeb()->ProcessParameters->ProcessGroupId == GetCurrentProcessId()) + if (RtlGetCurrentPeb()->ProcessParameters->Size >= + offsetof(RTL_USER_PROCESS_PARAMETERS, ProcessGroupId) + + sizeof(RtlGetCurrentPeb()->ProcessParameters->ProcessGroupId) && + RtlGetCurrentPeb()->ProcessParameters->ProcessGroupId == GetCurrentProcessId()) exit_code |= CP_GROUP_LEADER; if (GetFileType(GetStdHandle(STD_INPUT_HANDLE)) == FILE_TYPE_CHAR) exit_code |= CP_INPUT_VALID; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3442
From: Eric Pouech <epouech(a)codeweavers.com> Signed-off-by: Eric Pouech <epouech(a)codeweavers.com> --- dlls/kernel32/tests/console.c | 59 +++++++++++++++++++++++++++++++++++ 1 file changed, 59 insertions(+) diff --git a/dlls/kernel32/tests/console.c b/dlls/kernel32/tests/console.c index 931eff6df0b..56dc27f2a94 100644 --- a/dlls/kernel32/tests/console.c +++ b/dlls/kernel32/tests/console.c @@ -769,8 +769,14 @@ static void testCtrlHandler(void) ok(WaitForSingleObject(mch_event, 3000) == WAIT_OBJECT_0, "event sending didn't work\n"); CloseHandle(mch_event); + ok(SetConsoleCtrlHandler(NULL, FALSE), "Couldn't turn on ctrl-c handling\n"); + ok((RtlGetCurrentPeb()->ProcessParameters->ConsoleFlags & 1) == 0, + "Unexpect ConsoleFlags value %lx\n", RtlGetCurrentPeb()->ProcessParameters->ConsoleFlags); + /* Turning off ctrl-c handling doesn't work on win9x such way ... */ ok(SetConsoleCtrlHandler(NULL, TRUE), "Couldn't turn off ctrl-c handling\n"); + ok((RtlGetCurrentPeb()->ProcessParameters->ConsoleFlags & 1) != 0, + "Unexpect ConsoleFlags value %lx\n", RtlGetCurrentPeb()->ProcessParameters->ConsoleFlags); mch_event = CreateEventA(NULL, TRUE, FALSE, NULL); mch_count = 0; if(!(GetVersion() & 0x80000000)) @@ -4911,6 +4917,7 @@ static DWORD check_child_console_bits(const char* exec, DWORD flags, enum inheri #define CP_GROUP_LEADER 0x10 /* whether the child is the process group leader */ #define CP_INPUT_VALID 0x20 /* whether StdHandle(INPUT) is a valid console handle */ #define CP_OUTPUT_VALID 0x40 /* whether StdHandle(OUTPUT) is a valid console handle */ +#define CP_ENABLED_CTRLC 0x80 /* whether the ctrl-c handling isn't blocked */ #define CP_OWN_CONSOLE (CP_WITH_CONSOLE | CP_WITH_HANDLE | CP_INPUT_VALID | CP_OUTPUT_VALID | CP_ALONE) #define CP_INH_CONSOLE (CP_WITH_CONSOLE | CP_WITH_HANDLE | CP_INPUT_VALID | CP_OUTPUT_VALID) @@ -4923,6 +4930,7 @@ static void test_CreateProcessCUI(void) char **argv; BOOL res; int i; + BOOL saved_console_flags; static struct { @@ -4993,6 +5001,36 @@ static void test_CreateProcessCUI(void) {TRUE, DETACHED_PROCESS | CREATE_NO_WINDOW, STARTUPINFO_STD, CP_INPUT_VALID | CP_OUTPUT_VALID, .is_broken = 0x100}, /*35*/ {TRUE, CREATE_NEW_CONSOLE | CREATE_NO_WINDOW, STARTUPINFO_STD, CP_OWN_CONSOLE | CP_WITH_WINDOW, .is_broken = CP_WITH_CONSOLE | CP_WITH_HANDLE | CP_WITH_WINDOW | CP_ALONE}, }; + static struct group_flags_tests + { + /* input */ + BOOL use_cui; + DWORD cp_flags; + enum inheritance_model inherit; + BOOL noctrl_flag; + /* output */ + DWORD expected; + BOOL is_todo; + } + group_flags_tests[] = + { +/* 0 */ {TRUE, 0, CONSOLE_STD, TRUE, CP_INH_CONSOLE | CP_WITH_WINDOW}, + {TRUE, CREATE_NEW_PROCESS_GROUP, CONSOLE_STD, TRUE, CP_INH_CONSOLE | CP_WITH_WINDOW | CP_GROUP_LEADER, .is_todo = TRUE}, + {TRUE, 0, CONSOLE_STD, FALSE, CP_INH_CONSOLE | CP_WITH_WINDOW | CP_ENABLED_CTRLC}, + {TRUE, CREATE_NEW_PROCESS_GROUP, CONSOLE_STD, FALSE, CP_INH_CONSOLE | CP_WITH_WINDOW | CP_GROUP_LEADER, .is_todo = TRUE}, + {TRUE, 0, STARTUPINFO_STD, TRUE, CP_INH_CONSOLE | CP_WITH_WINDOW}, +/* 5 */ {TRUE, CREATE_NEW_PROCESS_GROUP, STARTUPINFO_STD, TRUE, CP_INH_CONSOLE | CP_WITH_WINDOW | CP_GROUP_LEADER, .is_todo = TRUE}, + {TRUE, 0, STARTUPINFO_STD, FALSE, CP_INH_CONSOLE | CP_WITH_WINDOW | CP_ENABLED_CTRLC}, + {TRUE, CREATE_NEW_PROCESS_GROUP, STARTUPINFO_STD, FALSE, CP_INH_CONSOLE | CP_WITH_WINDOW | CP_GROUP_LEADER, .is_todo = TRUE}, + {FALSE, 0, CONSOLE_STD, TRUE, 0, .is_todo = TRUE}, + {FALSE, CREATE_NEW_PROCESS_GROUP, CONSOLE_STD, TRUE, CP_GROUP_LEADER, .is_todo = TRUE}, +/* 10 */ {FALSE, 0, CONSOLE_STD, FALSE, CP_ENABLED_CTRLC, .is_todo = TRUE}, + {FALSE, CREATE_NEW_PROCESS_GROUP, CONSOLE_STD, FALSE, CP_GROUP_LEADER, .is_todo = TRUE}, + {FALSE, 0, STARTUPINFO_STD, TRUE, 0, .is_todo = TRUE}, + {FALSE, CREATE_NEW_PROCESS_GROUP, STARTUPINFO_STD, TRUE, CP_GROUP_LEADER, .is_todo = TRUE}, + {FALSE, 0, STARTUPINFO_STD, FALSE, CP_ENABLED_CTRLC, .is_todo = TRUE}, +/* 15 */ {FALSE, CREATE_NEW_PROCESS_GROUP, STARTUPINFO_STD, FALSE, CP_GROUP_LEADER, .is_todo = TRUE}, + }; hstd[0] = GetStdHandle(STD_INPUT_HANDLE); hstd[1] = GetStdHandle(STD_OUTPUT_HANDLE); @@ -5032,6 +5070,25 @@ static void test_CreateProcessCUI(void) i, res, with_console_tests[i].expected); } + saved_console_flags = RtlGetCurrentPeb()->ProcessParameters->ConsoleFlags; + + for (i = 0; i < ARRAY_SIZE(group_flags_tests); i++) + { + res = SetConsoleCtrlHandler(NULL, group_flags_tests[i].noctrl_flag); + ok(res, "Couldn't set ctrl handler\n"); + res = check_child_console_bits(group_flags_tests[i].use_cui ? cuiexec : guiexec, + group_flags_tests[i].cp_flags, + group_flags_tests[i].inherit); + todo_wine_if(group_flags_tests[i].is_todo) + ok(res == group_flags_tests[i].expected || + /* Win7 doesn't report group id */ + broken(res == (group_flags_tests[i].expected & ~CP_GROUP_LEADER)), + "[%d] Unexpected result %x (%lx)\n", + i, res, group_flags_tests[i].expected); + } + + RtlGetCurrentPeb()->ProcessParameters->ConsoleFlags = saved_console_flags; + DeleteFileA(guiexec); DeleteFileA(cuiexec); @@ -5085,6 +5142,8 @@ START_TEST(console) exit_code |= CP_INPUT_VALID; if (GetFileType(GetStdHandle(STD_OUTPUT_HANDLE)) == FILE_TYPE_CHAR) exit_code |= CP_OUTPUT_VALID; + if (!(RtlGetCurrentPeb()->ProcessParameters->ConsoleFlags & 1)) + exit_code |= CP_ENABLED_CTRLC; ExitProcess(exit_code); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3442
From: Eric Pouech <epouech(a)codeweavers.com> We were using bit 1 of RTL_USER_PROCESS_PARAMETERS for two different cases: - rightfully, as a sign to block ctrl-c events from being processed by handlers (and by default, terminating the process) - But this was also used to request for the creation of a new process group. This patch properly separates the two use cases, by using the ProcessGroupId field in RTL_USER_PROCESS_PARAMETERS (checked that Win10 behaves as this patch in RtlCreateUserProcess wrt. RTL_USER_PROCESS_PARAMETERS ProcessGroupId usage input/output). Introduce process_group_id in startup_info_t and use it to pass it to server. ProcessGroupId field in RTL_USER_PROCESS_PARAMETERS is now properly set. Note: this will change some external behavior. - before this patch, a child process created with Ctrl-C disabled (ConsoleFlags set), couldn't turn it on as the process was detached from unix console. - now, SIGINT handling is moved to kernelbase (and can be turned on/off at application will), - when creating a new windows group id, the child will be detached from unix console, so will no longer receives the SIGINT from ctrl-c in unix console (if parent was attached to this unix console). Signed-off-by: Eric Pouech <epouech(a)codeweavers.com> --- dlls/kernel32/tests/console.c | 8 ++++---- dlls/kernelbase/process.c | 6 +++++- dlls/ntdll/env.c | 1 + dlls/ntdll/unix/env.c | 4 ++++ dlls/ntdll/unix/process.c | 4 ++-- include/wine/server_protocol.h | 3 ++- server/process.c | 5 ++++- server/protocol.def | 1 + server/request.h | 2 +- server/trace.c | 5 +++-- tools/make_requests | 2 +- 11 files changed, 28 insertions(+), 13 deletions(-) diff --git a/dlls/kernel32/tests/console.c b/dlls/kernel32/tests/console.c index 56dc27f2a94..7f66d34dc4c 100644 --- a/dlls/kernel32/tests/console.c +++ b/dlls/kernel32/tests/console.c @@ -5015,13 +5015,13 @@ static void test_CreateProcessCUI(void) group_flags_tests[] = { /* 0 */ {TRUE, 0, CONSOLE_STD, TRUE, CP_INH_CONSOLE | CP_WITH_WINDOW}, - {TRUE, CREATE_NEW_PROCESS_GROUP, CONSOLE_STD, TRUE, CP_INH_CONSOLE | CP_WITH_WINDOW | CP_GROUP_LEADER, .is_todo = TRUE}, + {TRUE, CREATE_NEW_PROCESS_GROUP, CONSOLE_STD, TRUE, CP_INH_CONSOLE | CP_WITH_WINDOW | CP_GROUP_LEADER}, {TRUE, 0, CONSOLE_STD, FALSE, CP_INH_CONSOLE | CP_WITH_WINDOW | CP_ENABLED_CTRLC}, - {TRUE, CREATE_NEW_PROCESS_GROUP, CONSOLE_STD, FALSE, CP_INH_CONSOLE | CP_WITH_WINDOW | CP_GROUP_LEADER, .is_todo = TRUE}, + {TRUE, CREATE_NEW_PROCESS_GROUP, CONSOLE_STD, FALSE, CP_INH_CONSOLE | CP_WITH_WINDOW | CP_GROUP_LEADER}, {TRUE, 0, STARTUPINFO_STD, TRUE, CP_INH_CONSOLE | CP_WITH_WINDOW}, -/* 5 */ {TRUE, CREATE_NEW_PROCESS_GROUP, STARTUPINFO_STD, TRUE, CP_INH_CONSOLE | CP_WITH_WINDOW | CP_GROUP_LEADER, .is_todo = TRUE}, +/* 5 */ {TRUE, CREATE_NEW_PROCESS_GROUP, STARTUPINFO_STD, TRUE, CP_INH_CONSOLE | CP_WITH_WINDOW | CP_GROUP_LEADER}, {TRUE, 0, STARTUPINFO_STD, FALSE, CP_INH_CONSOLE | CP_WITH_WINDOW | CP_ENABLED_CTRLC}, - {TRUE, CREATE_NEW_PROCESS_GROUP, STARTUPINFO_STD, FALSE, CP_INH_CONSOLE | CP_WITH_WINDOW | CP_GROUP_LEADER, .is_todo = TRUE}, + {TRUE, CREATE_NEW_PROCESS_GROUP, STARTUPINFO_STD, FALSE, CP_INH_CONSOLE | CP_WITH_WINDOW | CP_GROUP_LEADER}, {FALSE, 0, CONSOLE_STD, TRUE, 0, .is_todo = TRUE}, {FALSE, CREATE_NEW_PROCESS_GROUP, CONSOLE_STD, TRUE, CP_GROUP_LEADER, .is_todo = TRUE}, /* 10 */ {FALSE, 0, CONSOLE_STD, FALSE, CP_ENABLED_CTRLC, .is_todo = TRUE}, diff --git a/dlls/kernelbase/process.c b/dlls/kernelbase/process.c index 744474710ad..b6775052699 100644 --- a/dlls/kernelbase/process.c +++ b/dlls/kernelbase/process.c @@ -187,7 +187,11 @@ static RTL_USER_PROCESS_PARAMETERS *create_process_params( const WCHAR *filename } RtlFreeUnicodeString( &newdirW ); - if (flags & CREATE_NEW_PROCESS_GROUP) params->ConsoleFlags = 1; + if (flags & CREATE_NEW_PROCESS_GROUP) + params->ConsoleFlags = 1; + else + params->ProcessGroupId = NtCurrentTeb()->Peb->ProcessParameters->ProcessGroupId; + if (flags & CREATE_NEW_CONSOLE) params->ConsoleHandle = CONSOLE_HANDLE_ALLOC; else if (!(flags & DETACHED_PROCESS)) { diff --git a/dlls/ntdll/env.c b/dlls/ntdll/env.c index ab54a9ec563..04e4ba94563 100644 --- a/dlls/ntdll/env.c +++ b/dlls/ntdll/env.c @@ -684,6 +684,7 @@ void init_user_process_params(void) new_params->dwFillAttribute = params->dwFillAttribute; new_params->dwFlags = params->dwFlags; new_params->wShowWindow = params->wShowWindow; + new_params->ProcessGroupId = params->ProcessGroupId; NtCurrentTeb()->Peb->ProcessParameters = new_params; NtFreeVirtualMemory( GetCurrentProcess(), (void **)¶ms, &size, MEM_RELEASE ); diff --git a/dlls/ntdll/unix/env.c b/dlls/ntdll/unix/env.c index 9cd71e4b020..3f00f6f68de 100644 --- a/dlls/ntdll/unix/env.c +++ b/dlls/ntdll/unix/env.c @@ -1823,6 +1823,7 @@ static void *build_wow64_parameters( const RTL_USER_PROCESS_PARAMETERS *params ) wow64_params->dwFillAttribute = params->dwFillAttribute; wow64_params->dwFlags = params->dwFlags; wow64_params->wShowWindow = params->wShowWindow; + wow64_params->ProcessGroupId = params->ProcessGroupId; dst = (WCHAR *)(wow64_params + 1); dup_unicode_string( ¶ms->CurrentDirectory.DosPath, &dst, &wow64_params->CurrentDirectory.DosPath ); @@ -1981,6 +1982,7 @@ static RTL_USER_PROCESS_PARAMETERS *build_initial_params( void **module ) params->Size = size; params->Flags = PROCESS_PARAMS_FLAG_NORMALIZED; params->wShowWindow = 1; /* SW_SHOWNORMAL */ + params->ProcessGroupId = GetCurrentProcessId(); params->CurrentDirectory.DosPath.Buffer = (WCHAR *)(params + 1); wcscpy( params->CurrentDirectory.DosPath.Buffer, get_dos_path( curdir )); @@ -2079,6 +2081,7 @@ void init_startup_info(void) params->dwFillAttribute = info->attribute; params->dwFlags = info->flags; params->wShowWindow = info->show; + params->ProcessGroupId = info->process_group_id; src = (WCHAR *)(info + 1); dst = (WCHAR *)(params + 1); @@ -2169,6 +2172,7 @@ void *create_startup_info( const UNICODE_STRING *nt_image, const RTL_USER_PROCES info->attribute = params->dwFillAttribute; info->flags = params->dwFlags; info->show = params->wShowWindow; + info->process_group_id = params->ProcessGroupId; ptr = info + 1; info->curdir_len = append_string( &ptr, params, ¶ms->CurrentDirectory.DosPath ); diff --git a/dlls/ntdll/unix/process.c b/dlls/ntdll/unix/process.c index 30bd6f083bd..4c7e545695c 100644 --- a/dlls/ntdll/unix/process.c +++ b/dlls/ntdll/unix/process.c @@ -441,7 +441,7 @@ static NTSTATUS spawn_process( const RTL_USER_PROCESS_PARAMETERS *params, int so { if (!(pid = fork())) /* grandchild */ { - if (params->ConsoleFlags || + if ((peb->ProcessParameters && params->ProcessGroupId != peb->ProcessParameters->ProcessGroupId) || params->ConsoleHandle == CONSOLE_HANDLE_ALLOC || params->ConsoleHandle == CONSOLE_HANDLE_ALLOC_NO_WINDOW || (params->hStdInput == INVALID_HANDLE_VALUE && params->hStdOutput == INVALID_HANDLE_VALUE)) @@ -620,7 +620,7 @@ static NTSTATUS fork_and_exec( OBJECT_ATTRIBUTES *attr, int unixdir, { close( fd[0] ); - if (params->ConsoleFlags || + if ((peb->ProcessParameters && params->ProcessGroupId != peb->ProcessParameters->ProcessGroupId) || params->ConsoleHandle == CONSOLE_HANDLE_ALLOC || params->ConsoleHandle == CONSOLE_HANDLE_ALLOC_NO_WINDOW || (params->hStdInput == INVALID_HANDLE_VALUE && params->hStdOutput == INVALID_HANDLE_VALUE)) diff --git a/include/wine/server_protocol.h b/include/wine/server_protocol.h index e610a8941d7..52228a33b00 100644 --- a/include/wine/server_protocol.h +++ b/include/wine/server_protocol.h @@ -208,6 +208,7 @@ typedef struct unsigned int attribute; unsigned int flags; unsigned int show; + process_id_t process_group_id; data_size_t curdir_len; data_size_t dllpath_len; data_size_t imagepath_len; @@ -6456,7 +6457,7 @@ union generic_reply /* ### protocol_version begin ### */ -#define SERVER_PROTOCOL_VERSION 780 +#define SERVER_PROTOCOL_VERSION 781 /* ### protocol_version end ### */ diff --git a/server/process.c b/server/process.c index 20d7fa678d5..a0d5ea64d97 100644 --- a/server/process.c +++ b/server/process.c @@ -1358,7 +1358,10 @@ DECL_HANDLER(new_process) /* debug_children is set to 1 by default */ } - if (!info->data->console_flags) process->group_id = parent->group_id; + if (info->data->process_group_id == parent->group_id) + process->group_id = parent->group_id; + else + info->data->process_group_id = process->group_id; info->process = (struct process *)grab_object( process ); reply->info = alloc_handle( current->process, info, SYNCHRONIZE, 0 ); diff --git a/server/protocol.def b/server/protocol.def index 919297c818c..68008f8a02b 100644 --- a/server/protocol.def +++ b/server/protocol.def @@ -224,6 +224,7 @@ typedef struct unsigned int attribute; unsigned int flags; unsigned int show; + process_id_t process_group_id; data_size_t curdir_len; data_size_t dllpath_len; data_size_t imagepath_len; diff --git a/server/request.h b/server/request.h index de0931ab763..b550fbd77fa 100644 --- a/server/request.h +++ b/server/request.h @@ -721,7 +721,7 @@ C_ASSERT( sizeof(property_data_t) == 16 ); C_ASSERT( sizeof(rectangle_t) == 16 ); C_ASSERT( sizeof(select_op_t) == 264 ); C_ASSERT( sizeof(short int) == 2 ); -C_ASSERT( sizeof(startup_info_t) == 92 ); +C_ASSERT( sizeof(startup_info_t) == 96 ); C_ASSERT( sizeof(struct filesystem_event) == 12 ); C_ASSERT( sizeof(struct handle_info) == 20 ); C_ASSERT( sizeof(struct luid) == 8 ); diff --git a/server/trace.c b/server/trace.c index 58a31670ea3..f99591c7b1f 100644 --- a/server/trace.c +++ b/server/trace.c @@ -960,10 +960,11 @@ static void dump_varargs_startup_info( const char *prefix, data_size_t size ) fprintf( stderr, "%s{debug_flags=%x,console_flags=%x,console=%04x,hstdin=%04x,hstdout=%04x,hstderr=%04x," - "x=%u,y=%u,xsize=%u,ysize=%u,xchars=%u,ychars=%u,attribute=%02x,flags=%x,show=%u", + "x=%u,y=%u,xsize=%u,ysize=%u,xchars=%u,ychars=%u,attribute=%02x,flags=%x,show=%u," + "process_group_id=%u", prefix, info.debug_flags, info.console_flags, info.console, info.hstdin, info.hstdout, info.hstderr, info.x, info.y, info.xsize, info.ysize, - info.xchars, info.ychars, info.attribute, info.flags, info.show ); + info.xchars, info.ychars, info.attribute, info.flags, info.show, info.process_group_id ); pos = dump_inline_unicode_string( ",curdir=L\"", pos, info.curdir_len, size ); pos = dump_inline_unicode_string( "\",dllpath=L\"", pos, info.dllpath_len, size ); pos = dump_inline_unicode_string( "\",imagepath=L\"", pos, info.imagepath_len, size ); diff --git a/tools/make_requests b/tools/make_requests index ec690642dda..5303ff058df 100755 --- a/tools/make_requests +++ b/tools/make_requests @@ -61,7 +61,7 @@ my %formats = "pe_image_info_t" => [ 80, 8 ], "property_data_t" => [ 16, 8 ], "select_op_t" => [ 264, 8 ], - "startup_info_t" => [ 92, 4 ], + "startup_info_t" => [ 96, 4 ], "user_apc_t" => [ 40, 8 ], "struct filesystem_event" => [ 12, 4 ], "struct handle_info" => [ 20, 4 ], -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3442
From: Eric Pouech <epouech(a)codeweavers.com> Signed-off-by: Eric Pouech <epouech(a)codeweavers.com> --- dlls/kernel32/tests/console.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/dlls/kernel32/tests/console.c b/dlls/kernel32/tests/console.c index 7f66d34dc4c..4f0a8b28376 100644 --- a/dlls/kernel32/tests/console.c +++ b/dlls/kernel32/tests/console.c @@ -5030,6 +5030,14 @@ static void test_CreateProcessCUI(void) {FALSE, CREATE_NEW_PROCESS_GROUP, STARTUPINFO_STD, TRUE, CP_GROUP_LEADER, .is_todo = TRUE}, {FALSE, 0, STARTUPINFO_STD, FALSE, CP_ENABLED_CTRLC, .is_todo = TRUE}, /* 15 */ {FALSE, CREATE_NEW_PROCESS_GROUP, STARTUPINFO_STD, FALSE, CP_GROUP_LEADER, .is_todo = TRUE}, + {TRUE, CREATE_NEW_PROCESS_GROUP | CREATE_NEW_CONSOLE, CONSOLE_STD, TRUE, CP_INH_CONSOLE | CP_WITH_WINDOW | CP_GROUP_LEADER | CP_ALONE}, + {FALSE, CREATE_NEW_PROCESS_GROUP | CREATE_NEW_CONSOLE, CONSOLE_STD, TRUE, CP_GROUP_LEADER}, + {TRUE, CREATE_NEW_PROCESS_GROUP | CREATE_NEW_CONSOLE, CONSOLE_STD, FALSE, CP_INH_CONSOLE | CP_WITH_WINDOW | CP_GROUP_LEADER | CP_ALONE | CP_ENABLED_CTRLC, .is_todo = TRUE}, + {FALSE, CREATE_NEW_PROCESS_GROUP | CREATE_NEW_CONSOLE, CONSOLE_STD, FALSE, CP_GROUP_LEADER | CP_ENABLED_CTRLC, .is_todo = TRUE}, +/* 20 */ {TRUE, CREATE_NEW_PROCESS_GROUP | DETACHED_PROCESS, CONSOLE_STD, TRUE, CP_GROUP_LEADER}, + {FALSE, CREATE_NEW_PROCESS_GROUP | DETACHED_PROCESS, CONSOLE_STD, TRUE, CP_GROUP_LEADER}, + {TRUE, CREATE_NEW_PROCESS_GROUP | DETACHED_PROCESS, CONSOLE_STD, FALSE, CP_GROUP_LEADER}, + {FALSE, CREATE_NEW_PROCESS_GROUP | DETACHED_PROCESS, CONSOLE_STD, FALSE, CP_GROUP_LEADER}, }; hstd[0] = GetStdHandle(STD_INPUT_HANDLE); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3442
From: Eric Pouech <epouech(a)codeweavers.com> (when CREATE_NEW_CONSOLE is used). Signed-off-by: Eric Pouech <epouech(a)codeweavers.com> --- dlls/kernel32/tests/console.c | 4 ++-- dlls/kernelbase/process.c | 5 ++++- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/dlls/kernel32/tests/console.c b/dlls/kernel32/tests/console.c index 4f0a8b28376..9099963e8e2 100644 --- a/dlls/kernel32/tests/console.c +++ b/dlls/kernel32/tests/console.c @@ -5032,8 +5032,8 @@ static void test_CreateProcessCUI(void) /* 15 */ {FALSE, CREATE_NEW_PROCESS_GROUP, STARTUPINFO_STD, FALSE, CP_GROUP_LEADER, .is_todo = TRUE}, {TRUE, CREATE_NEW_PROCESS_GROUP | CREATE_NEW_CONSOLE, CONSOLE_STD, TRUE, CP_INH_CONSOLE | CP_WITH_WINDOW | CP_GROUP_LEADER | CP_ALONE}, {FALSE, CREATE_NEW_PROCESS_GROUP | CREATE_NEW_CONSOLE, CONSOLE_STD, TRUE, CP_GROUP_LEADER}, - {TRUE, CREATE_NEW_PROCESS_GROUP | CREATE_NEW_CONSOLE, CONSOLE_STD, FALSE, CP_INH_CONSOLE | CP_WITH_WINDOW | CP_GROUP_LEADER | CP_ALONE | CP_ENABLED_CTRLC, .is_todo = TRUE}, - {FALSE, CREATE_NEW_PROCESS_GROUP | CREATE_NEW_CONSOLE, CONSOLE_STD, FALSE, CP_GROUP_LEADER | CP_ENABLED_CTRLC, .is_todo = TRUE}, + {TRUE, CREATE_NEW_PROCESS_GROUP | CREATE_NEW_CONSOLE, CONSOLE_STD, FALSE, CP_INH_CONSOLE | CP_WITH_WINDOW | CP_GROUP_LEADER | CP_ALONE | CP_ENABLED_CTRLC}, + {FALSE, CREATE_NEW_PROCESS_GROUP | CREATE_NEW_CONSOLE, CONSOLE_STD, FALSE, CP_GROUP_LEADER | CP_ENABLED_CTRLC}, /* 20 */ {TRUE, CREATE_NEW_PROCESS_GROUP | DETACHED_PROCESS, CONSOLE_STD, TRUE, CP_GROUP_LEADER}, {FALSE, CREATE_NEW_PROCESS_GROUP | DETACHED_PROCESS, CONSOLE_STD, TRUE, CP_GROUP_LEADER}, {TRUE, CREATE_NEW_PROCESS_GROUP | DETACHED_PROCESS, CONSOLE_STD, FALSE, CP_GROUP_LEADER}, diff --git a/dlls/kernelbase/process.c b/dlls/kernelbase/process.c index b6775052699..8291338fa58 100644 --- a/dlls/kernelbase/process.c +++ b/dlls/kernelbase/process.c @@ -188,7 +188,10 @@ static RTL_USER_PROCESS_PARAMETERS *create_process_params( const WCHAR *filename RtlFreeUnicodeString( &newdirW ); if (flags & CREATE_NEW_PROCESS_GROUP) - params->ConsoleFlags = 1; + { + if (!(flags & CREATE_NEW_CONSOLE)) + params->ConsoleFlags = 1; + } else params->ProcessGroupId = NtCurrentTeb()->Peb->ProcessParameters->ProcessGroupId; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3442
Hi, It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated. The tests also ran into some preexisting test failures. If you know how to fix them that would be helpful. See the TestBot job for the details: The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=136564 Your paranoid android. === w864 (32 bit report) === kernel32: console.c:1236: Test failed: WaitForSingleObject returned 0 console.c:1238: Test failed: WaitForSingleObject returned 0 console.c:1240: Test failed: WaitForSingleObject returned 0 console.c:1242: Test failed: WaitForSingleObject returned 0 console.c:1244: Test failed: WaitForSingleObject returned 0 console.c:1246: Test failed: NtWaitForSingleObject returned 0 console.c:1249: Test failed: NtWaitForSingleObject returned 0 === w10pro64 (32 bit report) === kernel32: console.c:1236: Test failed: WaitForSingleObject returned 0 console.c:1238: Test failed: WaitForSingleObject returned 0 console.c:1240: Test failed: WaitForSingleObject returned 0 console.c:1242: Test failed: WaitForSingleObject returned 0 console.c:1244: Test failed: WaitForSingleObject returned 0 console.c:1246: Test failed: NtWaitForSingleObject returned 0 console.c:1249: Test failed: NtWaitForSingleObject returned 0 === w864 (64 bit report) === kernel32: console.c:1236: Test failed: WaitForSingleObject returned 0 console.c:1238: Test failed: WaitForSingleObject returned 0 console.c:1240: Test failed: WaitForSingleObject returned 0 console.c:1242: Test failed: WaitForSingleObject returned 0 console.c:1244: Test failed: WaitForSingleObject returned 0 console.c:1246: Test failed: NtWaitForSingleObject returned 0 console.c:1249: Test failed: NtWaitForSingleObject returned 0
On Tue Aug 22 07:53:46 2023 +0000, eric pouech wrote:
changed this line in [version 3 of the diff](/wine/wine/-/merge_requests/3442/diffs?diff_id=64057&start_sha=a02758a41bc72dcdfe90b91b231866fe95e0d7c1#a666ff29c09175a075770ae07b2d8ca32ee2f5e5_444_444) V3 pushed (containing adapted proposed change by @jacek)
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/3442#note_42989
Jacek Caban (@jacek) commented about dlls/kernelbase/process.c:
RtlFreeUnicodeString( &newdirW );
if (flags & CREATE_NEW_PROCESS_GROUP) - params->ConsoleFlags = 1; + { + if (!(flags & CREATE_NEW_CONSOLE)) + params->ConsoleFlags = 1; + } else params->ProcessGroupId = NtCurrentTeb()->Peb->ProcessParameters->ProcessGroupId;
It could be slightly simplified by swapping ifs order: ``` if (!(flags & CREATE_NEW_PROCESS_GROUP)) //... else if (!(flags & CREATE_NEW_CONSOLE)) //... ``` -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3442#note_43039
The MR looks mostly good for me, but I'm afraid that we need to do something about test failures. This happens very sporadically on win10 as well, but it seems consistent on win8 with this MR: http://test.winehq.org/data/42811d775b1a299f1dddcbbae4a2b80cb1b77698/win22H2... I don't know where those events come from, maybe reading and printing events in case wait succeeded would give us a hint. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3442#note_43040
On Tue Aug 22 15:37:37 2023 +0000, Jacek Caban wrote:
The MR looks mostly good for me, but I'm afraid that we need to do something about test failures. This happens very sporadically on win10 as well, but it seems consistent on win8 with this MR: http://test.winehq.org/data/42811d775b1a299f1dddcbbae4a2b80cb1b77698/win22H2... I don't know where those events come from, maybe reading and printing events in case wait succeeded would give us a hint. yes, I wanted to have another look at it (tried something in V2, but doesn't look effective \<g\>), and at least understand the reason of the failures (even if the we end up adding broken() where needed)
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/3442#note_43045
participants (4)
-
Eric Pouech -
eric pouech (＠epo)
-
Jacek Caban (＠jacek)
-
Marvin