[PATCH 0/1] MR4776: winedbg: Wait for gdb to terminate before exiting (proxy mode).
This mainly allows Wine to reset the tty settings upon termination and not let gdb do it (cf bug report). Change: user is now required to explicitely terminate gdb ('quit' command) upon debuggee termination. Wine-Bugs: https://bugs.winehq.org/show_bug.cgi?id=56032 Signed-off-by: Eric Pouech <epouech(a)codeweavers.com> -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4776
From: Eric Pouech <epouech(a)codeweavers.com> This mainly allows Wine to reset the tty settings upon termination and not let gdb do it (cf bug report). Change: user is now required to explicitely terminate gdb ('quit' command) upon debuggee termination. Wine-Bugs: https://bugs.winehq.org/show_bug.cgi?id=56032 Signed-off-by: Eric Pouech <epouech(a)codeweavers.com> --- programs/winedbg/gdbproxy.c | 29 ++++++++++++++++------------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/programs/winedbg/gdbproxy.c b/programs/winedbg/gdbproxy.c index 238eecb1538..2b750193a44 100644 --- a/programs/winedbg/gdbproxy.c +++ b/programs/winedbg/gdbproxy.c @@ -64,6 +64,7 @@ struct reply_buffer struct gdb_context { /* gdb information */ + HANDLE gdb_ctrl_thread; SOCKET sock; /* incoming buffer */ char* in_buf; @@ -2391,10 +2392,15 @@ static int fetch_data(struct gdb_context* gdbctx) return gdbctx->in_len - in_len; } +static DWORD WINAPI gdb_ctrl_thread(void *pmt) +{ + return __wine_unix_spawnvp( (char **)pmt, TRUE ); +} + #define FLAG_NO_START 1 #define FLAG_WITH_XTERM 2 -static BOOL gdb_exec(unsigned port, unsigned flags) +static HANDLE gdb_exec(unsigned port, unsigned flags) { WCHAR tmp[MAX_PATH], buf[MAX_PATH]; const char *argv[6]; @@ -2405,7 +2411,7 @@ static BOOL gdb_exec(unsigned port, unsigned flags) if (!(gdb_path = getenv("WINE_GDB"))) gdb_path = "gdb"; GetTempPathW( MAX_PATH, buf ); GetTempFileNameW( buf, L"gdb", 0, tmp ); - if ((f = _wfopen( tmp, L"w+" )) == NULL) return FALSE; + if ((f = _wfopen( tmp, L"w+" )) == NULL) return NULL; unix_tmp = wine_get_unix_file_name( tmp ); fprintf(f, "target remote localhost:%d\n", ntohs(port)); fprintf(f, "set prompt Wine-gdb>\\ \n"); @@ -2428,12 +2434,7 @@ static BOOL gdb_exec(unsigned port, unsigned flags) argv[3] = "-x"; argv[4] = unix_tmp; argv[5] = NULL; - if (flags & FLAG_WITH_XTERM) - __wine_unix_spawnvp( (char **)argv, FALSE ); - else - __wine_unix_spawnvp( (char **)argv + 2, FALSE ); - HeapFree( GetProcessHeap(), 0, unix_tmp ); - return TRUE; + return CreateThread( NULL, 0, gdb_ctrl_thread, (flags & FLAG_WITH_XTERM) ? argv : argv + 2, 0, NULL ); } static BOOL gdb_startup(struct gdb_context* gdbctx, unsigned flags, unsigned port) @@ -2473,7 +2474,7 @@ static BOOL gdb_startup(struct gdb_context* gdbctx, unsigned flags, unsigned por if (flags & FLAG_NO_START) fprintf(stderr, "target remote localhost:%d\n", ntohs(s_addrs.sin_port)); else - gdb_exec(s_addrs.sin_port, flags); + gdbctx->gdb_ctrl_thread = gdb_exec(s_addrs.sin_port, flags); /* step 4: wait for gdb to connect actually */ FD_ZERO( &read_fds ); @@ -2505,6 +2506,7 @@ static BOOL gdb_init_context(struct gdb_context* gdbctx, unsigned flags, unsigne { int i; + gdbctx->gdb_ctrl_thread = NULL; gdbctx->sock = INVALID_SOCKET; gdbctx->in_buf = NULL; gdbctx->in_buf_alloc = 0; @@ -2569,12 +2571,13 @@ static int gdb_remote(unsigned flags, unsigned port) } if (FD_ISSET( gdbctx.sock, &read_fds )) { - if (fetch_data(&gdbctx) > 0) - { - if (extract_packets(&gdbctx)) break; - } + if (fetch_data(&gdbctx) == 0) /* connection closed */ + break; + extract_packets(&gdbctx); } } + /* connection closed, wait for gdb to terminate */ + WaitForSingleObject(gdbctx.gdb_ctrl_thread, INFINITE); return 0; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4776
Rémi Bernon (@rbernon) commented about programs/winedbg/gdbproxy.c:
argv[3] = "-x"; argv[4] = unix_tmp; argv[5] = NULL; - if (flags & FLAG_WITH_XTERM) - __wine_unix_spawnvp( (char **)argv, FALSE ); - else - __wine_unix_spawnvp( (char **)argv + 2, FALSE ); - HeapFree( GetProcessHeap(), 0, unix_tmp ); - return TRUE; + return CreateThread( NULL, 0, gdb_ctrl_thread, (flags & FLAG_WITH_XTERM) ? argv : argv + 2, 0, NULL );
This passes the `argv` variable from the stack to the other thread. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4776#note_56618
Rémi Bernon (@rbernon) commented about programs/winedbg/gdbproxy.c:
} if (FD_ISSET( gdbctx.sock, &read_fds )) { - if (fetch_data(&gdbctx) > 0) - { - if (extract_packets(&gdbctx)) break; - } + if (fetch_data(&gdbctx) == 0) /* connection closed */ + break; + extract_packets(&gdbctx);
The `extract_packets` return value is now ignored, it should either be made void, or used to break the loop too (I think the latter is better). -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4776#note_56619
On Wed Jan 3 17:13:08 2024 +0000, Rémi Bernon wrote:
The `extract_packets` return value is now ignored, it should either be made void, or used to break the loop too (I think the latter is better). Actually, I think this chunk might not even be needed? Shouldn't `select` fail already (or later) if connection has been closed?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/4776#note_56620
On Wed Jan 3 17:14:37 2024 +0000, Rémi Bernon wrote:
Actually, I think this chunk might not even be needed? Shouldn't `select` fail already (or later) if connection has been closed? actually the select no longer detects the lost connexion. IMO, it's a left over from the unix socket/select =\> WS2 socket evolution. didn't want to go all the way, but the err_fds should be dropped as well.
and the point is to no longer terminate the gdb stub on debuggee exit (if one day we implement restart or run commands we'll need that), but wait for gdb termination instead. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4776#note_56622
participants (3)
-
Eric Pouech -
eric pouech (＠epo)
-
Rémi Bernon