[PATCH v3 0/6] MR7608: server: Avoid delegating console waits to other objects.
Some console objects currently do several unique things: * Delegate waits onto the queue of another object. This is not really a problem for in-process waits, since we can just return the sync object for the delegate. However, it's also unnecessary, adds to the complexity of the server logic, and is one out of one places where this is done. * Make the wait state dependent on the process. This is difficult to emulate with ntsync and would require creating separate server objects for each process, hacking into duplicate_handle. * Fail a wait entirely in certain circumstances. This is pretty much impossible to emulate with in-process waits. Although ntsync has been in development for some time, I have regrettably failed to notice these problems until now. Fortunately, none of these behaviours happen on modern Windows. Although I/O on unbound handles delegates to the console of the current process, the signaled state does not. As the tests here show, the signaled state of a handle depends on the active console of the process in which the handle was created. If that console no longer exists, the signaled state is no longer updated [with one rather inexplicable exception]. Crucially, in current Windows waits never fail, and the state of an object is the same across all process which hold handles to it. Therefore this patch brings our behaviour to closer match current Windows. In theory these are fds and should use default_fd_signaled(). However, the points at which the handles are signaled are completely different, and I/O does not trigger console handles to become signaled when it normally would. Therefore for the time being I've kept the code using custom signaled ops. There is one other oddity related to consoles, which is the existence of console_add_queue(), which seeks to lazily create an input thread when a console is first waited on. This is one out of two places, after this patch, when the wait process is hijacked (the other being message queues). Fortunately this is easy to handle for in-process synchronization objects, by queueing the ioctl from the callback used to retrieve the in-process synchronization object itself. -- v3: server: Track unbound output signaled state based on its original console. server: Track unbound input signaled state based on its original console. server: Allow waiting on an orphaned screen buffer. server: Use a list of screen buffers per console. https://gitlab.winehq.org/wine/wine/-/merge_requests/7608
From: Elizabeth Figura <zfigura(a)codeweavers.com> --- dlls/kernel32/tests/console.c | 244 +++++++++++++++++++++++++++++++++- 1 file changed, 240 insertions(+), 4 deletions(-) diff --git a/dlls/kernel32/tests/console.c b/dlls/kernel32/tests/console.c index 03491b279af..dd879725658 100644 --- a/dlls/kernel32/tests/console.c +++ b/dlls/kernel32/tests/console.c @@ -4124,12 +4124,132 @@ static void test_GetConsoleScreenBufferInfoEx(HANDLE std_output) ok(GetLastError() == 0xdeadbeef, "got %lu, expected 0xdeadbeef\n", GetLastError()); } -static void test_FreeConsole(void) +/* Flush events and check that there was a double-click event queued. + * Windows may also queue focus change or mouse move events. */ +static void check_mouse_event(HANDLE handle) { - HANDLE handle, unbound_output = NULL, unbound_input = NULL; + BOOL ret, got_mouse_event = FALSE; + INPUT_RECORD ir; + DWORD count; + + while (PeekConsoleInputW(handle, &ir, 1, &count) && count) + { + ret = ReadConsoleInputW(handle, &ir, 1, &count); + ok(ret == TRUE, "got error %lu\n", GetLastError()); + ok(count == 1, "got count %lu\n", count); + if (ir.EventType == MOUSE_EVENT && ir.Event.MouseEvent.dwEventFlags == DOUBLE_CLICK) + got_mouse_event = TRUE; + } + ok(got_mouse_event, "didn't find mouse event\n"); +} + +static void test_unbound_handles_child(DWORD parent_pid, UINT_PTR parent_input_int, UINT_PTR parent_output_int) +{ + HANDLE parent_process, parent_input, parent_output, unbound_input, input, parent_event, child_event; + OBJECT_ATTRIBUTES attr = {sizeof(attr)}; + IO_STATUS_BLOCK iosb; + UNICODE_STRING name; + NTSTATUS status; + INPUT_RECORD ir; + DWORD count; + BOOL ret; + + parent_event = OpenEventA(EVENT_ALL_ACCESS, FALSE, "winetest_unbound_handles_parent"); + child_event = OpenEventA(EVENT_ALL_ACCESS, FALSE, "winetest_unbound_handles_child"); + + parent_process = OpenProcess(PROCESS_DUP_HANDLE, FALSE, parent_pid); + ok(!!parent_process, "got error %lu\n", GetLastError()); + + ret = DuplicateHandle(parent_process, (HANDLE)parent_input_int, + GetCurrentProcess(), &parent_input, 0, FALSE, DUPLICATE_SAME_ACCESS); + ok(ret, "got error %lu\n", GetLastError()); + ret = DuplicateHandle(parent_process, (HANDLE)parent_output_int, + GetCurrentProcess(), &parent_output, 0, FALSE, DUPLICATE_SAME_ACCESS); + ok(ret, "got error %lu\n", GetLastError()); + + ret = WaitForSingleObject(parent_input, 0); + todo_wine ok(!ret, "got %d\n", ret); + ret = WaitForSingleObject(parent_output, 0); + todo_wine ok(!ret, "got %d\n", ret); + + ret = PeekConsoleInputW(parent_input, &ir, 1, &count); + ok(!ret, "got %d\n", ret); + ok(GetLastError() == ERROR_INVALID_HANDLE, "got error %lu\n", GetLastError()); + + /* We cannot create an unbound handle, because we have no console. */ + + attr.ObjectName = &name; + attr.Attributes = OBJ_INHERIT; + RtlInitUnicodeString( &name, L"\\Device\\ConDrv\\Input" ); + status = NtCreateFile( &unbound_input, FILE_READ_DATA | FILE_WRITE_DATA | SYNCHRONIZE | FILE_READ_ATTRIBUTES | + FILE_WRITE_ATTRIBUTES, &attr, &iosb, NULL, FILE_ATTRIBUTE_NORMAL, + FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, FILE_CREATE, + FILE_NON_DIRECTORY_FILE | FILE_SYNCHRONOUS_IO_NONALERT, NULL, 0 ); + todo_wine ok(status == STATUS_INVALID_HANDLE, "got %#lx\n", status); + if (!status) NtClose( unbound_input ); + + /* Allocate a console. */ + + ret = AllocConsole(); + ok(ret == TRUE, "got error %lu\n", GetLastError()); + + unbound_input = create_unbound_handle(FALSE, TRUE); + + input = CreateFileA("CONIN$", GENERIC_READ | GENERIC_WRITE, 0, NULL, OPEN_EXISTING, 0, 0); + ok(input != INVALID_HANDLE_VALUE, "got error %lu\n", GetLastError()); + + memset(&ir, 0, sizeof(ir)); + ir.EventType = MOUSE_EVENT; + ir.Event.MouseEvent.dwEventFlags = DOUBLE_CLICK; + ret = WriteConsoleInputW(input, &ir, 1, &count); + ok(ret, "got error %lu\n", GetLastError()); + + ret = WaitForSingleObject(unbound_input, 0); + ok(!ret, "got %d\n", ret); + + /* We can read from our input using the parent's handle, but its signaled + * state will not reflect our input. */ + + check_mouse_event(parent_input); + + ret = WaitForSingleObject(unbound_input, 0); + ok(ret == WAIT_TIMEOUT, "got %d\n", ret); + ret = WaitForSingleObject(parent_input, 0); + todo_wine ok(!ret, "got %d\n", ret); + + CloseHandle(input); + ret = FreeConsole(); + ok(ret == TRUE, "got error %lu\n", GetLastError()); + + ret = WaitForSingleObject(unbound_input, 0); + todo_wine ok(ret == WAIT_TIMEOUT, "got %d\n", ret); + CloseHandle(unbound_input); + + ret = WaitForSingleObject(parent_input, 0); + todo_wine ok(!ret, "got %d\n", ret); + ret = WaitForSingleObject(parent_output, 0); + todo_wine ok(!ret, "got %d\n", ret); + + /* Parent will consume its input, designalling those handles. */ + SetEvent(child_event); + ret = WaitForSingleObject(parent_event, 1000); + ok(!ret, "got %d\n", ret); + + ret = WaitForSingleObject(parent_input, 0); + todo_wine ok(ret == WAIT_TIMEOUT, "got %d\n", ret); + ret = WaitForSingleObject(parent_output, 0); + todo_wine ok(ret == WAIT_TIMEOUT, "got %d\n", ret); +} + +static void test_FreeConsole(HANDLE input, HANDLE orig_output) +{ + static const INPUT_RECORD ir = {.EventType = MOUSE_EVENT, .Event.MouseEvent.dwEventFlags = DOUBLE_CLICK}; + HANDLE handle, output, parent_event, child_event, unbound_output = NULL, unbound_input = NULL, unbound_input2; + STARTUPINFOA si = {sizeof(si)}; + char buf[MAX_PATH], **argv; + PROCESS_INFORMATION info; DWORD size, mode, type; WCHAR title[16]; - char buf[32]; HWND hwnd; UINT cp; BOOL ret; @@ -4144,6 +4264,53 @@ static void test_FreeConsole(void) unbound_output = create_unbound_handle(TRUE, TRUE); } + output = CreateConsoleScreenBuffer(GENERIC_READ | GENERIC_WRITE, + FILE_SHARE_READ | FILE_SHARE_WRITE, NULL, CONSOLE_TEXTMODE_BUFFER, NULL); + ok(output != INVALID_HANDLE_VALUE, "got error %lu\n", GetLastError()); + + ret = WriteConsoleInputW(input, &ir, 1, &size); + ok(ret, "got error %lu\n", GetLastError()); + + ret = WaitForSingleObject(output, 0); + ok(!ret, "got %d\n", ret); + ret = WaitForSingleObject(unbound_output, 0); + ok(!ret, "got %d\n", ret); + ret = WaitForSingleObject(unbound_input, 0); + ok(!ret, "got %d\n", ret); + + parent_event = CreateEventA(NULL, FALSE, FALSE, "winetest_unbound_handles_parent"); + child_event = CreateEventA(NULL, FALSE, FALSE, "winetest_unbound_handles_child"); + + winetest_get_mainargs(&argv); + sprintf(buf, "\"%s\" console unbound_handles %#lx %#Ix %#Ix", + argv[0], GetCurrentProcessId(), (UINT_PTR)unbound_input, (UINT_PTR)unbound_output); + ret = CreateProcessA(NULL, buf, NULL, NULL, FALSE, DETACHED_PROCESS, NULL, NULL, &si, &info); + ok(ret, "got error %lu\n", GetLastError()); + + /* Wait for child to test our handles while there's input available. */ + ret = WaitForSingleObject(child_event, 1000); + todo_wine ok(!ret, "got %d\n", ret); + + /* Read the input. */ + check_mouse_event(unbound_input); + + ret = WaitForSingleObject(output, 0); + ok(ret == WAIT_TIMEOUT, "got %d\n", ret); + ret = WaitForSingleObject(unbound_output, 0); + ok(ret == WAIT_TIMEOUT, "got %d\n", ret); + ret = WaitForSingleObject(unbound_input, 0); + ok(ret == WAIT_TIMEOUT, "got %d\n", ret); + + /* Test that our handles are now designaled in the child. */ + SetEvent(parent_event); + + CloseHandle(info.hThread); + wait_child_process(info.hProcess); + CloseHandle(info.hProcess); + + ret = WriteConsoleInputW(input, &ir, 1, &size); + ok(ret, "got error %lu\n", GetLastError()); + ret = FreeConsole(); ok(ret, "FreeConsole failed: %lu\n", GetLastError()); @@ -4170,6 +4337,68 @@ static void test_FreeConsole(void) (GetLastError() == ERROR_INVALID_HANDLE || broken(GetLastError() == ERROR_FILE_NOT_FOUND /* winxp */)), "CreateFileA failed: %lu\n", GetLastError()); + CloseHandle(input); + CloseHandle(orig_output); + + ret = WaitForSingleObject(output, 0); + todo_wine ok(!ret, "got %d\n", ret); + ret = WaitForSingleObject(unbound_output, 0); + todo_wine ok(ret == WAIT_TIMEOUT, "got %d\n", ret); + ret = WaitForSingleObject(unbound_input, 0); + todo_wine ok(ret == WAIT_TIMEOUT, "got %d\n", ret); + + /* Create a new console. + * + * The previous unbound handles remain associated with the old console. + * As with the cross-process case, they can be read from, which reads from + * the current process's active console, but the signaled state does not + * reflect the current console. + * + * For some inexplicable reason, allocating the new console also causes the + * original orphaned handles to become signaled. */ + ret = AllocConsole(); + ok(ret, "got error %lu\n", GetLastError()); + + ret = WaitForSingleObject(output, 0); + todo_wine ok(!ret, "got %d\n", ret); + ret = WaitForSingleObject(unbound_output, 0); + todo_wine ok(!ret, "got %d\n", ret); + ret = WaitForSingleObject(unbound_input, 0); + todo_wine ok(!ret, "got %d\n", ret); + + input = CreateFileA("CONIN$", GENERIC_READ | GENERIC_WRITE, 0, NULL, OPEN_EXISTING, 0, 0); + ok(input != INVALID_HANDLE_VALUE, "got error %lu\n", GetLastError()); + ret = WriteConsoleInputW(input, &ir, 1, &size); + ok(ret, "got error %lu\n", GetLastError()); + + unbound_input2 = create_unbound_handle(FALSE, TRUE); + + ret = WaitForSingleObject(unbound_input2, 0); + ok(!ret, "got %d\n", ret); + + check_mouse_event(unbound_input); + + ret = WaitForSingleObject(output, 0); + todo_wine ok(!ret, "got %d\n", ret); + ret = WaitForSingleObject(unbound_output, 0); + todo_wine ok(!ret, "got %d\n", ret); + ret = WaitForSingleObject(unbound_input, 0); + todo_wine ok(!ret, "got %d\n", ret); + + ret = WaitForSingleObject(unbound_input2, 0); + ok(ret == WAIT_TIMEOUT, "got %d\n", ret); + + ret = FreeConsole(); + ok(ret, "got error %lu\n", GetLastError()); + CloseHandle(input); + + ret = WaitForSingleObject(output, 0); + todo_wine ok(!ret, "got %d\n", ret); + ret = WaitForSingleObject(unbound_output, 0); + todo_wine ok(!ret, "got %d\n", ret); + ret = WaitForSingleObject(unbound_input, 0); + todo_wine ok(!ret, "got %d\n", ret); + handle = CreateConsoleScreenBuffer(GENERIC_READ | GENERIC_WRITE, FILE_SHARE_READ | FILE_SHARE_WRITE, NULL, CONSOLE_TEXTMODE_BUFFER, NULL); @@ -4252,6 +4481,7 @@ static void test_FreeConsole(void) CloseHandle(unbound_input); CloseHandle(unbound_output); + CloseHandle(output); } static void test_SetConsoleScreenBufferInfoEx(HANDLE std_output) @@ -5473,6 +5703,12 @@ START_TEST(console) return; } + if (argc == 6 && !strcmp(argv[2], "unbound_handles")) + { + test_unbound_handles_child(strtoul(argv[3], NULL, 16), strtoull(argv[4], NULL, 16), strtoull(argv[5], NULL, 16)); + return; + } + test_current = argc >= 3 && !strcmp(argv[2], "--current"); using_pseudo_console = argc >= 3 && !strcmp(argv[2], "--pseudo-console"); @@ -5665,7 +5901,7 @@ START_TEST(console) test_pseudo_console(); test_AttachConsole(hConOut); test_AllocConsole(); - test_FreeConsole(); + test_FreeConsole(hConIn, hConOut); test_condrv_server_as_root_directory(); test_CreateProcessCUI(); test_CtrlHandlerSubsystem(); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7608
From: Elizabeth Figura <zfigura(a)codeweavers.com> --- dlls/kernel32/tests/console.c | 9 +++++++-- server/console.c | 14 ++++++++++++++ 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/dlls/kernel32/tests/console.c b/dlls/kernel32/tests/console.c index dd879725658..b879b7dcd80 100644 --- a/dlls/kernel32/tests/console.c +++ b/dlls/kernel32/tests/console.c @@ -4185,8 +4185,13 @@ static void test_unbound_handles_child(DWORD parent_pid, UINT_PTR parent_input_i FILE_WRITE_ATTRIBUTES, &attr, &iosb, NULL, FILE_ATTRIBUTE_NORMAL, FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, FILE_CREATE, FILE_NON_DIRECTORY_FILE | FILE_SYNCHRONOUS_IO_NONALERT, NULL, 0 ); - todo_wine ok(status == STATUS_INVALID_HANDLE, "got %#lx\n", status); - if (!status) NtClose( unbound_input ); + ok(status == STATUS_INVALID_HANDLE, "got %#lx\n", status); + RtlInitUnicodeString( &name, L"\\Device\\ConDrv\\Output" ); + status = NtCreateFile( &unbound_input, FILE_READ_DATA | FILE_WRITE_DATA | SYNCHRONIZE | FILE_READ_ATTRIBUTES | + FILE_WRITE_ATTRIBUTES, &attr, &iosb, NULL, FILE_ATTRIBUTE_NORMAL, + FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, FILE_CREATE, + FILE_NON_DIRECTORY_FILE | FILE_SYNCHRONOUS_IO_NONALERT, NULL, 0 ); + ok(status == STATUS_INVALID_HANDLE, "got %#lx\n", status); /* Allocate a console. */ diff --git a/server/console.c b/server/console.c index c3e7bf57e92..16093fed4bc 100644 --- a/server/console.c +++ b/server/console.c @@ -1314,6 +1314,13 @@ static struct object *console_device_lookup_name( struct object *obj, struct uni if (name->len == sizeof(inputW) && !memcmp( name->str, inputW, name->len )) { struct console_input *console_input; + + if (!current->process->console) + { + set_error( STATUS_INVALID_HANDLE ); + return NULL; + } + name->len = 0; if (!(console_input = alloc_object( &console_input_ops ))) return NULL; console_input->fd = alloc_pseudo_fd( &console_input_fd_ops, &console_input->obj, @@ -1329,6 +1336,13 @@ static struct object *console_device_lookup_name( struct object *obj, struct uni if (name->len == sizeof(outputW) && !memcmp( name->str, outputW, name->len )) { struct console_output *console_output; + + if (!current->process->console) + { + set_error( STATUS_INVALID_HANDLE ); + return NULL; + } + name->len = 0; if (!(console_output = alloc_object( &console_output_ops ))) return NULL; console_output->fd = alloc_pseudo_fd( &console_output_fd_ops, &console_output->obj, -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7608
From: Elizabeth Figura <zfigura(a)codeweavers.com> --- server/console.c | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/server/console.c b/server/console.c index 16093fed4bc..c081006590d 100644 --- a/server/console.c +++ b/server/console.c @@ -61,6 +61,7 @@ struct console struct fd *fd; /* for bare console, attached input fd */ struct async_queue ioctl_q; /* ioctl queue */ struct async_queue read_q; /* read queue */ + struct list screen_buffers;/* attached screen buffers */ }; static void console_dump( struct object *obj, int verbose ); @@ -462,8 +463,6 @@ static const struct fd_ops console_connection_fd_ops = default_fd_reselect_async /* reselect_async */ }; -static struct list screen_buffer_list = LIST_INIT(screen_buffer_list); - static int queue_host_ioctl( struct console_server *server, unsigned int code, unsigned int output, struct async *async, struct async_queue *queue ); @@ -539,6 +538,7 @@ static struct object *create_console(void) console->server = NULL; console->fd = NULL; console->last_id = 0; + list_init( &console->screen_buffers ); init_async_queue( &console->ioctl_q ); init_async_queue( &console->read_q ); @@ -639,7 +639,7 @@ static struct object *create_screen_buffer( struct console *console ) screen_buffer->id = ++console->last_id; screen_buffer->input = console; init_async_queue( &screen_buffer->ioctl_q ); - list_add_head( &screen_buffer_list, &screen_buffer->entry ); + list_add_head( &console->screen_buffers, &screen_buffer->entry ); screen_buffer->fd = alloc_pseudo_fd( &screen_buffer_fd_ops, &screen_buffer->obj, FILE_SYNCHRONOUS_IO_NONALERT ); @@ -755,10 +755,8 @@ static void console_destroy( struct object *obj ) if (console->active) release_object( console->active ); console->active = NULL; - LIST_FOR_EACH_ENTRY( curr, &screen_buffer_list, struct screen_buffer, entry ) - { - if (curr->input == console) curr->input = NULL; - } + LIST_FOR_EACH_ENTRY( curr, &console->screen_buffers, struct screen_buffer, entry ) + curr->input = NULL; free_async_queue( &console->ioctl_q ); free_async_queue( &console->read_q ); @@ -831,10 +829,13 @@ static void screen_buffer_destroy( struct object *obj ) assert( obj->ops == &screen_buffer_ops ); - list_remove( &screen_buffer->entry ); - if (screen_buffer->input && screen_buffer->input->server) - queue_host_ioctl( screen_buffer->input->server, IOCTL_CONDRV_CLOSE_OUTPUT, - screen_buffer->id, NULL, NULL ); + if (screen_buffer->input) + { + list_remove( &screen_buffer->entry ); + if (screen_buffer->input->server) + queue_host_ioctl( screen_buffer->input->server, IOCTL_CONDRV_CLOSE_OUTPUT, + screen_buffer->id, NULL, NULL ); + } if (screen_buffer->fd) release_object( screen_buffer->fd ); free_async_queue( &screen_buffer->ioctl_q ); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7608
From: Elizabeth Figura <zfigura(a)codeweavers.com> Wait on the handle directly instead of delegating to the console's wait queue. --- dlls/kernel32/tests/console.c | 8 ++++---- server/console.c | 35 +++++++++++++++++++---------------- 2 files changed, 23 insertions(+), 20 deletions(-) diff --git a/dlls/kernel32/tests/console.c b/dlls/kernel32/tests/console.c index b879b7dcd80..6126a1ffb6d 100644 --- a/dlls/kernel32/tests/console.c +++ b/dlls/kernel32/tests/console.c @@ -4346,7 +4346,7 @@ static void test_FreeConsole(HANDLE input, HANDLE orig_output) CloseHandle(orig_output); ret = WaitForSingleObject(output, 0); - todo_wine ok(!ret, "got %d\n", ret); + ok(!ret, "got %d\n", ret); ret = WaitForSingleObject(unbound_output, 0); todo_wine ok(ret == WAIT_TIMEOUT, "got %d\n", ret); ret = WaitForSingleObject(unbound_input, 0); @@ -4365,7 +4365,7 @@ static void test_FreeConsole(HANDLE input, HANDLE orig_output) ok(ret, "got error %lu\n", GetLastError()); ret = WaitForSingleObject(output, 0); - todo_wine ok(!ret, "got %d\n", ret); + ok(!ret, "got %d\n", ret); ret = WaitForSingleObject(unbound_output, 0); todo_wine ok(!ret, "got %d\n", ret); ret = WaitForSingleObject(unbound_input, 0); @@ -4384,7 +4384,7 @@ static void test_FreeConsole(HANDLE input, HANDLE orig_output) check_mouse_event(unbound_input); ret = WaitForSingleObject(output, 0); - todo_wine ok(!ret, "got %d\n", ret); + ok(!ret, "got %d\n", ret); ret = WaitForSingleObject(unbound_output, 0); todo_wine ok(!ret, "got %d\n", ret); ret = WaitForSingleObject(unbound_input, 0); @@ -4398,7 +4398,7 @@ static void test_FreeConsole(HANDLE input, HANDLE orig_output) CloseHandle(input); ret = WaitForSingleObject(output, 0); - todo_wine ok(!ret, "got %d\n", ret); + ok(!ret, "got %d\n", ret); ret = WaitForSingleObject(unbound_output, 0); todo_wine ok(!ret, "got %d\n", ret); ret = WaitForSingleObject(unbound_input, 0); diff --git a/server/console.c b/server/console.c index c081006590d..450e5ada368 100644 --- a/server/console.c +++ b/server/console.c @@ -215,7 +215,7 @@ struct screen_buffer static void screen_buffer_dump( struct object *obj, int verbose ); static void screen_buffer_destroy( struct object *obj ); -static int screen_buffer_add_queue( struct object *obj, struct wait_queue_entry *entry ); +static int screen_buffer_signaled( struct object *obj, struct wait_queue_entry *entry ); static struct fd *screen_buffer_get_fd( struct object *obj ); static struct object *screen_buffer_open_file( struct object *obj, unsigned int access, unsigned int sharing, unsigned int options ); @@ -225,10 +225,10 @@ static const struct object_ops screen_buffer_ops = sizeof(struct screen_buffer), /* size */ &file_type, /* type */ screen_buffer_dump, /* dump */ - screen_buffer_add_queue, /* add_queue */ - NULL, /* remove_queue */ - NULL, /* signaled */ - NULL, /* satisfied */ + add_queue, /* add_queue */ + remove_queue, /* remove_queue */ + screen_buffer_signaled, /* signaled */ + no_satisfied, /* satisfied */ no_signal, /* signal */ screen_buffer_get_fd, /* get_fd */ default_map_access, /* map_access */ @@ -756,7 +756,10 @@ static void console_destroy( struct object *obj ) console->active = NULL; LIST_FOR_EACH_ENTRY( curr, &console->screen_buffers, struct screen_buffer, entry ) + { curr->input = NULL; + wake_up( &curr->obj, 0 ); + } free_async_queue( &console->ioctl_q ); free_async_queue( &console->read_q ); @@ -840,21 +843,18 @@ static void screen_buffer_destroy( struct object *obj ) free_async_queue( &screen_buffer->ioctl_q ); } -static struct object *screen_buffer_open_file( struct object *obj, unsigned int access, - unsigned int sharing, unsigned int options ) +static int screen_buffer_signaled( struct object *obj, struct wait_queue_entry *entry ) { - return grab_object( obj ); + struct screen_buffer *screen_buffer = (struct screen_buffer *)obj; + assert( obj->ops == &screen_buffer_ops ); + if (!screen_buffer->input) return 1; + return screen_buffer->input->signaled; } -static int screen_buffer_add_queue( struct object *obj, struct wait_queue_entry *entry ) +static struct object *screen_buffer_open_file( struct object *obj, unsigned int access, + unsigned int sharing, unsigned int options ) { - struct screen_buffer *screen_buffer = (struct screen_buffer*)obj; - if (!screen_buffer->input) - { - set_error( STATUS_ACCESS_DENIED ); - return 0; - } - return console_add_queue( &screen_buffer->input->obj, entry ); + return grab_object( obj ); } static struct fd *screen_buffer_get_fd( struct object *obj ) @@ -1543,6 +1543,7 @@ struct object *create_console_device( struct object *root, const struct unicode_ DECL_HANDLER(get_next_console_request) { struct console_host_ioctl *ioctl = NULL, *next; + struct screen_buffer *screen_buffer; struct console_server *server; struct iosb *iosb = NULL; @@ -1563,6 +1564,8 @@ DECL_HANDLER(get_next_console_request) { server->console->signaled = 1; wake_up( &server->console->obj, 0 ); + LIST_FOR_EACH_ENTRY( screen_buffer, &server->console->screen_buffers, struct screen_buffer, entry ) + wake_up( &screen_buffer->obj, 0 ); } if (req->read) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7608
From: Elizabeth Figura <zfigura(a)codeweavers.com> Do not delegate to the current process's queue, either, but use the queue for this object. I/O acts on the current process's console, but the handle signaled state is the same across all handles to this object, and reflects the signaled state of the console of the process that created the object. --- dlls/kernel32/tests/console.c | 8 ++++---- server/console.c | 34 +++++++++++++++++++++++----------- 2 files changed, 27 insertions(+), 15 deletions(-) diff --git a/dlls/kernel32/tests/console.c b/dlls/kernel32/tests/console.c index 6126a1ffb6d..211a1990541 100644 --- a/dlls/kernel32/tests/console.c +++ b/dlls/kernel32/tests/console.c @@ -4168,7 +4168,7 @@ static void test_unbound_handles_child(DWORD parent_pid, UINT_PTR parent_input_i ok(ret, "got error %lu\n", GetLastError()); ret = WaitForSingleObject(parent_input, 0); - todo_wine ok(!ret, "got %d\n", ret); + ok(!ret, "got %d\n", ret); ret = WaitForSingleObject(parent_output, 0); todo_wine ok(!ret, "got %d\n", ret); @@ -4227,7 +4227,7 @@ static void test_unbound_handles_child(DWORD parent_pid, UINT_PTR parent_input_i ok(ret == TRUE, "got error %lu\n", GetLastError()); ret = WaitForSingleObject(unbound_input, 0); - todo_wine ok(ret == WAIT_TIMEOUT, "got %d\n", ret); + ok(ret == WAIT_TIMEOUT, "got %d\n", ret); CloseHandle(unbound_input); ret = WaitForSingleObject(parent_input, 0); @@ -4241,7 +4241,7 @@ static void test_unbound_handles_child(DWORD parent_pid, UINT_PTR parent_input_i ok(!ret, "got %d\n", ret); ret = WaitForSingleObject(parent_input, 0); - todo_wine ok(ret == WAIT_TIMEOUT, "got %d\n", ret); + ok(ret == WAIT_TIMEOUT, "got %d\n", ret); ret = WaitForSingleObject(parent_output, 0); todo_wine ok(ret == WAIT_TIMEOUT, "got %d\n", ret); } @@ -4350,7 +4350,7 @@ static void test_FreeConsole(HANDLE input, HANDLE orig_output) ret = WaitForSingleObject(unbound_output, 0); todo_wine ok(ret == WAIT_TIMEOUT, "got %d\n", ret); ret = WaitForSingleObject(unbound_input, 0); - todo_wine ok(ret == WAIT_TIMEOUT, "got %d\n", ret); + ok(ret == WAIT_TIMEOUT, "got %d\n", ret); /* Create a new console. * diff --git a/server/console.c b/server/console.c index 450e5ada368..658f427e2b7 100644 --- a/server/console.c +++ b/server/console.c @@ -62,6 +62,7 @@ struct console struct async_queue ioctl_q; /* ioctl queue */ struct async_queue read_q; /* read queue */ struct list screen_buffers;/* attached screen buffers */ + struct list inputs; /* attached console inputs */ }; static void console_dump( struct object *obj, int verbose ); @@ -297,12 +298,14 @@ struct console_input { struct object obj; /* object header */ struct fd *fd; /* pseudo-fd */ + struct list entry; /* entry in console->inputs */ + struct console *console; /* associated console at creation time */ }; static void console_input_dump( struct object *obj, int verbose ); +static int console_input_signaled( struct object *obj, struct wait_queue_entry *entry ); static struct object *console_input_open_file( struct object *obj, unsigned int access, unsigned int sharing, unsigned int options ); -static int console_input_add_queue( struct object *obj, struct wait_queue_entry *entry ); static struct fd *console_input_get_fd( struct object *obj ); static void console_input_destroy( struct object *obj ); @@ -311,9 +314,9 @@ static const struct object_ops console_input_ops = sizeof(struct console_input), /* size */ &device_type, /* type */ console_input_dump, /* dump */ - console_input_add_queue, /* add_queue */ - NULL, /* remove_queue */ - NULL, /* signaled */ + add_queue, /* add_queue */ + remove_queue, /* remove_queue */ + console_input_signaled, /* signaled */ no_satisfied, /* satisfied */ no_signal, /* signal */ console_input_get_fd, /* get_fd */ @@ -539,6 +542,7 @@ static struct object *create_console(void) console->fd = NULL; console->last_id = 0; list_init( &console->screen_buffers ); + list_init( &console->inputs ); init_async_queue( &console->ioctl_q ); init_async_queue( &console->read_q ); @@ -742,6 +746,7 @@ static void console_dump( struct object *obj, int verbose ) static void console_destroy( struct object *obj ) { struct console *console = (struct console *)obj; + struct console_input *input; struct screen_buffer *curr; assert( obj->ops == &console_ops ); @@ -761,6 +766,9 @@ static void console_destroy( struct object *obj ) wake_up( &curr->obj, 0 ); } + LIST_FOR_EACH_ENTRY( input, &console->inputs, struct console_input, entry ) + input->console = NULL; + free_async_queue( &console->ioctl_q ); free_async_queue( &console->read_q ); if (console->fd) @@ -1331,6 +1339,8 @@ static struct object *console_device_lookup_name( struct object *obj, struct uni release_object( console_input ); return NULL; } + console_input->console = current->process->console; + list_add_head( ¤t->process->console->inputs, &console_input->entry ); return &console_input->obj; } @@ -1406,14 +1416,12 @@ static void console_input_dump( struct object *obj, int verbose ) fputs( "console Input device\n", stderr ); } -static int console_input_add_queue( struct object *obj, struct wait_queue_entry *entry ) +static int console_input_signaled( struct object *obj, struct wait_queue_entry *entry ) { - if (!current->process->console) - { - set_error( STATUS_ACCESS_DENIED ); - return 0; - } - return console_add_queue( ¤t->process->console->obj, entry ); + struct console_input *console_input = (struct console_input *)obj; + assert( obj->ops == &console_input_ops ); + if (!console_input->console) return 0; + return console_input->console->signaled; } static struct fd *console_input_get_fd( struct object *obj ) @@ -1435,6 +1443,7 @@ static void console_input_destroy( struct object *obj ) assert( obj->ops == &console_input_ops ); if (console_input->fd) release_object( console_input->fd ); + if (console_input->console) list_remove( &console_input->entry ); } static void console_input_ioctl( struct fd *fd, ioctl_code_t code, struct async *async ) @@ -1545,6 +1554,7 @@ DECL_HANDLER(get_next_console_request) struct console_host_ioctl *ioctl = NULL, *next; struct screen_buffer *screen_buffer; struct console_server *server; + struct console_input *input; struct iosb *iosb = NULL; server = (struct console_server *)get_handle_obj( current->process, req->handle, 0, &console_server_ops ); @@ -1566,6 +1576,8 @@ DECL_HANDLER(get_next_console_request) wake_up( &server->console->obj, 0 ); LIST_FOR_EACH_ENTRY( screen_buffer, &server->console->screen_buffers, struct screen_buffer, entry ) wake_up( &screen_buffer->obj, 0 ); + LIST_FOR_EACH_ENTRY( input, &server->console->inputs, struct console_input, entry ) + wake_up( &input->obj, 0 ); } if (req->read) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7608
From: Elizabeth Figura <zfigura(a)codeweavers.com> Do not delegate to the current process's queue, either, but use the queue for this object. I/O acts on the current process's console, but the handle signaled state is the same across all handles to this object, and reflects the signaled state of the console of the process that created the object. --- dlls/kernel32/tests/console.c | 6 +++--- server/console.c | 34 +++++++++++++++++++++++----------- 2 files changed, 26 insertions(+), 14 deletions(-) diff --git a/dlls/kernel32/tests/console.c b/dlls/kernel32/tests/console.c index 211a1990541..ea5339c936f 100644 --- a/dlls/kernel32/tests/console.c +++ b/dlls/kernel32/tests/console.c @@ -4170,7 +4170,7 @@ static void test_unbound_handles_child(DWORD parent_pid, UINT_PTR parent_input_i ret = WaitForSingleObject(parent_input, 0); ok(!ret, "got %d\n", ret); ret = WaitForSingleObject(parent_output, 0); - todo_wine ok(!ret, "got %d\n", ret); + ok(!ret, "got %d\n", ret); ret = PeekConsoleInputW(parent_input, &ir, 1, &count); ok(!ret, "got %d\n", ret); @@ -4243,7 +4243,7 @@ static void test_unbound_handles_child(DWORD parent_pid, UINT_PTR parent_input_i ret = WaitForSingleObject(parent_input, 0); ok(ret == WAIT_TIMEOUT, "got %d\n", ret); ret = WaitForSingleObject(parent_output, 0); - todo_wine ok(ret == WAIT_TIMEOUT, "got %d\n", ret); + ok(ret == WAIT_TIMEOUT, "got %d\n", ret); } static void test_FreeConsole(HANDLE input, HANDLE orig_output) @@ -4348,7 +4348,7 @@ static void test_FreeConsole(HANDLE input, HANDLE orig_output) ret = WaitForSingleObject(output, 0); ok(!ret, "got %d\n", ret); ret = WaitForSingleObject(unbound_output, 0); - todo_wine ok(ret == WAIT_TIMEOUT, "got %d\n", ret); + ok(ret == WAIT_TIMEOUT, "got %d\n", ret); ret = WaitForSingleObject(unbound_input, 0); ok(ret == WAIT_TIMEOUT, "got %d\n", ret); diff --git a/server/console.c b/server/console.c index 658f427e2b7..047e2c86a61 100644 --- a/server/console.c +++ b/server/console.c @@ -63,6 +63,7 @@ struct console struct async_queue read_q; /* read queue */ struct list screen_buffers;/* attached screen buffers */ struct list inputs; /* attached console inputs */ + struct list outputs; /* attached console outputs */ }; static void console_dump( struct object *obj, int verbose ); @@ -357,10 +358,12 @@ struct console_output { struct object obj; /* object header */ struct fd *fd; /* pseudo-fd */ + struct list entry; /* entry in console->outputs */ + struct console *console; /* associated console at creation time */ }; static void console_output_dump( struct object *obj, int verbose ); -static int console_output_add_queue( struct object *obj, struct wait_queue_entry *entry ); +static int console_output_signaled( struct object *obj, struct wait_queue_entry *entry ); static struct fd *console_output_get_fd( struct object *obj ); static struct object *console_output_open_file( struct object *obj, unsigned int access, unsigned int sharing, unsigned int options ); @@ -371,9 +374,9 @@ static const struct object_ops console_output_ops = sizeof(struct console_output), /* size */ &device_type, /* type */ console_output_dump, /* dump */ - console_output_add_queue, /* add_queue */ - NULL, /* remove_queue */ - NULL, /* signaled */ + add_queue, /* add_queue */ + remove_queue, /* remove_queue */ + console_output_signaled, /* signaled */ no_satisfied, /* satisfied */ no_signal, /* signal */ console_output_get_fd, /* get_fd */ @@ -543,6 +546,7 @@ static struct object *create_console(void) console->last_id = 0; list_init( &console->screen_buffers ); list_init( &console->inputs ); + list_init( &console->outputs ); init_async_queue( &console->ioctl_q ); init_async_queue( &console->read_q ); @@ -746,6 +750,7 @@ static void console_dump( struct object *obj, int verbose ) static void console_destroy( struct object *obj ) { struct console *console = (struct console *)obj; + struct console_output *output; struct console_input *input; struct screen_buffer *curr; @@ -769,6 +774,9 @@ static void console_destroy( struct object *obj ) LIST_FOR_EACH_ENTRY( input, &console->inputs, struct console_input, entry ) input->console = NULL; + LIST_FOR_EACH_ENTRY( output, &console->outputs, struct console_output, entry ) + output->console = NULL; + free_async_queue( &console->ioctl_q ); free_async_queue( &console->read_q ); if (console->fd) @@ -1363,6 +1371,8 @@ static struct object *console_device_lookup_name( struct object *obj, struct uni release_object( console_output ); return NULL; } + console_output->console = current->process->console; + list_add_head( ¤t->process->console->outputs, &console_output->entry ); return &console_output->obj; } @@ -1487,14 +1497,12 @@ static void console_output_dump( struct object *obj, int verbose ) fputs( "console Output device\n", stderr ); } -static int console_output_add_queue( struct object *obj, struct wait_queue_entry *entry ) +static int console_output_signaled( struct object *obj, struct wait_queue_entry *entry ) { - if (!current->process->console || !current->process->console->active) - { - set_error( STATUS_ACCESS_DENIED ); - return 0; - } - return console_add_queue( ¤t->process->console->obj, entry ); + struct console_output *console_output = (struct console_output *)obj; + assert( obj->ops == &console_output_ops ); + if (!console_output->console) return 0; + return console_output->console->signaled; } static struct fd *console_output_get_fd( struct object *obj ) @@ -1516,6 +1524,7 @@ static void console_output_destroy( struct object *obj ) assert( obj->ops == &console_output_ops ); if (console_output->fd) release_object( console_output->fd ); + if (console_output->console) list_remove( &console_output->entry ); } static void console_output_ioctl( struct fd *fd, ioctl_code_t code, struct async *async ) @@ -1554,6 +1563,7 @@ DECL_HANDLER(get_next_console_request) struct console_host_ioctl *ioctl = NULL, *next; struct screen_buffer *screen_buffer; struct console_server *server; + struct console_output *output; struct console_input *input; struct iosb *iosb = NULL; @@ -1578,6 +1588,8 @@ DECL_HANDLER(get_next_console_request) wake_up( &screen_buffer->obj, 0 ); LIST_FOR_EACH_ENTRY( input, &server->console->inputs, struct console_input, entry ) wake_up( &input->obj, 0 ); + LIST_FOR_EACH_ENTRY( output, &server->console->outputs, struct console_output, entry ) + wake_up( &output->obj, 0 ); } if (req->read) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7608
The code looks good to me, but Windows CI failures seem relevant. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7608#note_98591
participants (3)
-
Elizabeth Figura -
Elizabeth Figura (＠zfigura)
-
Jacek Caban (＠jacek)