[PATCH v3 0/3] MR3312: programs/start: Allow Ctrl-C to terminate GUI applications (/exec mode).
Since dcf0bf1f, one could no longer kill a GUI application launched from unix shell (with start.exe /exec) with ctrl-c. (depends on https://gitlab.winehq.org/wine/wine/-/merge_requests/3310 to be fully functional). Signed-off-by: Eric Pouech <epouech(a)codeweavers.com> -- v3: programs/cmd: Better handle ctrl-c events. ntdll,server: Terminate GUI programs launched from unix shell. kernel32/console: Add tests for GenerateConsoleCtrlEvent(). https://gitlab.winehq.org/wine/wine/-/merge_requests/3312
From: Eric Pouech <epouech(a)codeweavers.com> Signed-off-by: Eric Pouech <epouech(a)codeweavers.com> --- dlls/kernel32/tests/console.c | 145 ++++++++++++++++++++++++++++++++++ 1 file changed, 145 insertions(+) diff --git a/dlls/kernel32/tests/console.c b/dlls/kernel32/tests/console.c index 6facf2ad214..962929600d4 100644 --- a/dlls/kernel32/tests/console.c +++ b/dlls/kernel32/tests/console.c @@ -5153,6 +5153,135 @@ static void test_CreateProcessCUI(void) SetStdHandle(STD_ERROR_HANDLE, hstd[2]); } +#define NO_EVENT 0xfe + +static HANDLE mch_child_kill_event; +static DWORD mch_child_event = NO_EVENT; +static BOOL WINAPI mch_child(DWORD event) +{ + mch_child_event = event; + SetEvent(mch_child_kill_event); + return TRUE; +} + +static void test_CtrlHandlerSubsystem(void) +{ + static char guiexec[MAX_PATH]; + static char cuiexec[MAX_PATH]; + + static struct + { + /* input */ + BOOL use_cui; + DWORD cp_flags; + enum pgid {PGID_PARENT, PGID_ZERO, PGID_CHILD} pgid_kind; + /* output */ + unsigned child_event; + } + tests[] = + { +/* 0 */ {FALSE, 0, PGID_PARENT, NO_EVENT}, + {FALSE, 0, PGID_ZERO, NO_EVENT}, + {FALSE, CREATE_NEW_PROCESS_GROUP, PGID_CHILD, NO_EVENT}, + {FALSE, CREATE_NEW_PROCESS_GROUP, PGID_PARENT, NO_EVENT}, + {FALSE, CREATE_NEW_PROCESS_GROUP, PGID_ZERO, NO_EVENT}, +/* 5 */ {TRUE, 0, PGID_PARENT, CTRL_C_EVENT}, + {TRUE, 0, PGID_ZERO, CTRL_C_EVENT}, + {TRUE, CREATE_NEW_PROCESS_GROUP, PGID_CHILD, NO_EVENT}, + {TRUE, CREATE_NEW_PROCESS_GROUP, PGID_PARENT, NO_EVENT}, + {TRUE, CREATE_NEW_PROCESS_GROUP, PGID_ZERO, NO_EVENT}, +/* 10 */ {TRUE, CREATE_NEW_CONSOLE, PGID_PARENT, NO_EVENT}, + {TRUE, CREATE_NEW_CONSOLE, PGID_ZERO, NO_EVENT}, + {TRUE, DETACHED_PROCESS, PGID_PARENT, NO_EVENT}, + {TRUE, DETACHED_PROCESS, PGID_ZERO, NO_EVENT}, + }; + SECURITY_ATTRIBUTES inheritable_attr = { sizeof(inheritable_attr), NULL, TRUE }; + STARTUPINFOA si = { sizeof(si) }; + PROCESS_INFORMATION info; + char buf[MAX_PATH]; + DWORD exit_code; + HANDLE event_child; + char **argv; + DWORD saved_console_flags; + DWORD pgid; + DWORD res; + int i; + + winetest_get_mainargs(&argv); + GetTempPathA(ARRAY_SIZE(guiexec), guiexec); + strcat(guiexec, "console_gui.exe"); + copy_change_subsystem(argv[0], guiexec, IMAGE_SUBSYSTEM_WINDOWS_GUI); + GetTempPathA(ARRAY_SIZE(cuiexec), cuiexec); + strcat(cuiexec, "console_cui.exe"); + copy_change_subsystem(argv[0], cuiexec, IMAGE_SUBSYSTEM_WINDOWS_CUI); + + event_child = CreateEventA(&inheritable_attr, FALSE, FALSE, NULL); + ok(event_child != NULL, "Couldn't create event\n"); + + saved_console_flags = RtlGetCurrentPeb()->ProcessParameters->ConsoleFlags; + + /* protect self against ctrl-c, but don't mask it on child */ + res = SetConsoleCtrlHandler(NULL, FALSE); + res = SetConsoleCtrlHandler(mydummych, TRUE); + ok(res, "Couldn't set ctrl-c handler flag\n"); + + for (i = 0; i < ARRAY_SIZE(tests); i++) + { + winetest_push_context("test #%u", i); + + sprintf(buf, "\"%s\" console ctrl_handler %p", tests[i].use_cui ? cuiexec : guiexec, event_child); + + res = CreateProcessA(NULL, buf, NULL, NULL, TRUE, tests[i].cp_flags, + NULL, NULL, &si, &info); + ok(res, "CreateProcess failed: %lu %s\n", GetLastError(), tests[i].use_cui ? cuiexec : guiexec); + + res = WaitForSingleObject(event_child, 1000); + ok(res == WAIT_OBJECT_0, "Child didn't init %lu %p\n", res, event_child); + + switch (tests[i].pgid_kind) + { + case PGID_PARENT: + pgid = RtlGetCurrentPeb()->ProcessParameters->ProcessGroupId; + break; + case PGID_CHILD: + ok((tests[i].cp_flags & CREATE_NEW_PROCESS_GROUP) != 0, + "PGID should only be used with new process groupw\n"); + pgid = info.dwProcessId; + break; + case PGID_ZERO: + pgid = 0; + break; + default: + ok(0, "Unexpected pgid kind %u\n", tests[i].pgid_kind); + pgid = 0; + } + + res = GenerateConsoleCtrlEvent(CTRL_C_EVENT, pgid); + ok(res, "GenerateConsoleCtrlEvent failed: %lu\n", GetLastError()); + + res = WaitForSingleObject(info.hProcess, 2000); + ok(res == WAIT_OBJECT_0, "Expecting child to be terminated\n"); + res = GetExitCodeProcess(info.hProcess, &exit_code); + ok(res, "Couldn't get exit code\n"); + + ok(tests[i].child_event == exit_code, "Unexpected exit code %#lx, instead of %#x\n", + exit_code, tests[i].child_event); + + CloseHandle(info.hProcess); + CloseHandle(info.hThread); + winetest_pop_context(); + } + + CloseHandle(event_child); + + RtlGetCurrentPeb()->ProcessParameters->ConsoleFlags = saved_console_flags; + res = SetConsoleCtrlHandler(mydummych, FALSE); + ok(res, "Couldn't remove ctrl-c handler flag\n"); + + DeleteFileA(guiexec); + DeleteFileA(cuiexec); +} + START_TEST(console) { HANDLE hConIn, hConOut, revert_output = NULL, unbound_output; @@ -5181,6 +5310,21 @@ START_TEST(console) return; } + if (argc == 4 && !strcmp(argv[2], "ctrl_handler")) + { + HANDLE event; + + SetConsoleCtrlHandler(mch_child, TRUE); + mch_child_kill_event = CreateEventA(NULL, FALSE, FALSE, NULL); + ok(mch_child_kill_event != NULL, "Couldn't create event\n"); + sscanf(argv[3], "%p", &event); + ret = SetEvent(event); + ok(ret, "SetEvent failed\n"); + + WaitForSingleObject(mch_child_kill_event, 1000); /* enough for all events to be distributed? */ + ExitProcess(mch_child_event); + } + if (argc == 3 && !strcmp(argv[2], "check_console")) { DWORD exit_code = 0, pcslist; @@ -5408,6 +5552,7 @@ START_TEST(console) test_AllocConsole(); test_FreeConsole(); test_CreateProcessCUI(); + test_CtrlHandlerSubsystem(); } else if (revert_output) SetConsoleActiveScreenBuffer(revert_output); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3312
From: Eric Pouech <epouech(a)codeweavers.com> Since dcf0bf1f, one could no longer kill a GUI application launched from unix shell (with start.exe /exec) with ctrl-c. Basically, when a GUI program is run from a unix shell, we must propagate ctrl-c using the unix process group. Signed-off-by: Eric Pouech <epouech(a)codeweavers.com> --- dlls/ntdll/loader.c | 2 +- include/wine/condrv.h | 5 +++-- programs/conhost/conhost.c | 2 +- server/console.c | 35 ++++++++++++++++++++--------------- 4 files changed, 25 insertions(+), 19 deletions(-) diff --git a/dlls/ntdll/loader.c b/dlls/ntdll/loader.c index fc2d2ff7a12..29297202784 100644 --- a/dlls/ntdll/loader.c +++ b/dlls/ntdll/loader.c @@ -3219,7 +3219,7 @@ NTSTATUS WINAPI __wine_ctrl_routine( void *arg ) { DWORD ret = 0; - if (pCtrlRoutine && NtCurrentTeb()->Peb->ProcessParameters->ConsoleHandle) ret = pCtrlRoutine( arg ); + if (pCtrlRoutine) ret = pCtrlRoutine( arg ); RtlExitUserThread( ret ); } diff --git a/include/wine/condrv.h b/include/wine/condrv.h index 51534720444..1a5c4f4555b 100644 --- a/include/wine/condrv.h +++ b/include/wine/condrv.h @@ -190,8 +190,9 @@ struct condrv_scroll_params /* IOCTL_CONDRV_CTRL_EVENT params */ struct condrv_ctrl_event { - int event; /* the event to send */ - unsigned int group_id; /* the group to send the event to */ + int event; /* the event to send */ + unsigned int group_id; /* the group to send the event to */ + /* (when from server, target process group: 0 = windows, 1 = unix) */ }; /* Wine specific values for console inheritance (params->ConsoleHandle) */ diff --git a/programs/conhost/conhost.c b/programs/conhost/conhost.c index eb3c574789f..38cfb16dbdb 100644 --- a/programs/conhost/conhost.c +++ b/programs/conhost/conhost.c @@ -1518,7 +1518,7 @@ NTSTATUS write_console_input( struct console *console, const INPUT_RECORD *recor IO_STATUS_BLOCK io; ctrl_event.event = event; - ctrl_event.group_id = 0; + ctrl_event.group_id = console->is_unix; NtDeviceIoControlFile( console->server, NULL, NULL, NULL, &io, IOCTL_CONDRV_CTRL_EVENT, &ctrl_event, sizeof(ctrl_event), NULL, 0 ); } diff --git a/server/console.c b/server/console.c index b64283baf4a..87ea6ec1808 100644 --- a/server/console.c +++ b/server/console.c @@ -663,16 +663,22 @@ struct thread *console_get_renderer( struct console *console ) struct console_signal_info { - struct console *console; - process_id_t group; - int signal; + struct console *console; + process_id_t group; + int unix_pgid; + int signal; }; static int propagate_console_signal_cb(struct process *process, void *user) { struct console_signal_info* csi = (struct console_signal_info*)user; + unsigned cmp; - if (process->console == csi->console && (!csi->group || process->group_id == csi->group)) + if (csi->unix_pgid) + cmp = csi->unix_pgid == getpgid(process->unix_pid) && csi->console->renderer->process != process; + else + cmp = process->console == csi->console && (!csi->group || process->group_id == csi->group); + if (cmp) { /* find a suitable thread to signal */ struct thread *thread; @@ -684,8 +690,8 @@ static int propagate_console_signal_cb(struct process *process, void *user) return FALSE; } -static void propagate_console_signal( struct console *console, - int sig, process_id_t group_id ) +static void propagate_console_signal( int sig, struct console *console, + process_id_t group_id, int unix_pgid ) { struct console_signal_info csi; @@ -703,8 +709,9 @@ static void propagate_console_signal( struct console *console, set_error( STATUS_NOT_IMPLEMENTED ); return; } - csi.console = console; - csi.group = group_id; + csi.console = console; + csi.group = group_id; + csi.unix_pgid = unix_pgid; enum_processes(propagate_console_signal_cb, &csi); } @@ -983,7 +990,7 @@ static void console_ioctl( struct fd *fd, ioctl_code_t code, struct async *async set_error( STATUS_INVALID_PARAMETER ); return; } - propagate_console_signal( console, event->event, group ); + propagate_console_signal( event->event, console, group, 0 ); return; } @@ -1141,12 +1148,10 @@ static void console_server_ioctl( struct fd *fd, ioctl_code_t code, struct async set_error( STATUS_INVALID_PARAMETER ); return; } - if (!server->console) - { - set_error( STATUS_INVALID_HANDLE ); - return; - } - propagate_console_signal( server->console, event->event, event->group_id ); + if (event->group_id) + propagate_console_signal( event->event, server->console, 0, getpgid(server->console->renderer->process->unix_pid)); + else + propagate_console_signal( event->event, server->console, 0, 0); return; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3312
From: Eric Pouech <epouech(a)codeweavers.com> cmd shouldn't terminate itself when user hits ctrl-c. cmd should terminate the currently running CUI child it waits for upon ctrl-c. Signed-off-by: Eric Pouech <epouech(a)codeweavers.com> --- programs/cmd/wcmdmain.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/programs/cmd/wcmdmain.c b/programs/cmd/wcmdmain.c index d00c6e07566..cc81534fff3 100644 --- a/programs/cmd/wcmdmain.c +++ b/programs/cmd/wcmdmain.c @@ -2411,6 +2411,12 @@ void WCMD_free_commands(CMD_LIST *cmds) { } } +static BOOL WINAPI my_event_handler(DWORD ctrl) +{ + WCMD_output(L"\n"); + return ctrl == CTRL_C_EVENT; +} + /***************************************************************************** * Main entry point. This is a console application so we have a main() not a @@ -2658,6 +2664,10 @@ int __cdecl wmain (int argc, WCHAR *argvW[]) if (opt_s && *cmd=='\"') WCMD_strip_quotes(cmd); } + else + { + SetConsoleCtrlHandler(my_event_handler, TRUE); + } /* Save cwd into appropriate env var (Must be before the /c processing */ GetCurrentDirectoryW(ARRAY_SIZE(string), string); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3312
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=136749 Your paranoid android. === w7u_2qxl (32 bit report) === kernel32: console.c:5260: Test failed: test #0: GenerateConsoleCtrlEvent failed: 87 console.c:5260: Test failed: test #3: GenerateConsoleCtrlEvent failed: 87 console.c:5260: Test failed: test #5: GenerateConsoleCtrlEvent failed: 87 console.c:5267: Test failed: test #5: Unexpected exit code 0xfe, instead of 0 console.c:5260: Test failed: test #8: GenerateConsoleCtrlEvent failed: 87 console.c:5260: Test failed: test #10: GenerateConsoleCtrlEvent failed: 87 console.c:5260: Test failed: test #12: GenerateConsoleCtrlEvent failed: 87 === w7u_adm (32 bit report) === kernel32: console.c:5260: Test failed: test #0: GenerateConsoleCtrlEvent failed: 87 console.c:5260: Test failed: test #3: GenerateConsoleCtrlEvent failed: 87 console.c:5260: Test failed: test #5: GenerateConsoleCtrlEvent failed: 87 console.c:5267: Test failed: test #5: Unexpected exit code 0xfe, instead of 0 console.c:5260: Test failed: test #8: GenerateConsoleCtrlEvent failed: 87 console.c:5260: Test failed: test #10: GenerateConsoleCtrlEvent failed: 87 console.c:5260: Test failed: test #12: GenerateConsoleCtrlEvent failed: 87 === w7u_el (32 bit report) === kernel32: console.c:5260: Test failed: test #0: GenerateConsoleCtrlEvent failed: 87 console.c:5260: Test failed: test #3: GenerateConsoleCtrlEvent failed: 87 console.c:5260: Test failed: test #5: GenerateConsoleCtrlEvent failed: 87 console.c:5267: Test failed: test #5: Unexpected exit code 0xfe, instead of 0 console.c:5260: Test failed: test #8: GenerateConsoleCtrlEvent failed: 87 console.c:5260: Test failed: test #10: GenerateConsoleCtrlEvent failed: 87 console.c:5260: Test failed: test #12: GenerateConsoleCtrlEvent failed: 87 === w1064v1507 (32 bit report) === kernel32: console.c:5239: Test failed: test #4: Child didn't init 258 00000300 console.c:5267: Test failed: test #5: Unexpected exit code 0xfe, instead of 0 console.c:5267: Test failed: test #6: Unexpected exit code 0xfe, instead of 0 console.c:5267: Test failed: test #7: Unexpected exit code 0xc0000142, instead of 0xfe === w7pro64 (64 bit report) === kernel32: console.c:5260: Test failed: test #0: GenerateConsoleCtrlEvent failed: 87 console.c:5260: Test failed: test #3: GenerateConsoleCtrlEvent failed: 87 console.c:5260: Test failed: test #5: GenerateConsoleCtrlEvent failed: 87 console.c:5267: Test failed: test #5: Unexpected exit code 0xfe, instead of 0 console.c:5260: Test failed: test #8: GenerateConsoleCtrlEvent failed: 87 console.c:5260: Test failed: test #10: GenerateConsoleCtrlEvent failed: 87 console.c:5260: Test failed: test #12: GenerateConsoleCtrlEvent failed: 87 === w1064v1507 (64 bit report) === kernel32: console.c:5239: Test failed: test #3: Child didn't init 258 0000000000000114 console.c:5263: Test failed: test #5: Expecting child to be terminated console.c:5267: Test failed: test #5: Unexpected exit code 0x103, instead of 0 console.c:5267: Test failed: test #6: Unexpected exit code 0xfe, instead of 0 console.c:5267: Test failed: test #7: Unexpected exit code 0xc0000142, instead of 0xfe === w1064_adm (64 bit report) === kernel32: console.c:5239: Test failed: test #11: Child didn't init 258 0000000000000210
V2 pushed, with radical different approach (title of not changed): V2 contains: * set of tests for GenerateConsoleCtrlEvent (no issue found) * changed the way ctrl events are generated. when running 'wine \<gui-app\>' from unix console, we wait for \<gui-app\> to terminate before returning to unix-shell prompt, we rather rely on unix process group to identify the children to sent the events to. * fix cmd to handle properly ctrl-c event (terminate running child if any, but don't terminate itself) * -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3312#note_43566
participants (3)
-
Eric Pouech -
eric pouech (＠epo)
-
Marvin