Skip to content

Commit e069cfd

Browse files
authored
Also spawn valgrind for pg_autoctl subprocesses (#692)
This makes sure memory leaks in subprocesses are also detected correctly.
1 parent f36fb31 commit e069cfd

5 files changed

Lines changed: 25 additions & 10 deletions

File tree

Makefile

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,8 +60,8 @@ ifeq ($(VALGRIND),)
6060
BINPATH = ./src/bin/pg_autoctl/pg_autoctl
6161
PG_AUTOCTL = PG_AUTOCTL_DEBUG=1 ./src/bin/pg_autoctl/pg_autoctl
6262
else
63-
BINPATH = ./src/tools/pg_autoctl.valgrind
64-
PG_AUTOCTL = PG_AUTOCTL_DEBUG=1 ./src/tools/pg_autoctl.valgrind
63+
BINPATH = $(abspath $(PWD))/src/tools/pg_autoctl.valgrind
64+
PG_AUTOCTL = PG_AUTOCTL_DEBUG=1 PG_AUTOCTL_DEBUG_BIN_PATH="$(BINPATH)" ./src/tools/pg_autoctl.valgrind
6565
endif
6666

6767

src/bin/pg_autoctl/cli_do_tmux.c

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -855,7 +855,7 @@ prepare_tmux_script(TmuxOptions *options, PQExpBuffer script)
855855
* tmux_start_server starts a tmux session with the given script.
856856
*/
857857
bool
858-
tmux_start_server(const char *scriptName)
858+
tmux_start_server(const char *scriptName, const char *binpath)
859859
{
860860
char *args[8];
861861
int argsIndex = 0;
@@ -869,6 +869,12 @@ tmux_start_server(const char *scriptName)
869869
return false;
870870
}
871871

872+
if (binpath && setenv("PG_AUTOCTL_DEBUG_BIN_PATH", binpath, 1) != 0)
873+
{
874+
log_error("Failed to set environment PG_AUTOCTL_DEBUG_BIN_PATH: %m");
875+
return false;
876+
}
877+
872878
if (!search_path_first("tmux", tmux, LOG_ERROR))
873879
{
874880
log_fatal("Failed to find program tmux in PATH");
@@ -1358,7 +1364,7 @@ cli_do_tmux_session(int argc, char **argv)
13581364
/*
13591365
* Start a tmux session from the script.
13601366
*/
1361-
if (!tmux_start_server(scriptName))
1367+
if (!tmux_start_server(scriptName, options.binpath))
13621368
{
13631369
success = false;
13641370
log_fatal("Failed to start the tmux session, see above for details");

src/bin/pg_autoctl/cli_do_tmux.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ void tmux_pg_autoctl_create_postgres(PQExpBuffer script,
9292
int candidatePriority,
9393
bool skipHBA);
9494

95-
bool tmux_start_server(const char *scriptName);
95+
bool tmux_start_server(const char *scriptName, const char *binpath);
9696
bool pg_autoctl_getpid(const char *pgdata, pid_t *pid);
9797

9898
bool tmux_has_session(const char *tmux_path, const char *sessionName);

src/bin/pg_autoctl/cli_do_tmux_azure.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -253,7 +253,7 @@ tmux_azure_start_or_attach_session(AzureRegionResources *azRegion)
253253
return false;
254254
}
255255

256-
if (!tmux_start_server(scriptName))
256+
if (!tmux_start_server(scriptName, NULL))
257257
{
258258
log_fatal("Failed to start the tmux session, see above for details");
259259
destroyPQExpBuffer(script);

src/bin/pg_autoctl/main.c

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -121,13 +121,22 @@ main(int argc, char **argv)
121121
* hard-coded LOG_INFO as our log level. For now we won't see the log_debug
122122
* output, but as a developer you could always change the LOG_INFO to
123123
* LOG_DEBUG above and then see the message.
124+
*
125+
* When running pg_autoctl using valgrind we also want the subprocesses to
126+
* be run with valgrind. However, valgrind modifies the argv variables to
127+
* be the pg_autoctl binary, instead of the valgrind binary. So to make
128+
* sure subprocesses are spawned using valgrind, we allow overriding To
129+
* this program path detection using the PG_AUTOCTL_DEBUG_BIN_PATH
130+
* environment variable.
124131
*/
125132
strlcpy(pg_autoctl_argv0, argv[0], MAXPGPATH);
126-
127-
if (!set_program_absolute_path(pg_autoctl_program, MAXPGPATH))
133+
if (!get_env_copy("PG_AUTOCTL_DEBUG_BIN_PATH", pg_autoctl_program, MAXPGPATH))
128134
{
129-
/* errors have already been logged */
130-
exit(EXIT_CODE_INTERNAL_ERROR);
135+
if (!set_program_absolute_path(pg_autoctl_program, MAXPGPATH))
136+
{
137+
/* errors have already been logged */
138+
exit(EXIT_CODE_INTERNAL_ERROR);
139+
}
131140
}
132141

133142
if (!commandline_run(&command, argc, argv))

0 commit comments

Comments
 (0)