[PATCH] server/console: ensure conhost has created the TTY input stream before waiting
some PE executables (like mingw's gdb port) just do something like: - WaitForSingleObject(GetStdHandle(STD_INPUT_HANDLE), INFINITE) and hang for ever (the read operations are done *after* the wait operation succeeds) (of course, the real wait operation is more complex, but the problematic part boils down to that) this hangs for ever because conhost's main input thread hasn't been started yet this patch sends once a peek IOCTL before waiting, hence ensuring conhost's main input stream has been started this lets i686 and x64_86 mingw's gdb port to accept user's input (this is second attempt to fix this bug, based on Jacek's feedback on first try) A+ Signed-off-by: Eric Pouech <eric.pouech(a)gmail.com> --- server/console.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/server/console.c b/server/console.c index 977c428cf9f..b4ef1d21874 100644 --- a/server/console.c +++ b/server/console.c @@ -136,6 +136,7 @@ struct console_server struct list read_queue; /* blocking read queue */ int busy; /* flag if server processing an ioctl */ int term_fd; /* UNIX terminal fd */ + int once_input; /* flag if input thread has already been requested */ struct termios termios; /* original termios */ }; @@ -892,6 +893,7 @@ static struct object *create_console_server( void ) server->console = NULL; server->busy = 0; server->term_fd = -1; + server->once_input = 0; list_init( &server->queue ); list_init( &server->read_queue ); server->fd = alloc_pseudo_fd( &console_server_fd_ops, &server->obj, FILE_SYNCHRONOUS_IO_NONALERT ); @@ -1310,11 +1312,19 @@ static void console_input_dump( struct object *obj, int verbose ) static int console_input_add_queue( struct object *obj, struct wait_queue_entry *entry ) { + struct console_server* server; if (!current->process->console) { set_error( STATUS_ACCESS_DENIED ); return 0; } + server = current->process->console->server; + /* before waiting, ensure conhost's input thread has been started */ + if ( server && !server->once_input ) + { + queue_host_ioctl( server, IOCTL_CONDRV_PEEK, 0, NULL, NULL); + server->once_input = 1; + } return add_queue( ¤t->process->console->obj, entry ); }
Hi Eric, On 12/8/21 3:56 PM, Eric Pouech wrote:
some PE executables (like mingw's gdb port) just do something like: - WaitForSingleObject(GetStdHandle(STD_INPUT_HANDLE), INFINITE) and hang for ever (the read operations are done*after* the wait operation succeeds) (of course, the real wait operation is more complex, but the problematic part boils down to that)
this hangs for ever because conhost's main input thread hasn't been started yet
this patch sends once a peek IOCTL before waiting, hence ensuring conhost's main input stream has been started
this lets i686 and x64_86 mingw's gdb port to accept user's input
(this is second attempt to fix this bug, based on Jacek's feedback on first try)
I think this is a good solution in general. Looking closer, we will also need it for console_ops as well. Those are two different objects: one is meant for handles operating on currently attached console while the other is always attached to specific console. I'd suggest setting the initialization flag in IOCTL_CONDRV_SETUP_INPUT as a cheap optimization for cases when input is read before waiting. Please keep white space changes consistent with server style. Thanks, Jacek
Some PE executables (like mingw's gdb port) just do something like: - WaitForSingleObject(GetStdHandle(STD_INPUT_HANDLE), INFINITE) and hang for ever (the read operations are done *after* the wait operation succeeds) (of course, the real wait operation is more complex, but the problematic part boils down to that) This hangs for ever because conhost's main input thread hasn't been started yet. This patch sends once a peek IOCTL before waiting on console input objects, hence ensuring conhost's main input stream has been started. This lets i686 and x64_86 mingw's gdb port to accept user's input. V2 => V3 (including Jacek's comments) - space style conventions - move once_input flag setting into CONDRV_SETUP_INPUT handling - applied same change to console objects' add_queue as to console_input objects (and let the later call into the former) - made console_server flags' storage a bit field A+ Signed-off-by: Eric Pouech <eric.pouech(a)gmail.com> --- server/console.c | 43 ++++++++++++++++++++++++++++++------------- 1 file changed, 30 insertions(+), 13 deletions(-) diff --git a/server/console.c b/server/console.c index 977c428cf9f..5224324c7f6 100644 --- a/server/console.c +++ b/server/console.c @@ -71,13 +71,14 @@ static struct object *console_lookup_name( struct object *obj, struct unicode_st unsigned int attr, struct object *root ); static struct object *console_open_file( struct object *obj, unsigned int access, unsigned int sharing, unsigned int options ); +static int console_add_queue( struct object *obj, struct wait_queue_entry *entry ); static const struct object_ops console_ops = { sizeof(struct console), /* size */ &file_type, /* type */ console_dump, /* dump */ - add_queue, /* add_queue */ + console_add_queue, /* add_queue */ remove_queue, /* remove_queue */ console_signaled, /* signaled */ no_satisfied, /* satisfied */ @@ -129,14 +130,15 @@ struct console_host_ioctl struct console_server { - struct object obj; /* object header */ - struct fd *fd; /* pseudo-fd for ioctls */ - struct console *console; /* attached console */ - struct list queue; /* ioctl queue */ - struct list read_queue; /* blocking read queue */ - int busy; /* flag if server processing an ioctl */ - int term_fd; /* UNIX terminal fd */ - struct termios termios; /* original termios */ + struct object obj; /* object header */ + struct fd *fd; /* pseudo-fd for ioctls */ + struct console *console; /* attached console */ + struct list queue; /* ioctl queue */ + struct list read_queue; /* blocking read queue */ + unsigned int busy : 1; /* flag if server processing an ioctl */ + unsigned int once_input : 1; /* flag if input thread has already been requested */ + int term_fd; /* UNIX terminal fd */ + struct termios termios; /* original termios */ }; static void console_server_dump( struct object *obj, int verbose ); @@ -462,6 +464,19 @@ static const struct fd_ops console_connection_fd_ops = 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 ); + +static int console_add_queue( struct object *obj, struct wait_queue_entry *entry ) +{ + struct console *console = (struct console*)obj; + assert( obj->ops == &console_ops ); + /* before waiting, ensure conhost's input thread has been started */ + if (console->server && !console->server->once_input) + queue_host_ioctl( console->server, IOCTL_CONDRV_PEEK, 0, NULL, NULL ); + return add_queue( &console->obj, entry ); +} + static int console_signaled( struct object *obj, struct wait_queue_entry *entry ) { struct console *console = (struct console*)obj; @@ -889,9 +904,10 @@ static struct object *create_console_server( void ) struct console_server *server; if (!(server = alloc_object( &console_server_ops ))) return NULL; - server->console = NULL; - server->busy = 0; - server->term_fd = -1; + server->console = NULL; + server->busy = 0; + server->once_input = 0; + server->term_fd = -1; list_init( &server->queue ); list_init( &server->read_queue ); server->fd = alloc_pseudo_fd( &console_server_fd_ops, &server->obj, FILE_SYNCHRONOUS_IO_NONALERT ); @@ -1086,6 +1102,7 @@ static void console_server_ioctl( struct fd *fd, ioctl_code_t code, struct async struct file *file; int unix_fd; + server->once_input = 1; if (get_req_data_size() != sizeof(unsigned int) || get_reply_max_size()) { set_error( STATUS_INVALID_PARAMETER ); @@ -1315,7 +1332,7 @@ static int console_input_add_queue( struct object *obj, struct wait_queue_entry set_error( STATUS_ACCESS_DENIED ); return 0; } - return add_queue( ¤t->process->console->obj, entry ); + return console_add_queue( ¤t->process->console->obj, entry ); } static struct fd *console_input_get_fd( struct object *obj )
Hi Eric, On 12/15/21 10:46 AM, Eric Pouech wrote:
+static int console_add_queue( struct object *obj, struct wait_queue_entry *entry ) +{ + struct console*console = (struct console*)obj; + assert( obj->ops == &console_ops ); + /* before waiting, ensure conhost's input thread has been started */ + if (console->server && !console->server->once_input) + queue_host_ioctl( console->server, IOCTL_CONDRV_PEEK, 0, NULL, NULL );
I think I mislead you, sorry about that. We still need to set once_input here because non-Unix consoles will never call IOCTL_CONDRV_SETUP_INPUT. And instead of setting it in IOCTL_CONDRV_SETUP_INPUT, we could simply add term_fd != -1 check here. Other parts look good to me now. Thanks, Jacek
Some PE executables (like mingw's gdb port) just do something like: - WaitForSingleObject(GetStdHandle(STD_INPUT_HANDLE), INFINITE) and hang for ever (the read operations are done *after* the wait operation succeeds) (of course, the real wait operation is more complex, but the problematic part boils down to that) This hangs for ever because conhost's main input thread hasn't been started yet. This patch sends once a peek IOCTL before waiting on console input objects, hence ensuring conhost's main input stream has been started. This lets i686 and x64_86 mingw's gdb port to accept user's input. V3 => V4 (as per Jacek's comments) - support correctly servers not connected to a TTY V2 => V3 (including Jacek's comments) - space style conventions - move once_input flag setting into CONDRV_SETUP_INPUT handling - applied same change to console objects' add_queue as to console_input objects (and let the later call into the former) - made console_server flags' storage a bit field A+ Signed-off-by: Eric Pouech <eric.pouech(a)gmail.com> --- server/console.c | 46 +++++++++++++++++++++++++++++++++------------- 1 file changed, 33 insertions(+), 13 deletions(-) diff --git a/server/console.c b/server/console.c index 977c428cf9f..136c14862e3 100644 --- a/server/console.c +++ b/server/console.c @@ -71,13 +71,14 @@ static struct object *console_lookup_name( struct object *obj, struct unicode_st unsigned int attr, struct object *root ); static struct object *console_open_file( struct object *obj, unsigned int access, unsigned int sharing, unsigned int options ); +static int console_add_queue( struct object *obj, struct wait_queue_entry *entry ); static const struct object_ops console_ops = { sizeof(struct console), /* size */ &file_type, /* type */ console_dump, /* dump */ - add_queue, /* add_queue */ + console_add_queue, /* add_queue */ remove_queue, /* remove_queue */ console_signaled, /* signaled */ no_satisfied, /* satisfied */ @@ -129,14 +130,15 @@ struct console_host_ioctl struct console_server { - struct object obj; /* object header */ - struct fd *fd; /* pseudo-fd for ioctls */ - struct console *console; /* attached console */ - struct list queue; /* ioctl queue */ - struct list read_queue; /* blocking read queue */ - int busy; /* flag if server processing an ioctl */ - int term_fd; /* UNIX terminal fd */ - struct termios termios; /* original termios */ + struct object obj; /* object header */ + struct fd *fd; /* pseudo-fd for ioctls */ + struct console *console; /* attached console */ + struct list queue; /* ioctl queue */ + struct list read_queue; /* blocking read queue */ + unsigned int busy : 1; /* flag if server processing an ioctl */ + unsigned int once_input : 1; /* flag if input thread has already been requested */ + int term_fd; /* UNIX terminal fd */ + struct termios termios; /* original termios */ }; static void console_server_dump( struct object *obj, int verbose ); @@ -462,6 +464,23 @@ static const struct fd_ops console_connection_fd_ops = 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 ); + +static int console_add_queue( struct object *obj, struct wait_queue_entry *entry ) +{ + struct console *console = (struct console*)obj; + assert( obj->ops == &console_ops ); + /* before waiting, ensure conhost's input thread has been started */ + if (console->server && !console->server->once_input) + { + console->server->once_input = 1; + if (console->server->term_fd == -1) + queue_host_ioctl( console->server, IOCTL_CONDRV_PEEK, 0, NULL, NULL ); + } + return add_queue( &console->obj, entry ); +} + static int console_signaled( struct object *obj, struct wait_queue_entry *entry ) { struct console *console = (struct console*)obj; @@ -889,9 +908,10 @@ static struct object *create_console_server( void ) struct console_server *server; if (!(server = alloc_object( &console_server_ops ))) return NULL; - server->console = NULL; - server->busy = 0; - server->term_fd = -1; + server->console = NULL; + server->busy = 0; + server->once_input = 0; + server->term_fd = -1; list_init( &server->queue ); list_init( &server->read_queue ); server->fd = alloc_pseudo_fd( &console_server_fd_ops, &server->obj, FILE_SYNCHRONOUS_IO_NONALERT ); @@ -1315,7 +1335,7 @@ static int console_input_add_queue( struct object *obj, struct wait_queue_entry set_error( STATUS_ACCESS_DENIED ); return 0; } - return add_queue( ¤t->process->console->obj, entry ); + return console_add_queue( ¤t->process->console->obj, entry ); } static struct fd *console_input_get_fd( struct object *obj )
participants (2)
-
Eric Pouech -
Jacek Caban