Use atexit() in initdb and pg_basebackup
Hackorum builds and tests every patch posted to the lists, not only commitfest submissions. This is Hackorum's own CI rather than the PostgreSQL project's, and it is still under testing - please report anything that looks wrong.
This thread has been committed, so CI has stopped here. Anything below is the last result it produced.
You can run a PostgreSQL built from this patch straight from Docker, with no checkout and no build:
docker run --rm -p 5432:5432 ghcr.io/hackorum-dev/postgres-patch:t39907psql -h localhost -U postgresBuilt from patchset v4 (message #4), July 28, 2026 at 04:45 AM.
Every patchset is also pushed to a branch of our PostgreSQL fork, so you can check out the same tree CI built. Without a PostgreSQL checkout:
git clone --branch t39907_4 https://github.com/hackorum-dev/postgres.gitIn a checkout you already have, add the fork once:
git remote add hackorum https://github.com/hackorum-dev/postgres.gitthen, for this patchset and every later one:
git fetch hackorum t39907_4 && git checkout t39907_4Patchset v4 (message #4) is on t39907_4
initdb and pg_basebackup can use atexit() to register cleanup actions
instead of requiring the use of custom exit_nicely() etc. Patches attached.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On 2018-Dec-29, Peter Eisentraut wrote:
@@ -387,6 +388,7 @@ StreamLog(void)
if (!conn)
/* Error message already written in GetConnection() */
return;
+ atexit(disconnect_atexit);if (!CheckServerVersionForStreaming(conn))
{
Seems you're registering the atexit cb twice here; you should only do so
in the first "!conn" block.
It would be nicer to be able to call atexit() in GetConnection() instead
of at each callsite, but that would require a place to save each conn
struct into, which is probably more work than warranted.
@@ -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.
Other than the first point, LGTM.
--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On Fri, Jan 04, 2019 at 04:35:51PM -0300, Alvaro Herrera wrote:
On 2018-Dec-29, Peter Eisentraut wrote:
@@ -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.
Other than the first point, LGTM.
Re-meuh (French version). That's partially a problem of this patch
because all those flags get reset. I think that it would be cleaner
to replace all those boolean flags with just a simple bits16 or such,
making the flag cleanup reset way cleaner, and less error-prone if
more flag types are added in the future.
--
Michael
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
Attachments:
t39907_4v2-0001-pg_basebackup-Use-atexit.patchtext/plain; charset=UTF-8; name=v2-0001-pg_basebackup-Use-atexit.patch; x-mac-creator=0; x-mac-type=0Download+104-102
v2-0002-initdb-Use-atexit.patchtext/plain; charset=UTF-8; name=v2-0002-initdb-Use-atexit.patch; x-mac-creator=0; x-mac-type=0Download+43-40
v2-0003-isolationtester-Use-atexit.patchtext/plain; charset=UTF-8; name=v2-0003-isolationtester-Use-atexit.patch; x-mac-creator=0; x-mac-type=0Download+19-24
On 2019-Jan-05, Peter Eisentraut wrote:
On 04/01/2019 20:35, Alvaro Herrera wrote:
+ /* 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.
Ah, yeah, much better, LGTM.
I also added an atexit() conversion in isolationtester. It's mostly the
same thing.
LGTM in a quick eyeball.
--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On 05/01/2019 16:42, Alvaro Herrera wrote:
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.Ah, yeah, much better, LGTM.
I also added an atexit() conversion in isolationtester. It's mostly the
same thing.LGTM in a quick eyeball.
committed
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services