Date: Fri, 4 Jan 2019 16:35:51 -0300
From: Alvaro Herrera <alvherre@2ndquadrant.com>
To: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
Cc: pgsql-hackers <pgsql-hackers@postgresql.org>
Subject: Re: Use atexit() in initdb and pg_basebackup
Message-ID: <201901041935.aqi5ogzbyiak@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <ec4135ba-84e9-28bf-b584-0e78d47448d5@2ndquadrant.com>
User-Agent: NeoMutt/20180716-1013-2c1e88
Precedence: bulk

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