Skip to content

Commit d4a258d

Browse files
committed
Make sure archive and restore comamnds do not leak files
As reported by Roman Leventov in percona/postgres#582 we had an issue where we easily could leak tmeporary files and directories when on failure, especially when the wrapped restore or archive command fails. The solution in this commit is to move the cleanup of temporary files and directories to and atexit() callback plus add a signal handler for SIGINT and SIGTERM. The signal handler makes sure to kill itself after running cleanup so the parent process gets the right error. We have a tiny race condition between creating the temporary directory and registering the signal handler but that does not seem worth fixing since it would just leak a directory, not the WAL file, the race conditions being tiny.
1 parent 2f0315d commit d4a258d

File tree

2 files changed

+54
-19
lines changed

2 files changed

+54
-19
lines changed

src/bin/pg_tde_archive_decrypt.c

Lines changed: 27 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@
88
#include "access/pg_tde_fe_init.h"
99
#include "access/pg_tde_xlog_smgr.h"
1010

11+
#include <signal.h>
12+
1113
#define TMPFS_DIRECTORY "/dev/shm"
1214

1315
static bool
@@ -134,6 +136,27 @@ usage(const char *progname)
134136
"\n"), progname, progname);
135137
}
136138

139+
static char tmpdir[MAXPGPATH] = TMPFS_DIRECTORY "/pg_tde_archiveXXXXXX";
140+
static char tmppath[MAXPGPATH];
141+
142+
static void
143+
cleanup_tmpdir(void)
144+
{
145+
if (unlink(tmppath) < 0 && errno != ENOENT)
146+
pg_log_warning("could not remove file \"%s\": %m", tmppath);
147+
if (rmdir(tmpdir) < 0 && errno != ENOENT)
148+
pg_log_warning("could not remove directory \"%s\": %m", tmpdir);
149+
}
150+
151+
/* Clean up temporary files and dirs on SIGINT/SIGTERM */
152+
static void
153+
trapsig(int signum)
154+
{
155+
cleanup_tmpdir();
156+
signal(signum, SIG_DFL);
157+
kill(getpid(), signum);
158+
}
159+
137160
int
138161
main(int argc, char *argv[])
139162
{
@@ -143,8 +166,6 @@ main(int argc, char *argv[])
143166
char *command;
144167
char *sep;
145168
char *sourcename;
146-
char tmpdir[MAXPGPATH] = TMPFS_DIRECTORY "/pg_tde_archiveXXXXXX";
147-
char tmppath[MAXPGPATH];
148169
bool issegment;
149170
int rc;
150171

@@ -199,6 +220,10 @@ main(int argc, char *argv[])
199220
s = stpcpy(s, "/");
200221
stpcpy(s, sourcename);
201222

223+
signal(SIGTERM, trapsig);
224+
signal(SIGINT, trapsig);
225+
atexit(cleanup_tmpdir);
226+
202227
command = replace_percent_placeholders(command,
203228
"ARCHIVE-COMMAND", "fp",
204229
targetname, tmppath);
@@ -228,13 +253,5 @@ main(int argc, char *argv[])
228253

229254
free(command);
230255

231-
if (issegment)
232-
{
233-
if (unlink(tmppath) < 0)
234-
pg_log_warning("could not remove file \"%s\": %m", tmppath);
235-
if (rmdir(tmpdir) < 0)
236-
pg_log_warning("could not remove directory \"%s\": %m", tmpdir);
237-
}
238-
239256
return 0;
240257
}

src/bin/pg_tde_restore_encrypt.c

Lines changed: 27 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@
88
#include "access/pg_tde_fe_init.h"
99
#include "access/pg_tde_xlog_smgr.h"
1010

11+
#include <signal.h>
12+
1113
#define TMPFS_DIRECTORY "/dev/shm"
1214

1315
/*
@@ -129,6 +131,27 @@ usage(const char *progname)
129131
"\n"), progname, progname);
130132
}
131133

134+
static char tmpdir[MAXPGPATH] = TMPFS_DIRECTORY "/pg_tde_restoreXXXXXX";
135+
static char tmppath[MAXPGPATH];
136+
137+
static void
138+
cleanup_tmpdir(void)
139+
{
140+
if (unlink(tmppath) < 0 && errno != ENOENT)
141+
pg_log_warning("could not remove file \"%s\": %m", tmppath);
142+
if (rmdir(tmpdir) < 0 && errno != ENOENT)
143+
pg_log_warning("could not remove directory \"%s\": %m", tmpdir);
144+
}
145+
146+
/* Clean up temporary files and dirs on SIGINT/SIGTERM */
147+
static void
148+
trapsig(int signum)
149+
{
150+
cleanup_tmpdir();
151+
signal(signum, SIG_DFL);
152+
kill(getpid(), signum);
153+
}
154+
132155
int
133156
main(int argc, char *argv[])
134157
{
@@ -138,8 +161,6 @@ main(int argc, char *argv[])
138161
char *command;
139162
char *sep;
140163
char *targetname;
141-
char tmpdir[MAXPGPATH] = TMPFS_DIRECTORY "/pg_tde_restoreXXXXXX";
142-
char tmppath[MAXPGPATH];
143164
bool issegment;
144165
int rc;
145166

@@ -194,6 +215,10 @@ main(int argc, char *argv[])
194215
s = stpcpy(s, "/");
195216
stpcpy(s, targetname);
196217

218+
signal(SIGTERM, trapsig);
219+
signal(SIGINT, trapsig);
220+
atexit(cleanup_tmpdir);
221+
197222
command = replace_percent_placeholders(command,
198223
"RESTORE-COMMAND", "fp",
199224
sourcename, tmppath);
@@ -222,14 +247,7 @@ main(int argc, char *argv[])
222247
free(command);
223248

224249
if (issegment)
225-
{
226250
write_encrypted_segment(targetpath, sourcename, tmppath);
227251

228-
if (unlink(tmppath) < 0)
229-
pg_log_warning("could not remove file \"%s\": %m", tmppath);
230-
if (rmdir(tmpdir) < 0)
231-
pg_log_warning("could not remove directory \"%s\": %m", tmpdir);
232-
}
233-
234252
return 0;
235253
}

0 commit comments

Comments
 (0)