On 04/01/2019 20:35, Alvaro Herrera wrote: > Seems you're registering the atexit cb twice here; you should only do so > in the first "!conn" block.
OK, fixed. >> @@ -3438,5 +3437,8 @@ main(int argc, char *argv[]) >> >> destroyPQExpBuffer(start_db_cmd); >> >> + /* prevent cleanup */ >> + made_new_pgdata = found_existing_pgdata = made_new_xlogdir = >> found_existing_xlogdir = false; >> + >> return 0; >> } > > This is a bit ugly, but meh. Yeah. Actually, we already have a solution of this in pg_basebackup, with a bool success variable. I rewrote it like that. At least it's better for uniformity. I also added an atexit() conversion in isolationtester. It's mostly the same thing. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
From 1cb0310a012e9b1c3324ca9df4bfd6ede51c5d96 Mon Sep 17 00:00:00 2001 From: Peter Eisentraut <pe...@eisentraut.org> Date: Sat, 29 Dec 2018 13:21:47 +0100 Subject: [PATCH v2 1/3] pg_basebackup: Use atexit() Instead of using our custom disconnect_and_exit(), just register the desired cleanup using atexit() and use the standard exit() to leave the program. --- src/bin/pg_basebackup/pg_basebackup.c | 142 +++++++++++++------------ src/bin/pg_basebackup/pg_receivewal.c | 45 ++++---- src/bin/pg_basebackup/pg_recvlogical.c | 18 ++-- 3 files changed, 104 insertions(+), 101 deletions(-) diff --git a/src/bin/pg_basebackup/pg_basebackup.c b/src/bin/pg_basebackup/pg_basebackup.c index 88421c7893..e51ef91d66 100644 --- a/src/bin/pg_basebackup/pg_basebackup.c +++ b/src/bin/pg_basebackup/pg_basebackup.c @@ -136,7 +136,6 @@ static PQExpBuffer recoveryconfcontents = NULL; /* Function headers */ static void usage(void); -static void disconnect_and_exit(int code) pg_attribute_noreturn(); static void verify_dir_is_empty_or_create(char *dirname, bool *created, bool *found); static void progress_report(int tablespacenum, const char *filename, bool force); @@ -217,25 +216,25 @@ cleanup_directories_atexit(void) } static void -disconnect_and_exit(int code) +disconnect_atexit(void) { if (conn != NULL) PQfinish(conn); +} #ifndef WIN32 - - /* - * On windows, our background thread dies along with the process. But on - * Unix, if we have started a subprocess, we want to kill it off so it - * doesn't remain running trying to stream data. - */ +/* + * On windows, our background thread dies along with the process. But on + * Unix, if we have started a subprocess, we want to kill it off so it + * doesn't remain running trying to stream data. + */ +static void +kill_bgchild_atexit(void) +{ if (bgchild > 0) kill(bgchild, SIGTERM); -#endif - - exit(code); } - +#endif /* * Split argument into old_dir and new_dir and append to tablespace mapping @@ -562,7 +561,7 @@ StartLogStreamer(char *startpos, uint32 timeline, char *sysidentifier) fprintf(stderr, _("%s: could not parse write-ahead log location \"%s\"\n"), progname, startpos); - disconnect_and_exit(1); + exit(1); } param->startptr = ((uint64) hi) << 32 | lo; /* Round off to even segment position */ @@ -575,7 +574,7 @@ StartLogStreamer(char *startpos, uint32 timeline, char *sysidentifier) fprintf(stderr, _("%s: could not create pipe for background process: %s\n"), progname, strerror(errno)); - disconnect_and_exit(1); + exit(1); } #endif @@ -604,7 +603,7 @@ StartLogStreamer(char *startpos, uint32 timeline, char *sysidentifier) { if (!CreateReplicationSlot(param->bgconn, replication_slot, NULL, temp_replication_slot, true, true, false)) - disconnect_and_exit(1); + exit(1); if (verbose) { @@ -635,7 +634,7 @@ StartLogStreamer(char *startpos, uint32 timeline, char *sysidentifier) fprintf(stderr, _("%s: could not create directory \"%s\": %s\n"), progname, statusdir, strerror(errno)); - disconnect_and_exit(1); + exit(1); } } @@ -654,19 +653,20 @@ StartLogStreamer(char *startpos, uint32 timeline, char *sysidentifier) { fprintf(stderr, _("%s: could not create background process: %s\n"), progname, strerror(errno)); - disconnect_and_exit(1); + exit(1); } /* * Else we are in the parent process and all is well. */ + atexit(kill_bgchild_atexit); #else /* WIN32 */ bgchild = _beginthreadex(NULL, 0, (void *) LogStreamerMain, param, 0, NULL); if (bgchild == 0) { fprintf(stderr, _("%s: could not create background thread: %s\n"), progname, strerror(errno)); - disconnect_and_exit(1); + exit(1); } #endif } @@ -691,7 +691,7 @@ verify_dir_is_empty_or_create(char *dirname, bool *created, bool *found) fprintf(stderr, _("%s: could not create directory \"%s\": %s\n"), progname, dirname, strerror(errno)); - disconnect_and_exit(1); + exit(1); } if (created) *created = true; @@ -714,7 +714,7 @@ verify_dir_is_empty_or_create(char *dirname, bool *created, bool *found) fprintf(stderr, _("%s: directory \"%s\" exists but is not empty\n"), progname, dirname); - disconnect_and_exit(1); + exit(1); case -1: /* @@ -722,7 +722,7 @@ verify_dir_is_empty_or_create(char *dirname, bool *created, bool *found) */ fprintf(stderr, _("%s: could not access directory \"%s\": %s\n"), progname, dirname, strerror(errno)); - disconnect_and_exit(1); + exit(1); } } @@ -933,7 +933,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 @@ -943,7 +943,7 @@ writeTarData( { fprintf(stderr, _("%s: could not write to file \"%s\": %s\n"), progname, current_file, strerror(errno)); - disconnect_and_exit(1); + exit(1); } } } @@ -1005,7 +1005,7 @@ ReceiveTarFile(PGconn *conn, PGresult *res, int rownum) fprintf(stderr, _("%s: could not set compression level %d: %s\n"), progname, compresslevel, get_gz_error(ztarfile)); - disconnect_and_exit(1); + exit(1); } } else @@ -1026,7 +1026,7 @@ ReceiveTarFile(PGconn *conn, PGresult *res, int rownum) fprintf(stderr, _("%s: could not set compression level %d: %s\n"), progname, compresslevel, get_gz_error(ztarfile)); - disconnect_and_exit(1); + exit(1); } } else @@ -1054,7 +1054,7 @@ ReceiveTarFile(PGconn *conn, PGresult *res, int rownum) fprintf(stderr, _("%s: could not set compression level %d: %s\n"), progname, compresslevel, get_gz_error(ztarfile)); - disconnect_and_exit(1); + exit(1); } } else @@ -1075,7 +1075,7 @@ ReceiveTarFile(PGconn *conn, PGresult *res, int rownum) fprintf(stderr, _("%s: could not create compressed file \"%s\": %s\n"), progname, filename, get_gz_error(ztarfile)); - disconnect_and_exit(1); + exit(1); } } else @@ -1086,7 +1086,7 @@ ReceiveTarFile(PGconn *conn, PGresult *res, int rownum) { fprintf(stderr, _("%s: could not create file \"%s\": %s\n"), progname, filename, strerror(errno)); - disconnect_and_exit(1); + exit(1); } } @@ -1098,7 +1098,7 @@ ReceiveTarFile(PGconn *conn, PGresult *res, int rownum) { fprintf(stderr, _("%s: could not get COPY data stream: %s"), progname, PQerrorMessage(conn)); - disconnect_and_exit(1); + exit(1); } while (1) @@ -1167,7 +1167,7 @@ ReceiveTarFile(PGconn *conn, PGresult *res, int rownum) fprintf(stderr, _("%s: could not close compressed file \"%s\": %s\n"), progname, filename, get_gz_error(ztarfile)); - disconnect_and_exit(1); + exit(1); } } else @@ -1180,7 +1180,7 @@ ReceiveTarFile(PGconn *conn, PGresult *res, int rownum) fprintf(stderr, _("%s: could not close file \"%s\": %s\n"), progname, filename, strerror(errno)); - disconnect_and_exit(1); + exit(1); } } } @@ -1191,7 +1191,7 @@ ReceiveTarFile(PGconn *conn, PGresult *res, int rownum) { fprintf(stderr, _("%s: could not read COPY data: %s"), progname, PQerrorMessage(conn)); - disconnect_and_exit(1); + exit(1); } if (!writerecoveryconf || !basetablespace) @@ -1427,7 +1427,7 @@ ReceiveAndUnpackTarFile(PGconn *conn, PGresult *res, int rownum) { fprintf(stderr, _("%s: could not get COPY data stream: %s"), progname, PQerrorMessage(conn)); - disconnect_and_exit(1); + exit(1); } while (1) @@ -1456,7 +1456,7 @@ ReceiveAndUnpackTarFile(PGconn *conn, PGresult *res, int rownum) { fprintf(stderr, _("%s: could not read COPY data: %s"), progname, PQerrorMessage(conn)); - disconnect_and_exit(1); + exit(1); } if (file == NULL) @@ -1470,7 +1470,7 @@ ReceiveAndUnpackTarFile(PGconn *conn, PGresult *res, int rownum) { fprintf(stderr, _("%s: invalid tar block header size: %d\n"), progname, r); - disconnect_and_exit(1); + exit(1); } totaldone += 512; @@ -1520,7 +1520,7 @@ ReceiveAndUnpackTarFile(PGconn *conn, PGresult *res, int rownum) fprintf(stderr, _("%s: could not create directory \"%s\": %s\n"), progname, filename, strerror(errno)); - disconnect_and_exit(1); + exit(1); } } #ifndef WIN32 @@ -1553,7 +1553,7 @@ ReceiveAndUnpackTarFile(PGconn *conn, PGresult *res, int rownum) _("%s: could not create symbolic link from \"%s\" to \"%s\": %s\n"), progname, filename, mapped_tblspc_path, strerror(errno)); - disconnect_and_exit(1); + exit(1); } } else @@ -1561,7 +1561,7 @@ ReceiveAndUnpackTarFile(PGconn *conn, PGresult *res, int rownum) fprintf(stderr, _("%s: unrecognized link indicator \"%c\"\n"), progname, copybuf[156]); - disconnect_and_exit(1); + exit(1); } continue; /* directory or link handled */ } @@ -1574,7 +1574,7 @@ ReceiveAndUnpackTarFile(PGconn *conn, PGresult *res, int rownum) { fprintf(stderr, _("%s: could not create file \"%s\": %s\n"), progname, filename, strerror(errno)); - disconnect_and_exit(1); + exit(1); } #ifndef WIN32 @@ -1614,7 +1614,7 @@ ReceiveAndUnpackTarFile(PGconn *conn, PGresult *res, int rownum) { fprintf(stderr, _("%s: could not write to file \"%s\": %s\n"), progname, filename, strerror(errno)); - disconnect_and_exit(1); + exit(1); } totaldone += r; progress_report(rownum, filename, false); @@ -1640,7 +1640,7 @@ ReceiveAndUnpackTarFile(PGconn *conn, PGresult *res, int rownum) fprintf(stderr, _("%s: COPY stream ended before last file was finished\n"), progname); - disconnect_and_exit(1); + exit(1); } if (copybuf != NULL) @@ -1687,14 +1687,14 @@ GenerateRecoveryConf(PGconn *conn) if (!recoveryconfcontents) { fprintf(stderr, _("%s: out of memory\n"), progname); - disconnect_and_exit(1); + exit(1); } connOptions = PQconninfo(conn); if (connOptions == NULL) { fprintf(stderr, _("%s: out of memory\n"), progname); - disconnect_and_exit(1); + exit(1); } initPQExpBuffer(&conninfo_buf); @@ -1745,7 +1745,7 @@ GenerateRecoveryConf(PGconn *conn) PQExpBufferDataBroken(conninfo_buf)) { fprintf(stderr, _("%s: out of memory\n"), progname); - disconnect_and_exit(1); + exit(1); } termPQExpBuffer(&conninfo_buf); @@ -1771,7 +1771,7 @@ WriteRecoveryConf(void) if (cf == NULL) { fprintf(stderr, _("%s: could not open file \"%s\": %s\n"), progname, filename, strerror(errno)); - disconnect_and_exit(1); + exit(1); } if (fwrite(recoveryconfcontents->data, recoveryconfcontents->len, 1, cf) != 1) @@ -1779,7 +1779,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); @@ -1789,7 +1789,7 @@ WriteRecoveryConf(void) if (cf == NULL) { fprintf(stderr, _("%s: could not create file \"%s\": %s\n"), progname, filename, strerror(errno)); - disconnect_and_exit(1); + exit(1); } fclose(cf); @@ -1830,7 +1830,7 @@ BaseBackup(void) fprintf(stderr, _("%s: incompatible server version %s\n"), progname, serverver ? serverver : "'unknown'"); - disconnect_and_exit(1); + exit(1); } /* @@ -1844,7 +1844,7 @@ BaseBackup(void) * but add a hint about using -X none. */ fprintf(stderr, _("HINT: use -X none or -X fetch to disable log streaming\n")); - disconnect_and_exit(1); + exit(1); } /* @@ -1857,7 +1857,7 @@ BaseBackup(void) * Run IDENTIFY_SYSTEM so we can get the timeline */ if (!RunIdentifySystem(conn, &sysidentifier, &latesttli, NULL, NULL)) - disconnect_and_exit(1); + exit(1); /* * Start the actual backup @@ -1896,7 +1896,7 @@ BaseBackup(void) { fprintf(stderr, _("%s: could not send replication command \"%s\": %s"), progname, "BASE_BACKUP", PQerrorMessage(conn)); - disconnect_and_exit(1); + exit(1); } /* @@ -1907,14 +1907,14 @@ 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: server returned unexpected response to BASE_BACKUP command; got %d rows and %d fields, expected %d rows and %d fields\n"), progname, PQntuples(res), PQnfields(res), 1, 2); - disconnect_and_exit(1); + exit(1); } strlcpy(xlogstart, PQgetvalue(res, 0, 0), sizeof(xlogstart)); @@ -1946,12 +1946,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); } /* @@ -1984,7 +1984,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); } /* @@ -2028,14 +2028,14 @@ BaseBackup(void) fprintf(stderr, _("%s: could not get write-ahead log end position from server: %s"), progname, PQerrorMessage(conn)); - disconnect_and_exit(1); + exit(1); } if (PQntuples(res) != 1) { fprintf(stderr, _("%s: no write-ahead log end position returned from server\n"), progname); - disconnect_and_exit(1); + exit(1); } strlcpy(xlogend, PQgetvalue(res, 0, 0), sizeof(xlogend)); if (verbose && includewal != NO_WAL) @@ -2059,7 +2059,7 @@ BaseBackup(void) fprintf(stderr, _("%s: final receive failed: %s"), progname, PQerrorMessage(conn)); } - disconnect_and_exit(1); + exit(1); } if (bgchild > 0) @@ -2089,7 +2089,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 */ @@ -2098,19 +2098,19 @@ 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, (int) r, (int) bgchild); - disconnect_and_exit(1); + exit(1); } if (status != 0) { fprintf(stderr, "%s: %s\n", progname, wait_result_to_str(status)); - disconnect_and_exit(1); + exit(1); } /* Exited normally, we're happy! */ #else /* WIN32 */ @@ -2125,7 +2125,7 @@ BaseBackup(void) fprintf(stderr, _("%s: could not parse write-ahead log location \"%s\"\n"), progname, xlogend); - disconnect_and_exit(1); + exit(1); } xlogendptr = ((uint64) hi) << 32 | lo; InterlockedIncrement(&has_xlogendptr); @@ -2137,20 +2137,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_handle, &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 @@ -2164,6 +2164,7 @@ BaseBackup(void) */ PQclear(res); PQfinish(conn); + conn = NULL; /* * Make data persistent on disk once backup is completed. For tar format @@ -2542,6 +2543,7 @@ main(int argc, char **argv) /* Error message already written in GetConnection() */ exit(1); } + atexit(disconnect_atexit); /* * Set umask so that directories/files are created with the same @@ -2563,7 +2565,7 @@ main(int argc, char **argv) /* determine remote server's xlog segment size */ if (!RetrieveWalSegSize(conn)) - disconnect_and_exit(1); + exit(1); /* Create pg_wal symlink, if required */ if (xlog_dir) @@ -2585,11 +2587,11 @@ main(int argc, char **argv) { fprintf(stderr, _("%s: could not create symbolic link \"%s\": %s\n"), progname, linkloc, strerror(errno)); - disconnect_and_exit(1); + exit(1); } #else fprintf(stderr, _("%s: symlinks are not supported on this platform\n"), progname); - disconnect_and_exit(1); + exit(1); #endif free(linkloc); } diff --git a/src/bin/pg_basebackup/pg_receivewal.c b/src/bin/pg_basebackup/pg_receivewal.c index 52b258b61f..dc6705c330 100644 --- a/src/bin/pg_basebackup/pg_receivewal.c +++ b/src/bin/pg_basebackup/pg_receivewal.c @@ -55,11 +55,12 @@ static void StreamLog(void); static bool stop_streaming(XLogRecPtr segendpos, uint32 timeline, bool segment_finished); -#define disconnect_and_exit(code) \ - { \ - if (conn != NULL) PQfinish(conn); \ - exit(code); \ - } +static void +disconnect_atexit(void) +{ + if (conn != NULL) + PQfinish(conn); +} /* Routines to evaluate segment file format */ #define IsCompressXLogFileName(fname) \ @@ -168,7 +169,7 @@ get_destination_dir(char *dest_folder) { fprintf(stderr, _("%s: could not open directory \"%s\": %s\n"), progname, basedir, strerror(errno)); - disconnect_and_exit(1); + exit(1); } return dir; @@ -186,7 +187,7 @@ close_destination_dir(DIR *dest_dir, char *dest_folder) { fprintf(stderr, _("%s: could not close directory \"%s\": %s\n"), progname, dest_folder, strerror(errno)); - disconnect_and_exit(1); + exit(1); } } @@ -267,7 +268,7 @@ FindStreamingStart(uint32 *tli) { fprintf(stderr, _("%s: could not stat file \"%s\": %s\n"), progname, fullpath, strerror(errno)); - disconnect_and_exit(1); + exit(1); } if (statbuf.st_size != WalSegSz) @@ -293,13 +294,13 @@ FindStreamingStart(uint32 *tli) { fprintf(stderr, _("%s: could not open compressed file \"%s\": %s\n"), progname, fullpath, strerror(errno)); - disconnect_and_exit(1); + exit(1); } if (lseek(fd, (off_t) (-4), SEEK_END) < 0) { fprintf(stderr, _("%s: could not seek in compressed file \"%s\": %s\n"), progname, fullpath, strerror(errno)); - disconnect_and_exit(1); + exit(1); } r = read(fd, (char *) buf, sizeof(buf)); if (r != sizeof(buf)) @@ -310,7 +311,7 @@ FindStreamingStart(uint32 *tli) else fprintf(stderr, _("%s: could not read compressed file \"%s\": read %d of %zu\n"), progname, fullpath, r, sizeof(buf)); - disconnect_and_exit(1); + exit(1); } close(fd); @@ -341,7 +342,7 @@ FindStreamingStart(uint32 *tli) { fprintf(stderr, _("%s: could not read directory \"%s\": %s\n"), progname, basedir, strerror(errno)); - disconnect_and_exit(1); + exit(1); } close_destination_dir(dir, basedir); @@ -395,7 +396,7 @@ StreamLog(void) * There's no hope of recovering from a version mismatch, so don't * retry. */ - disconnect_and_exit(1); + exit(1); } /* @@ -404,7 +405,7 @@ StreamLog(void) * existing output directory. */ if (!RunIdentifySystem(conn, NULL, &servertli, &serverpos, NULL)) - disconnect_and_exit(1); + exit(1); /* * Figure out where to start streaming. @@ -452,6 +453,7 @@ StreamLog(void) } PQfinish(conn); + conn = NULL; FreeWalDirectoryMethod(); pg_free(stream.walmethod); @@ -701,6 +703,7 @@ main(int argc, char **argv) if (!conn) /* error message already written in GetConnection() */ exit(1); + atexit(disconnect_atexit); /* * Run IDENTIFY_SYSTEM to make sure we've successfully have established a @@ -708,7 +711,7 @@ main(int argc, char **argv) * connection. */ if (!RunIdentifySystem(conn, NULL, NULL, NULL, &db_name)) - disconnect_and_exit(1); + exit(1); /* * Set umask so that directories/files are created with the same @@ -722,7 +725,7 @@ main(int argc, char **argv) /* determine remote server's xlog segment size */ if (!RetrieveWalSegSize(conn)) - disconnect_and_exit(1); + exit(1); /* * Check that there is a database associated with connection, none should @@ -733,7 +736,7 @@ main(int argc, char **argv) fprintf(stderr, _("%s: replication connection using slot \"%s\" is unexpectedly database specific\n"), progname, replication_slot); - disconnect_and_exit(1); + exit(1); } /* @@ -747,8 +750,8 @@ main(int argc, char **argv) progname, replication_slot); if (!DropReplicationSlot(conn, replication_slot)) - disconnect_and_exit(1); - disconnect_and_exit(0); + exit(1); + exit(0); } /* Create a replication slot */ @@ -761,8 +764,8 @@ main(int argc, char **argv) if (!CreateReplicationSlot(conn, replication_slot, NULL, false, true, false, slot_exists_ok)) - disconnect_and_exit(1); - disconnect_and_exit(0); + exit(1); + exit(0); } /* diff --git a/src/bin/pg_basebackup/pg_recvlogical.c b/src/bin/pg_basebackup/pg_recvlogical.c index 471b6b11be..d75b66f1e7 100644 --- a/src/bin/pg_basebackup/pg_recvlogical.c +++ b/src/bin/pg_basebackup/pg_recvlogical.c @@ -65,7 +65,6 @@ static XLogRecPtr output_fsync_lsn = InvalidXLogRecPtr; static void usage(void); static void StreamLogicalLog(void); -static void disconnect_and_exit(int code) pg_attribute_noreturn(); static bool flushAndSendFeedback(PGconn *conn, TimestampTz *now); static void prepareToTerminate(PGconn *conn, XLogRecPtr endpos, bool keepalive, XLogRecPtr lsn); @@ -167,12 +166,10 @@ sendFeedback(PGconn *conn, TimestampTz now, bool force, bool replyRequested) } static void -disconnect_and_exit(int code) +disconnect_atexit(void) { if (conn != NULL) PQfinish(conn); - - exit(code); } static bool @@ -944,20 +941,21 @@ main(int argc, char **argv) if (!conn) /* Error message already written in GetConnection() */ exit(1); + atexit(disconnect_atexit); /* * Run IDENTIFY_SYSTEM to make sure we connected using a database specific * replication connection. */ if (!RunIdentifySystem(conn, NULL, NULL, NULL, &db_name)) - disconnect_and_exit(1); + exit(1); if (db_name == NULL) { fprintf(stderr, _("%s: could not establish database-specific replication connection\n"), progname); - disconnect_and_exit(1); + exit(1); } /* @@ -979,7 +977,7 @@ main(int argc, char **argv) progname, replication_slot); if (!DropReplicationSlot(conn, replication_slot)) - disconnect_and_exit(1); + exit(1); } /* Create a replication slot. */ @@ -992,12 +990,12 @@ main(int argc, char **argv) if (!CreateReplicationSlot(conn, replication_slot, plugin, false, false, false, slot_exists_ok)) - disconnect_and_exit(1); + exit(1); startpos = InvalidXLogRecPtr; } if (!do_start_slot) - disconnect_and_exit(0); + exit(0); /* Stream loop */ while (true) @@ -1009,7 +1007,7 @@ main(int argc, char **argv) * We've been Ctrl-C'ed or reached an exit limit condition. That's * not an error, so exit without an errorcode. */ - disconnect_and_exit(0); + exit(0); } else if (noloop) { base-commit: c5c7fa261f57fadd93f166dc33ce2b1d188ad4e7 -- 2.20.1
From f402b0ddd978e25d4587e83f965473a828acdb9d Mon Sep 17 00:00:00 2001 From: Peter Eisentraut <pe...@eisentraut.org> Date: Sat, 29 Dec 2018 13:21:57 +0100 Subject: [PATCH v2 2/3] initdb: Use atexit() Replace exit_nicely() calls with standard exit() and register the cleanup actions using atexit(). --- src/bin/initdb/initdb.c | 82 +++++++++++++++++++++-------------------- 1 file changed, 43 insertions(+), 39 deletions(-) diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c index 3ebe05dbf0..e55ba668ce 100644 --- a/src/bin/initdb/initdb.c +++ b/src/bin/initdb/initdb.c @@ -159,6 +159,7 @@ static char *dictionary_file; static char *info_schema_file; static char *features_file; static char *system_views_file; +static bool success = false; static bool made_new_pgdata = false; static bool found_existing_pgdata = false; static bool made_new_xlogdir = false; @@ -237,7 +238,6 @@ static char **filter_lines_with_token(char **lines, const char *token); static char **readfile(const char *path); static void writefile(char *path, char **lines); static FILE *popen_check(const char *command, const char *mode); -static void exit_nicely(void) pg_attribute_noreturn(); static char *get_id(void); static int get_encoding_id(const char *encoding_name); static void set_input(char **dest, const char *filename); @@ -291,13 +291,13 @@ void initialize_data_directory(void); do { \ cmdfd = popen_check(cmd, "w"); \ if (cmdfd == NULL) \ - exit_nicely(); /* message already printed by popen_check */ \ + exit(1); /* message already printed by popen_check */ \ } while (0) #define PG_CMD_CLOSE \ do { \ if (pclose_check(cmdfd)) \ - exit_nicely(); /* message already printed by pclose_check */ \ + exit(1); /* message already printed by pclose_check */ \ } while (0) #define PG_CMD_PUTS(line) \ @@ -493,7 +493,7 @@ readfile(const char *path) { fprintf(stderr, _("%s: could not open file \"%s\" for reading: %s\n"), progname, path, strerror(errno)); - exit_nicely(); + exit(1); } /* pass over the file twice - the first time to size the result */ @@ -549,7 +549,7 @@ writefile(char *path, char **lines) { fprintf(stderr, _("%s: could not open file \"%s\" for writing: %s\n"), progname, path, strerror(errno)); - exit_nicely(); + exit(1); } for (line = lines; *line != NULL; line++) { @@ -557,7 +557,7 @@ writefile(char *path, char **lines) { fprintf(stderr, _("%s: could not write file \"%s\": %s\n"), progname, path, strerror(errno)); - exit_nicely(); + exit(1); } free(*line); } @@ -565,7 +565,7 @@ writefile(char *path, char **lines) { fprintf(stderr, _("%s: could not write file \"%s\": %s\n"), progname, path, strerror(errno)); - exit_nicely(); + exit(1); } } @@ -592,8 +592,11 @@ popen_check(const char *command, const char *mode) * if we created the data directory remove it too */ static void -exit_nicely(void) +cleanup_directories_atexit(void) { + if (success) + return; + if (!noclean) { if (made_new_pgdata) @@ -645,8 +648,6 @@ exit_nicely(void) _("%s: WAL directory \"%s\" not removed at user's request\n"), progname, xlog_dir); } - - exit(1); } /* @@ -877,14 +878,14 @@ write_version_file(const char *extrapath) { fprintf(stderr, _("%s: could not open file \"%s\" for writing: %s\n"), progname, path, strerror(errno)); - exit_nicely(); + exit(1); } if (fprintf(version_file, "%s\n", PG_MAJORVERSION) < 0 || fclose(version_file)) { fprintf(stderr, _("%s: could not write file \"%s\": %s\n"), progname, path, strerror(errno)); - exit_nicely(); + exit(1); } free(path); } @@ -905,13 +906,13 @@ set_null_conf(void) { fprintf(stderr, _("%s: could not open file \"%s\" for writing: %s\n"), progname, path, strerror(errno)); - exit_nicely(); + exit(1); } if (fclose(conf_file)) { fprintf(stderr, _("%s: could not write file \"%s\": %s\n"), progname, path, strerror(errno)); - exit_nicely(); + exit(1); } free(path); } @@ -1262,7 +1263,7 @@ setup_config(void) { fprintf(stderr, _("%s: could not change permissions of \"%s\": %s\n"), progname, path, strerror(errno)); - exit_nicely(); + exit(1); } /* @@ -1282,7 +1283,7 @@ setup_config(void) { fprintf(stderr, _("%s: could not change permissions of \"%s\": %s\n"), progname, path, strerror(errno)); - exit_nicely(); + exit(1); } free(conflines); @@ -1369,7 +1370,7 @@ setup_config(void) { fprintf(stderr, _("%s: could not change permissions of \"%s\": %s\n"), progname, path, strerror(errno)); - exit_nicely(); + exit(1); } free(conflines); @@ -1385,7 +1386,7 @@ setup_config(void) { fprintf(stderr, _("%s: could not change permissions of \"%s\": %s\n"), progname, path, strerror(errno)); - exit_nicely(); + exit(1); } free(conflines); @@ -1423,7 +1424,7 @@ bootstrap_template1(void) "Check your installation or specify the correct path " "using the option -L.\n"), progname, bki_file, PG_VERSION); - exit_nicely(); + exit(1); } /* Substitute for various symbols used in the BKI file */ @@ -1541,7 +1542,7 @@ get_su_pwd(void) if (strcmp(pwd1, pwd2) != 0) { fprintf(stderr, _("Passwords didn't match.\n")); - exit_nicely(); + exit(1); } } else @@ -1561,7 +1562,7 @@ get_su_pwd(void) { fprintf(stderr, _("%s: could not open file \"%s\" for reading: %s\n"), progname, pwfilename, strerror(errno)); - exit_nicely(); + exit(1); } if (!fgets(pwd1, sizeof(pwd1), pwf)) { @@ -1571,7 +1572,7 @@ get_su_pwd(void) else fprintf(stderr, _("%s: password file \"%s\" is empty\n"), progname, pwfilename); - exit_nicely(); + exit(1); } fclose(pwf); @@ -2104,7 +2105,7 @@ make_postgres(FILE *cmdfd) * if you are handling SIGFPE. * * I avoided doing the forbidden things by setting a flag instead of calling - * exit_nicely() directly. + * exit() directly. * * Also note the behaviour of Windows with SIGINT, which says this: * Note SIGINT is not supported for any Win32 application, including @@ -2125,7 +2126,7 @@ trapsig(int signum) } /* - * call exit_nicely() if we got a signal, or else output "ok". + * call exit() if we got a signal, or else output "ok". */ static void check_ok(void) @@ -2134,14 +2135,14 @@ check_ok(void) { printf(_("caught signal\n")); fflush(stdout); - exit_nicely(); + exit(1); } else if (output_failed) { printf(_("could not write to child process: %s\n"), strerror(output_errno)); fflush(stdout); - exit_nicely(); + exit(1); } else { @@ -2775,7 +2776,7 @@ create_data_directory(void) { fprintf(stderr, _("%s: could not create directory \"%s\": %s\n"), progname, pg_data, strerror(errno)); - exit_nicely(); + exit(1); } else check_ok(); @@ -2793,7 +2794,7 @@ create_data_directory(void) { fprintf(stderr, _("%s: could not change permissions of directory \"%s\": %s\n"), progname, pg_data, strerror(errno)); - exit_nicely(); + exit(1); } else check_ok(); @@ -2822,7 +2823,7 @@ create_data_directory(void) /* Trouble accessing directory */ fprintf(stderr, _("%s: could not access directory \"%s\": %s\n"), progname, pg_data, strerror(errno)); - exit_nicely(); + exit(1); } } @@ -2845,7 +2846,7 @@ create_xlog_or_symlink(void) if (!is_absolute_path(xlog_dir)) { fprintf(stderr, _("%s: WAL directory location must be an absolute path\n"), progname); - exit_nicely(); + exit(1); } /* check if the specified xlog directory exists/is empty */ @@ -2861,7 +2862,7 @@ create_xlog_or_symlink(void) { fprintf(stderr, _("%s: could not create directory \"%s\": %s\n"), progname, xlog_dir, strerror(errno)); - exit_nicely(); + exit(1); } else check_ok(); @@ -2879,7 +2880,7 @@ create_xlog_or_symlink(void) { fprintf(stderr, _("%s: could not change permissions of directory \"%s\": %s\n"), progname, xlog_dir, strerror(errno)); - exit_nicely(); + exit(1); } else check_ok(); @@ -2901,13 +2902,13 @@ create_xlog_or_symlink(void) _("If you want to store the WAL there, either remove or empty the directory\n" "\"%s\".\n"), xlog_dir); - exit_nicely(); + exit(1); default: /* Trouble accessing directory */ fprintf(stderr, _("%s: could not access directory \"%s\": %s\n"), progname, xlog_dir, strerror(errno)); - exit_nicely(); + exit(1); } #ifdef HAVE_SYMLINK @@ -2915,11 +2916,11 @@ create_xlog_or_symlink(void) { fprintf(stderr, _("%s: could not create symbolic link \"%s\": %s\n"), progname, subdirloc, strerror(errno)); - exit_nicely(); + exit(1); } #else fprintf(stderr, _("%s: symlinks are not supported on this platform\n"), progname); - exit_nicely(); + exit(1); #endif } else @@ -2929,7 +2930,7 @@ create_xlog_or_symlink(void) { fprintf(stderr, _("%s: could not create directory \"%s\": %s\n"), progname, subdirloc, strerror(errno)); - exit_nicely(); + exit(1); } } @@ -2991,7 +2992,7 @@ initialize_data_directory(void) { fprintf(stderr, _("%s: could not create directory \"%s\": %s\n"), progname, path, strerror(errno)); - exit_nicely(); + exit(1); } free(path); @@ -3266,6 +3267,8 @@ main(int argc, char *argv[]) exit(1); } + atexit(cleanup_directories_atexit); + /* If we only need to fsync, just do it and exit */ if (sync_only) { @@ -3276,7 +3279,7 @@ main(int argc, char *argv[]) { fprintf(stderr, _("%s: could not access directory \"%s\": %s\n"), progname, pg_data, strerror(errno)); - exit_nicely(); + exit(1); } fputs(_("syncing data to disk ... "), stdout); @@ -3412,5 +3415,6 @@ main(int argc, char *argv[]) destroyPQExpBuffer(start_db_cmd); + success = true; return 0; } -- 2.20.1
From 862c11cfd7c0d1784e23ceb9074b7846d964b7d1 Mon Sep 17 00:00:00 2001 From: Peter Eisentraut <pe...@eisentraut.org> Date: Sat, 5 Jan 2019 15:05:49 +0100 Subject: [PATCH v2 3/3] isolationtester: Use atexit() Replace exit_nicely() calls with standard exit() and register the cleanup actions using atexit(). --- src/test/isolation/isolationtester.c | 42 +++++++++++++--------------- 1 file changed, 19 insertions(+), 23 deletions(-) diff --git a/src/test/isolation/isolationtester.c b/src/test/isolation/isolationtester.c index 9134b0505e..e59cf667ba 100644 --- a/src/test/isolation/isolationtester.c +++ b/src/test/isolation/isolationtester.c @@ -32,7 +32,6 @@ static int nconns = 0; /* In dry run only output permutations to be run by the tester. */ static int dry_run = false; -static void exit_nicely(void) pg_attribute_noreturn(); static void run_testspec(TestSpec *testspec); static void run_all_permutations(TestSpec *testspec); static void run_all_permutations_recurse(TestSpec *testspec, int nsteps, @@ -51,15 +50,14 @@ static void printResultSet(PGresult *res); static void isotesterNoticeProcessor(void *arg, const char *message); static void blackholeNoticeProcessor(void *arg, const char *message); -/* close all connections and exit */ static void -exit_nicely(void) +disconnect_atexit(void) { int i; for (i = 0; i < nconns; i++) - PQfinish(conns[i]); - exit(1); + if (conns[i]) + PQfinish(conns[i]); } int @@ -140,7 +138,7 @@ main(int argc, char **argv) { fprintf(stderr, "duplicate step name: %s\n", testspec->allsteps[i]->name); - exit_nicely(); + exit(1); } } @@ -162,6 +160,7 @@ main(int argc, char **argv) */ nconns = 1 + testspec->nsessions; conns = calloc(nconns, sizeof(PGconn *)); + atexit(disconnect_atexit); backend_pids = calloc(nconns, sizeof(*backend_pids)); for (i = 0; i < nconns; i++) { @@ -170,7 +169,7 @@ main(int argc, char **argv) { fprintf(stderr, "Connection %d to database failed: %s", i, PQerrorMessage(conns[i])); - exit_nicely(); + exit(1); } /* @@ -196,7 +195,7 @@ main(int argc, char **argv) if (PQresultStatus(res) != PGRES_COMMAND_OK) { fprintf(stderr, "message level setup failed: %s", PQerrorMessage(conns[i])); - exit_nicely(); + exit(1); } PQclear(res); @@ -210,14 +209,14 @@ main(int argc, char **argv) { fprintf(stderr, "backend pid query returned %d rows and %d columns, expected 1 row and 1 column", PQntuples(res), PQnfields(res)); - exit_nicely(); + exit(1); } } else { fprintf(stderr, "backend pid query failed: %s", PQerrorMessage(conns[i])); - exit_nicely(); + exit(1); } PQclear(res); } @@ -254,7 +253,7 @@ main(int argc, char **argv) { fprintf(stderr, "prepare of lock wait query failed: %s", PQerrorMessage(conns[0])); - exit_nicely(); + exit(1); } PQclear(res); termPQExpBuffer(&wait_query); @@ -265,9 +264,6 @@ main(int argc, char **argv) */ run_testspec(testspec); - /* Clean up and exit */ - for (i = 0; i < nconns; i++) - PQfinish(conns[i]); return 0; } @@ -375,7 +371,7 @@ run_named_permutations(TestSpec *testspec) { fprintf(stderr, "undefined step \"%s\" specified in permutation\n", p->stepnames[j]); - exit_nicely(); + exit(1); } steps[j] = *this; } @@ -510,7 +506,7 @@ run_permutation(TestSpec *testspec, int nsteps, Step **steps) else if (PQresultStatus(res) != PGRES_COMMAND_OK) { fprintf(stderr, "setup failed: %s", PQerrorMessage(conns[0])); - exit_nicely(); + exit(1); } PQclear(res); } @@ -530,7 +526,7 @@ run_permutation(TestSpec *testspec, int nsteps, Step **steps) fprintf(stderr, "setup of session %s failed: %s", testspec->sessions[i]->name, PQerrorMessage(conns[i + 1])); - exit_nicely(); + exit(1); } PQclear(res); } @@ -612,7 +608,7 @@ run_permutation(TestSpec *testspec, int nsteps, Step **steps) { fprintf(stdout, "failed to send query for step %s: %s\n", step->name, PQerrorMessage(conn)); - exit_nicely(); + exit(1); } /* Try to complete this step without blocking. */ @@ -723,7 +719,7 @@ try_complete_step(Step *step, int flags) if (sock < 0) { fprintf(stderr, "invalid socket: %s", PQerrorMessage(conn)); - exit_nicely(); + exit(1); } gettimeofday(&start_time, NULL); @@ -741,7 +737,7 @@ try_complete_step(Step *step, int flags) if (errno == EINTR) continue; fprintf(stderr, "select failed: %s\n", strerror(errno)); - exit_nicely(); + exit(1); } else if (ret == 0) /* select() timeout: check for lock wait */ { @@ -761,7 +757,7 @@ try_complete_step(Step *step, int flags) { fprintf(stderr, "lock wait query failed: %s", PQerrorMessage(conns[0])); - exit_nicely(); + exit(1); } waiting = ((PQgetvalue(res, 0, 0))[0] == 't'); PQclear(res); @@ -818,14 +814,14 @@ try_complete_step(Step *step, int flags) { fprintf(stderr, "step %s timed out after 200 seconds\n", step->name); - exit_nicely(); + exit(1); } } else if (!PQconsumeInput(conn)) /* select(): data available */ { fprintf(stderr, "PQconsumeInput failed: %s\n", PQerrorMessage(conn)); - exit_nicely(); + exit(1); } } -- 2.20.1