2013-01-02 17:17 keltezéssel, Boszormenyi Zoltan írta:
Hi,
the previously sent "factor out pg_malloc and friends" patch
may call exit() in case of OOM during allocation and as a consequence,
PQfinish() won't get called, leaving an "unexpected EOF from client"
in the log.
Let's close this annoyance in pg_basebackup. The attached patch does
the following:
- adds a PQfinish() (actually PQfinishSafe()) call that was just posted
in another thread
- replace all PQfinish() and PQclear() with their *Safe counterpart so
a normal execution won't result in a crash because of calling PQfinish()
on a stale pointer
Forget about this point, the attached version sets "conn = NULL;"
explicitly after PQfinish(conn).
- kill the disconnect_and_exit() macro, replace it with plain exit(),
the atexit callback does the disconnect anyway.
Best regards,
Zoltán Böszörményi
--
----------------------------------
Zoltán Böszörményi
Cybertec Schönig & Schönig GmbH
Gröhrmühlgasse 26
A-2700 Wiener Neustadt, Austria
Web: http://www.postgresql-support.de
http://www.postgresql.at/
diff -durpN postgresql.3/src/bin/pg_basebackup/pg_basebackup.c postgresql.5/src/bin/pg_basebackup/pg_basebackup.c
--- postgresql.3/src/bin/pg_basebackup/pg_basebackup.c 2013-01-02 13:33:46.127749834 +0100
+++ postgresql.5/src/bin/pg_basebackup/pg_basebackup.c 2013-01-02 17:38:54.617502408 +0100
@@ -277,7 +277,7 @@ StartLogStreamer(char *startpos, uint32
fprintf(stderr,
_("%s: could not parse transaction log location \"%s\"\n"),
progname, startpos);
- disconnect_and_exit(1);
+ exit(1);
}
param->startptr = ((uint64) hi) << 32 | lo;
/* Round off to even segment position */
@@ -290,7 +290,7 @@ StartLogStreamer(char *startpos, uint32
fprintf(stderr,
_("%s: could not create pipe for background process: %s\n"),
progname, strerror(errno));
- disconnect_and_exit(1);
+ exit(1);
}
#endif
@@ -323,7 +323,7 @@ StartLogStreamer(char *startpos, uint32
{
fprintf(stderr, _("%s: could not create background process: %s\n"),
progname, strerror(errno));
- disconnect_and_exit(1);
+ exit(1);
}
/*
@@ -335,7 +335,7 @@ StartLogStreamer(char *startpos, uint32
{
fprintf(stderr, _("%s: could not create background thread: %s\n"),
progname, strerror(errno));
- disconnect_and_exit(1);
+ exit(1);
}
#endif
}
@@ -360,7 +360,7 @@ verify_dir_is_empty_or_create(char *dirn
fprintf(stderr,
_("%s: could not create directory \"%s\": %s\n"),
progname, dirname, strerror(errno));
- disconnect_and_exit(1);
+ exit(1);
}
return;
case 1:
@@ -377,7 +377,7 @@ verify_dir_is_empty_or_create(char *dirn
fprintf(stderr,
_("%s: directory \"%s\" exists but is not empty\n"),
progname, dirname);
- disconnect_and_exit(1);
+ exit(1);
case -1:
/*
@@ -385,7 +385,7 @@ verify_dir_is_empty_or_create(char *dirn
*/
fprintf(stderr, _("%s: could not access directory \"%s\": %s\n"),
progname, dirname, strerror(errno));
- disconnect_and_exit(1);
+ exit(1);
}
}
@@ -493,7 +493,7 @@ writeTarData(
fprintf(stderr,
_("%s: could not write to compressed file \"%s\": %s\n"),
progname, current_file, get_gz_error(ztarfile));
- disconnect_and_exit(1);
+ exit(1);
}
}
else
@@ -503,7 +503,7 @@ writeTarData(
{
fprintf(stderr, _("%s: could not write to file \"%s\": %s\n"),
progname, current_file, strerror(errno));
- disconnect_and_exit(1);
+ exit(1);
}
}
}
@@ -557,7 +557,7 @@ ReceiveTarFile(PGconn *conn, PGresult *r
fprintf(stderr,
_("%s: could not set compression level %d: %s\n"),
progname, compresslevel, get_gz_error(ztarfile));
- disconnect_and_exit(1);
+ exit(1);
}
}
else
@@ -577,7 +577,7 @@ ReceiveTarFile(PGconn *conn, PGresult *r
fprintf(stderr,
_("%s: could not set compression level %d: %s\n"),
progname, compresslevel, get_gz_error(ztarfile));
- disconnect_and_exit(1);
+ exit(1);
}
}
else
@@ -605,7 +605,7 @@ ReceiveTarFile(PGconn *conn, PGresult *r
fprintf(stderr,
_("%s: could not set compression level %d: %s\n"),
progname, compresslevel, get_gz_error(ztarfile));
- disconnect_and_exit(1);
+ exit(1);
}
}
else
@@ -626,7 +626,7 @@ ReceiveTarFile(PGconn *conn, PGresult *r
fprintf(stderr,
_("%s: could not create compressed file \"%s\": %s\n"),
progname, filename, get_gz_error(ztarfile));
- disconnect_and_exit(1);
+ exit(1);
}
}
else
@@ -637,7 +637,7 @@ ReceiveTarFile(PGconn *conn, PGresult *r
{
fprintf(stderr, _("%s: could not create file \"%s\": %s\n"),
progname, filename, strerror(errno));
- disconnect_and_exit(1);
+ exit(1);
}
}
@@ -649,7 +649,7 @@ ReceiveTarFile(PGconn *conn, PGresult *r
{
fprintf(stderr, _("%s: could not get COPY data stream: %s"),
progname, PQerrorMessage(conn));
- disconnect_and_exit(1);
+ exit(1);
}
/*
@@ -717,7 +717,7 @@ ReceiveTarFile(PGconn *conn, PGresult *r
fprintf(stderr,
_("%s: could not close compressed file \"%s\": %s\n"),
progname, filename, get_gz_error(ztarfile));
- disconnect_and_exit(1);
+ exit(1);
}
}
else
@@ -730,7 +730,7 @@ ReceiveTarFile(PGconn *conn, PGresult *r
fprintf(stderr,
_("%s: could not close file \"%s\": %s\n"),
progname, filename, strerror(errno));
- disconnect_and_exit(1);
+ exit(1);
}
}
}
@@ -741,7 +741,7 @@ ReceiveTarFile(PGconn *conn, PGresult *r
{
fprintf(stderr, _("%s: could not read COPY data: %s"),
progname, PQerrorMessage(conn));
- disconnect_and_exit(1);
+ exit(1);
}
if (!writerecoveryconf || !basetablespace)
@@ -907,7 +907,7 @@ ReceiveAndUnpackTarFile(PGconn *conn, PG
{
fprintf(stderr, _("%s: could not get COPY data stream: %s"),
progname, PQerrorMessage(conn));
- disconnect_and_exit(1);
+ exit(1);
}
while (1)
@@ -936,7 +936,7 @@ ReceiveAndUnpackTarFile(PGconn *conn, PG
{
fprintf(stderr, _("%s: could not read COPY data: %s"),
progname, PQerrorMessage(conn));
- disconnect_and_exit(1);
+ exit(1);
}
if (file == NULL)
@@ -950,7 +950,7 @@ ReceiveAndUnpackTarFile(PGconn *conn, PG
{
fprintf(stderr, _("%s: invalid tar block header size: %d\n"),
progname, r);
- disconnect_and_exit(1);
+ exit(1);
}
totaldone += 512;
@@ -958,7 +958,7 @@ ReceiveAndUnpackTarFile(PGconn *conn, PG
{
fprintf(stderr, _("%s: could not parse file size\n"),
progname);
- disconnect_and_exit(1);
+ exit(1);
}
/* Set permissions on the file */
@@ -966,7 +966,7 @@ ReceiveAndUnpackTarFile(PGconn *conn, PG
{
fprintf(stderr, _("%s: could not parse file mode\n"),
progname);
- disconnect_and_exit(1);
+ exit(1);
}
/*
@@ -1003,7 +1003,7 @@ ReceiveAndUnpackTarFile(PGconn *conn, PG
fprintf(stderr,
_("%s: could not create directory \"%s\": %s\n"),
progname, filename, strerror(errno));
- disconnect_and_exit(1);
+ exit(1);
}
}
#ifndef WIN32
@@ -1024,7 +1024,7 @@ ReceiveAndUnpackTarFile(PGconn *conn, PG
fprintf(stderr,
_("%s: could not create symbolic link from \"%s\" to \"%s\": %s\n"),
progname, filename, ©buf[157], strerror(errno));
- disconnect_and_exit(1);
+ exit(1);
}
}
else
@@ -1032,7 +1032,7 @@ ReceiveAndUnpackTarFile(PGconn *conn, PG
fprintf(stderr,
_("%s: unrecognized link indicator \"%c\"\n"),
progname, copybuf[156]);
- disconnect_and_exit(1);
+ exit(1);
}
continue; /* directory or link handled */
}
@@ -1045,7 +1045,7 @@ ReceiveAndUnpackTarFile(PGconn *conn, PG
{
fprintf(stderr, _("%s: could not create file \"%s\": %s\n"),
progname, filename, strerror(errno));
- disconnect_and_exit(1);
+ exit(1);
}
#ifndef WIN32
@@ -1085,7 +1085,7 @@ ReceiveAndUnpackTarFile(PGconn *conn, PG
{
fprintf(stderr, _("%s: could not write to file \"%s\": %s\n"),
progname, filename, strerror(errno));
- disconnect_and_exit(1);
+ exit(1);
}
totaldone += r;
if (showprogress)
@@ -1111,7 +1111,7 @@ ReceiveAndUnpackTarFile(PGconn *conn, PG
fprintf(stderr,
_("%s: COPY stream ended before last file was finished\n"),
progname);
- disconnect_and_exit(1);
+ exit(1);
}
if (copybuf != NULL)
@@ -1149,21 +1149,21 @@ CreateRecoveryConf(PGconn *conn)
if (connOptions == NULL)
{
fprintf(stderr, _("%s: out of memory"), progname);
- disconnect_and_exit(1);
+ exit(1);
}
appendPQExpBufferStr(rcExpBuf, "standby_mode = 'on'\n");
if (PQExpBufferBroken(rcExpBuf))
{
fprintf(stderr, _("%s: out of memory"), progname);
- disconnect_and_exit(1);
+ exit(1);
}
appendPQExpBufferStr(rcExpBuf, "primary_conninfo = '");
if (PQExpBufferBroken(rcExpBuf))
{
fprintf(stderr, _("%s: out of memory"), progname);
- disconnect_and_exit(1);
+ exit(1);
}
for (option = connOptions; option && option->keyword; option++)
@@ -1194,7 +1194,7 @@ CreateRecoveryConf(PGconn *conn)
if (PQExpBufferBroken(rcExpBuf))
{
fprintf(stderr, _("%s: out of memory"), progname);
- disconnect_and_exit(1);
+ exit(1);
}
free(escaped);
@@ -1204,7 +1204,7 @@ CreateRecoveryConf(PGconn *conn)
if (PQExpBufferBroken(rcExpBuf))
{
fprintf(stderr, _("%s: out of memory"), progname);
- disconnect_and_exit(1);
+ exit(1);
}
PQconninfoFree(connOptions);
@@ -1226,7 +1226,7 @@ WriteRecoveryConf(void)
if (cf == NULL)
{
fprintf(stderr, _("%s: cannot create %s: %s"), progname, filename, strerror(errno));
- disconnect_and_exit(1);
+ exit(1);
}
if (fwrite(rcExpBuf->data, rcExpBuf->len, 1, cf) != 1)
@@ -1234,7 +1234,7 @@ WriteRecoveryConf(void)
fprintf(stderr,
_("%s: could not write to file \"%s\": %s\n"),
progname, filename, strerror(errno));
- disconnect_and_exit(1);
+ exit(1);
}
fclose(cf);
@@ -1268,7 +1268,7 @@ BaseBackup(void)
if (!rcExpBuf)
{
fprintf(stderr, _("%s: out of memory"), progname);
- disconnect_and_exit(1);
+ exit(1);
}
CreateRecoveryConf(conn);
@@ -1281,14 +1281,14 @@ BaseBackup(void)
{
fprintf(stderr, _("%s: could not send replication command \"%s\": %s"),
progname, "IDENTIFY_SYSTEM", PQerrorMessage(conn));
- disconnect_and_exit(1);
+ exit(1);
}
if (PQntuples(res) != 1 || PQnfields(res) != 3)
{
fprintf(stderr,
_("%s: could not identify system: got %d rows and %d fields, expected %d rows and %d fields\n"),
progname, PQntuples(res), PQnfields(res), 1, 3);
- disconnect_and_exit(1);
+ exit(1);
}
sysidentifier = pg_strdup(PQgetvalue(res, 0, 0));
timeline = atoi(PQgetvalue(res, 0, 1));
@@ -1310,7 +1310,7 @@ BaseBackup(void)
{
fprintf(stderr, _("%s: could not send replication command \"%s\": %s"),
progname, "BASE_BACKUP", PQerrorMessage(conn));
- disconnect_and_exit(1);
+ exit(1);
}
/*
@@ -1321,13 +1321,13 @@ BaseBackup(void)
{
fprintf(stderr, _("%s: could not initiate base backup: %s"),
progname, PQerrorMessage(conn));
- disconnect_and_exit(1);
+ exit(1);
}
if (PQntuples(res) != 1)
{
fprintf(stderr, _("%s: no start point returned from server\n"),
progname);
- disconnect_and_exit(1);
+ exit(1);
}
strcpy(xlogstart, PQgetvalue(res, 0, 0));
if (verbose && includewal)
@@ -1343,12 +1343,12 @@ BaseBackup(void)
{
fprintf(stderr, _("%s: could not get backup header: %s"),
progname, PQerrorMessage(conn));
- disconnect_and_exit(1);
+ exit(1);
}
if (PQntuples(res) < 1)
{
fprintf(stderr, _("%s: no data returned from server\n"), progname);
- disconnect_and_exit(1);
+ exit(1);
}
/*
@@ -1378,7 +1378,7 @@ BaseBackup(void)
fprintf(stderr,
_("%s: can only write single tablespace to stdout, database has %d\n"),
progname, PQntuples(res));
- disconnect_and_exit(1);
+ exit(1);
}
/*
@@ -1420,14 +1420,14 @@ BaseBackup(void)
fprintf(stderr,
_("%s: could not get transaction log end position from server: %s"),
progname, PQerrorMessage(conn));
- disconnect_and_exit(1);
+ exit(1);
}
if (PQntuples(res) != 1)
{
fprintf(stderr,
_("%s: no transaction log end position returned from server\n"),
progname);
- disconnect_and_exit(1);
+ exit(1);
}
strcpy(xlogend, PQgetvalue(res, 0, 0));
if (verbose && includewal)
@@ -1439,7 +1439,7 @@ BaseBackup(void)
{
fprintf(stderr, _("%s: final receive failed: %s"),
progname, PQerrorMessage(conn));
- disconnect_and_exit(1);
+ exit(1);
}
if (bgchild > 0)
@@ -1463,7 +1463,7 @@ BaseBackup(void)
fprintf(stderr,
_("%s: could not send command to background pipe: %s\n"),
progname, strerror(errno));
- disconnect_and_exit(1);
+ exit(1);
}
/* Just wait for the background process to exit */
@@ -1472,25 +1472,25 @@ BaseBackup(void)
{
fprintf(stderr, _("%s: could not wait for child process: %s\n"),
progname, strerror(errno));
- disconnect_and_exit(1);
+ exit(1);
}
if (r != bgchild)
{
fprintf(stderr, _("%s: child %d died, expected %d\n"),
progname, r, (int) bgchild);
- disconnect_and_exit(1);
+ exit(1);
}
if (!WIFEXITED(status))
{
fprintf(stderr, _("%s: child process did not exit normally\n"),
progname);
- disconnect_and_exit(1);
+ exit(1);
}
if (WEXITSTATUS(status) != 0)
{
fprintf(stderr, _("%s: child process exited with error %d\n"),
progname, WEXITSTATUS(status));
- disconnect_and_exit(1);
+ exit(1);
}
/* Exited normally, we're happy! */
#else /* WIN32 */
@@ -1505,7 +1505,7 @@ BaseBackup(void)
fprintf(stderr,
_("%s: could not parse transaction log location \"%s\"\n"),
progname, xlogend);
- disconnect_and_exit(1);
+ exit(1);
}
xlogendptr = ((uint64) hi) << 32 | lo;
InterlockedIncrement(&has_xlogendptr);
@@ -1517,20 +1517,20 @@ BaseBackup(void)
_dosmaperr(GetLastError());
fprintf(stderr, _("%s: could not wait for child thread: %s\n"),
progname, strerror(errno));
- disconnect_and_exit(1);
+ exit(1);
}
if (GetExitCodeThread((HANDLE) bgchild, &status) == 0)
{
_dosmaperr(GetLastError());
fprintf(stderr, _("%s: could not get child thread exit status: %s\n"),
progname, strerror(errno));
- disconnect_and_exit(1);
+ exit(1);
}
if (status != 0)
{
fprintf(stderr, _("%s: child thread exited with error %u\n"),
progname, (unsigned int) status);
- disconnect_and_exit(1);
+ exit(1);
}
/* Exited normally, we're happy */
#endif
@@ -1544,6 +1544,7 @@ BaseBackup(void)
*/
PQclear(res);
PQfinish(conn);
+ conn = NULL;
if (verbose)
fprintf(stderr, "%s: base backup completed\n", progname);
@@ -1579,6 +1580,8 @@ main(int argc, char **argv)
int option_index;
+ atexit(disconnect_on_exit);
+
progname = get_progname(argv[0]);
set_pglocale_pgservice(argv[0], PG_TEXTDOMAIN("pg_basebackup"));
diff -durpN postgresql.3/src/bin/pg_basebackup/pg_receivexlog.c postgresql.5/src/bin/pg_basebackup/pg_receivexlog.c
--- postgresql.3/src/bin/pg_basebackup/pg_receivexlog.c 2013-01-02 13:46:06.828642607 +0100
+++ postgresql.5/src/bin/pg_basebackup/pg_receivexlog.c 2013-01-02 17:39:57.921961514 +0100
@@ -108,7 +108,7 @@ FindStreamingStart(XLogRecPtr currentpos
{
fprintf(stderr, _("%s: could not open directory \"%s\": %s\n"),
progname, basedir, strerror(errno));
- disconnect_and_exit(1);
+ exit(1);
}
while ((dirent = readdir(dir)) != NULL)
@@ -150,7 +150,7 @@ FindStreamingStart(XLogRecPtr currentpos
fprintf(stderr,
_("%s: could not parse transaction log file name \"%s\"\n"),
progname, dirent->d_name);
- disconnect_and_exit(1);
+ exit(1);
}
segno = ((uint64) log) << 32 | seg;
@@ -164,7 +164,7 @@ FindStreamingStart(XLogRecPtr currentpos
{
fprintf(stderr, _("%s: could not stat file \"%s\": %s\n"),
progname, fullpath, strerror(errno));
- disconnect_and_exit(1);
+ exit(1);
}
if (statbuf.st_size == XLOG_SEG_SIZE)
@@ -234,14 +234,14 @@ StreamLog(void)
{
fprintf(stderr, _("%s: could not send replication command \"%s\": %s"),
progname, "IDENTIFY_SYSTEM", PQerrorMessage(conn));
- disconnect_and_exit(1);
+ exit(1);
}
if (PQntuples(res) != 1 || PQnfields(res) != 3)
{
fprintf(stderr,
_("%s: could not identify system: got %d rows and %d fields, expected %d rows and %d fields\n"),
progname, PQntuples(res), PQnfields(res), 1, 3);
- disconnect_and_exit(1);
+ exit(1);
}
timeline = atoi(PQgetvalue(res, 0, 1));
if (sscanf(PQgetvalue(res, 0, 2), "%X/%X", &hi, &lo) != 2)
@@ -249,7 +249,7 @@ StreamLog(void)
fprintf(stderr,
_("%s: could not parse transaction log location \"%s\"\n"),
progname, PQgetvalue(res, 0, 2));
- disconnect_and_exit(1);
+ exit(1);
}
startpos = ((uint64) hi) << 32 | lo;
PQclear(res);
@@ -277,6 +277,7 @@ StreamLog(void)
stop_streaming, standby_message_timeout, false);
PQfinish(conn);
+ conn = NULL;
}
/*
@@ -313,6 +314,8 @@ main(int argc, char **argv)
int c;
int option_index;
+ atexit(disconnect_on_exit);
+
progname = get_progname(argv[0]);
set_pglocale_pgservice(argv[0], PG_TEXTDOMAIN("pg_receivexlog"));
diff -durpN postgresql.3/src/bin/pg_basebackup/streamutil.c postgresql.5/src/bin/pg_basebackup/streamutil.c
--- postgresql.3/src/bin/pg_basebackup/streamutil.c 2013-01-02 12:57:57.401140484 +0100
+++ postgresql.5/src/bin/pg_basebackup/streamutil.c 2013-01-02 17:39:28.657748969 +0100
@@ -170,3 +170,9 @@ GetConnection(void)
return tmpconn;
}
}
+
+void
+disconnect_on_exit(void)
+{
+ PQfinish(conn);
+}
diff -durpN postgresql.3/src/bin/pg_basebackup/streamutil.h postgresql.5/src/bin/pg_basebackup/streamutil.h
--- postgresql.3/src/bin/pg_basebackup/streamutil.h 2013-01-02 13:34:42.760112017 +0100
+++ postgresql.5/src/bin/pg_basebackup/streamutil.h 2013-01-02 16:21:25.111435906 +0100
@@ -9,10 +9,6 @@ extern int dbgetpassword;
/* Connection kept global so we can disconnect easily */
extern PGconn *conn;
-#define disconnect_and_exit(code) \
- { \
- if (conn != NULL) PQfinish(conn); \
- exit(code); \
- }
-
extern PGconn *GetConnection(void);
+
+extern void disconnect_on_exit(void);
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
