Re: NULL pointer dereference in syslogger with load_libraries() and -DEXEC_BACKEND at startup

On Mon, May 25, 2026 at 11:39:54PM -0300, Euler Taveira wrote:
> It seems I was too optimistic about this patch. Since the commit 0c8e082fba8
> sets MyBackendType to B_LOGGER earlier, it breaks the following assumption in
> syslogger.c.
>
> /*
> * If we're told to write to a structured log file, but it's not open,
> * dump the data to syslogFile (which is always open) instead. This can
> * happen if structured output is enabled after postmaster start and we've
> * been unable to open logFile. There are also race conditions during a
> * parameter change whereby backends might send us structured output
> * before we open the logFile or after we close it. Writing formatted
> * output to the regular log file isn't great, but it beats dropping log
> * output on the floor.
>
> It shouldn't assume syslogFile is always open. The send_message_to_server_log()
> shouldn't be executing the following code path in this case.

This comment has been rewritten in 2022 when jsonlog was added in
dc686681e079, but this assumption is much older than that:
bff84a547d71, where the syslogger has been made more robust. And we
assume that the log file is always open, so this change is breaking
an old assumption.

> It could set MyBackendType only if child_type != B_LOGGER during
> launch_backend.c and set it at the same code path as in the past. However, I
> consider this solution ugly and hackish.

While thinking about an approach that could allow to keep
0c8e082fba8d, I was wondering whether we should have a boolean flag
that tracks if the log file is opened or not that gets set (we should
not care about the reset) when we are done with its creation, but I'm
feeling that this makes the logic feeble. We know only rely on
MyBackendType for the job which means to complicate all these checks.
The part that makes me uneasy is that the logging facility should be
robust by design, and simpler is always better IMO.

An exception where we don't set MyBackendType and have an exception
for this corresponding child_type value does not really feel right to
me either.. As a whole, I am not sure to like what has been done
here.
--
Michael