Feedback-ID: ie3de48e3:Fastmail
Date: Tue, 26 May 2026 19:21:56 +0200
From: =?utf-8?Q?=C3=81lvaro?= Herrera <alvherre@kurilemu.de>
To: Euler Taveira <euler@eulerto.com>
Cc: Michael Paquier <michael@paquier.xyz>,
	Kyotaro Horiguchi <horikyota.ntt@gmail.com>,
 pgsql-hackers@lists.postgresql.org
Subject: Re: NULL pointer dereference in syslogger with load_libraries() and
 -DEXEC_BACKEND at startup
Message-ID: <ahXPSvAisIgsIdso@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <b8ff6a67-8742-4527-b99d-ff33dd11f56c@app.fastmail.com>
Archived-At: 
 <https://www.postgresql.org/message-id/ahXPSvAisIgsIdso%40alvherre.pgsql>
Precedence: bulk

On 2026-May-26, Euler Taveira wrote:

> It fixes the issue for me. However, I'm wondering if we can avoid adding a new
> boolean for it. We already have redirection_done that guarantees the syslogger
> is working properly.

Yeah, that was my first thought as well, but after looking at it, I
thought it may be better not to mess with that, since it was almost
surgical in how it was designed; furthermore, it is written to be
transmitted from postmaster to its children, whereas this new state
variable is local to the syslogger process.

Also, consider the case where you have postmaster running for a while
(so redirection_done has been set already), and for whatever reason
syslogger restarts.  Will it inherit the correct value?  I don't know (I
think it won't), and it seems error-prone to assume it will work
correctly.  I think this would only fail if for whatever reason the
syslogger itself wanted to print an error before setting up the file
descriptors, but I don't think this is impossible: for example, if
postmaster_child_launch() runs into an OOM during internal_forkexec().

Plus, redirection_done means something completely different: it is used
to say that syslogPipe[] has been set up for message chunk transmission.
The new variable indicates that the logging file descriptors for the
various output files (syslogFile etc) have been set up in the syslogger
process.

All in all, I don't think we should cut corners here just to save one
boolean variable.

-- 
Álvaro Herrera         PostgreSQL Developer  —  https://www.EnterpriseDB.com/
"No hay ausente sin culpa ni presente sin disculpa" (Prov. francés)