[PATCH v2 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 -- v2: 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 8aa41aba183..f8382fb684d 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 ); @@ -1980,6 +1981,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 )); @@ -2078,6 +2080,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); @@ -2168,6 +2171,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..6abfe0fc5e9 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 (params->ProcessGroupId == GetCurrentProcessId() || 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 (params->ProcessGroupId == GetCurrentProcessId() || 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=136274 Your paranoid android. === w864 (32 bit report) === kernel32: console.c:1249: Test failed: NtWaitForSingleObject returned 0
V2 pushed: - separated first patch by setting in a dedicated patch the testing of availability of ProcessGroupId field (not present in Win7) - removed one case of stateful test (potentially to help with V1 test failures, but most of local and testbot runs couldn't repro them) - made the implementation better match Win10 behavior -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3442#note_42485
On Fri Jul 28 15:56:21 2023 +0000, **** wrote:
Marvin replied on the mailing list: ``` 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=135386 Your paranoid android. === w864 (32 bit report) === kernel32: 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 === debian11b (64 bit WoW report) === win32u: win32u.c:1060: Test failed: windows.devices.bluetooth:bluetooth start dlls/windows.devices.bluetooth/tests/bluetooth.c ``` V2 ran clear, so resolving thread
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/3442#note_42487
On Wed Aug 16 14:53:48 2023 +0000, **** wrote:
Marvin replied on the mailing list: ``` 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=136274 Your paranoid android. === w864 (32 bit report) === kernel32: console.c:1249: Test failed: NtWaitForSingleObject returned 0 ``` hmmm... same standalone test (https://testbot.winehq.org/JobDetails.pl?Key=136268) ran fine... not sure where this comes from (and only on w864, same kind of failure than V1)
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/3442#note_42496
Jacek Caban (@jacek) commented about dlls/ntdll/unix/process.c:
{ if (!(pid = fork())) /* grandchild */ { - if (params->ConsoleFlags || + if (params->ProcessGroupId == GetCurrentProcessId() ||
`GetCurrentProcessId()` will return parent process ID at this stage. Should we use something like `peb->ProcessParameters->ProcessGroupId` here instead? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3442#note_42842
On Mon Aug 21 11:00:30 2023 +0000, Jacek Caban wrote:
`GetCurrentProcessId()` will return parent process ID at this stage. Should we use something like `peb->ProcessParameters->ProcessGroupId` here instead? yes, good catch (or even more simply, check for params-\>ProcessGroupId being 0).
I'll update the MR. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3442#note_42858
On Mon Aug 21 12:54:07 2023 +0000, eric pouech wrote:
yes, good catch (or even more simply, check for params-\>ProcessGroupId being 0). I'll update the MR. thinking about it, I'll change as you suggest (trigger for new session is any value different from parent in ProcessGroupId, not just 0)
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/3442#note_42868
participants (4)
-
Eric Pouech -
eric pouech (＠epo)
-
Jacek Caban (＠jacek)
-
Marvin