From 175147e0fca540ff6f5340ad989aea7115b12a39 Mon Sep 17 00:00:00 2001 From: Jelte Fennema Date: Thu, 29 Apr 2021 17:54:15 +0200 Subject: [PATCH] Also spawn valgrind for pg_autoctl subprocesses This makes sure memory leaks in subprocesses are also detected correctly. --- Makefile | 4 ++-- src/bin/pg_autoctl/cli_do_tmux.c | 10 ++++++++-- src/bin/pg_autoctl/cli_do_tmux.h | 2 +- src/bin/pg_autoctl/cli_do_tmux_azure.c | 2 +- src/bin/pg_autoctl/main.c | 17 +++++++++++++---- 5 files changed, 25 insertions(+), 10 deletions(-) diff --git a/Makefile b/Makefile index 12b0796bf..35332ca90 100644 --- a/Makefile +++ b/Makefile @@ -60,8 +60,8 @@ ifeq ($(VALGRIND),) BINPATH = ./src/bin/pg_autoctl/pg_autoctl PG_AUTOCTL = PG_AUTOCTL_DEBUG=1 ./src/bin/pg_autoctl/pg_autoctl else - BINPATH = ./src/tools/pg_autoctl.valgrind - PG_AUTOCTL = PG_AUTOCTL_DEBUG=1 ./src/tools/pg_autoctl.valgrind + BINPATH = $(abspath $(PWD))/src/tools/pg_autoctl.valgrind + PG_AUTOCTL = PG_AUTOCTL_DEBUG=1 PG_AUTOCTL_DEBUG_BIN_PATH="$(BINPATH)" ./src/tools/pg_autoctl.valgrind endif diff --git a/src/bin/pg_autoctl/cli_do_tmux.c b/src/bin/pg_autoctl/cli_do_tmux.c index 16d83ac09..85edef2b0 100644 --- a/src/bin/pg_autoctl/cli_do_tmux.c +++ b/src/bin/pg_autoctl/cli_do_tmux.c @@ -855,7 +855,7 @@ prepare_tmux_script(TmuxOptions *options, PQExpBuffer script) * tmux_start_server starts a tmux session with the given script. */ bool -tmux_start_server(const char *scriptName) +tmux_start_server(const char *scriptName, const char *binpath) { char *args[8]; int argsIndex = 0; @@ -869,6 +869,12 @@ tmux_start_server(const char *scriptName) return false; } + if (binpath && setenv("PG_AUTOCTL_DEBUG_BIN_PATH", binpath, 1) != 0) + { + log_error("Failed to set environment PG_AUTOCTL_DEBUG_BIN_PATH: %m"); + return false; + } + if (!search_path_first("tmux", tmux, LOG_ERROR)) { log_fatal("Failed to find program tmux in PATH"); @@ -1358,7 +1364,7 @@ cli_do_tmux_session(int argc, char **argv) /* * Start a tmux session from the script. */ - if (!tmux_start_server(scriptName)) + if (!tmux_start_server(scriptName, options.binpath)) { success = false; log_fatal("Failed to start the tmux session, see above for details"); diff --git a/src/bin/pg_autoctl/cli_do_tmux.h b/src/bin/pg_autoctl/cli_do_tmux.h index 9f87249d0..bf5f72dc3 100644 --- a/src/bin/pg_autoctl/cli_do_tmux.h +++ b/src/bin/pg_autoctl/cli_do_tmux.h @@ -92,7 +92,7 @@ void tmux_pg_autoctl_create_postgres(PQExpBuffer script, int candidatePriority, bool skipHBA); -bool tmux_start_server(const char *scriptName); +bool tmux_start_server(const char *scriptName, const char *binpath); bool pg_autoctl_getpid(const char *pgdata, pid_t *pid); bool tmux_has_session(const char *tmux_path, const char *sessionName); diff --git a/src/bin/pg_autoctl/cli_do_tmux_azure.c b/src/bin/pg_autoctl/cli_do_tmux_azure.c index a085dc101..f4a5373ed 100644 --- a/src/bin/pg_autoctl/cli_do_tmux_azure.c +++ b/src/bin/pg_autoctl/cli_do_tmux_azure.c @@ -253,7 +253,7 @@ tmux_azure_start_or_attach_session(AzureRegionResources *azRegion) return false; } - if (!tmux_start_server(scriptName)) + if (!tmux_start_server(scriptName, NULL)) { log_fatal("Failed to start the tmux session, see above for details"); destroyPQExpBuffer(script); diff --git a/src/bin/pg_autoctl/main.c b/src/bin/pg_autoctl/main.c index 2c000b488..08e179b2a 100644 --- a/src/bin/pg_autoctl/main.c +++ b/src/bin/pg_autoctl/main.c @@ -121,13 +121,22 @@ main(int argc, char **argv) * hard-coded LOG_INFO as our log level. For now we won't see the log_debug * output, but as a developer you could always change the LOG_INFO to * LOG_DEBUG above and then see the message. + * + * When running pg_autoctl using valgrind we also want the subprocesses to + * be run with valgrind. However, valgrind modifies the argv variables to + * be the pg_autoctl binary, instead of the valgrind binary. So to make + * sure subprocesses are spawned using valgrind, we allow overriding To + * this program path detection using the PG_AUTOCTL_DEBUG_BIN_PATH + * environment variable. */ strlcpy(pg_autoctl_argv0, argv[0], MAXPGPATH); - - if (!set_program_absolute_path(pg_autoctl_program, MAXPGPATH)) + if (!get_env_copy("PG_AUTOCTL_DEBUG_BIN_PATH", pg_autoctl_program, MAXPGPATH)) { - /* errors have already been logged */ - exit(EXIT_CODE_INTERNAL_ERROR); + if (!set_program_absolute_path(pg_autoctl_program, MAXPGPATH)) + { + /* errors have already been logged */ + exit(EXIT_CODE_INTERNAL_ERROR); + } } if (!commandline_run(&command, argc, argv))