Bruce Momjian:
> With no one "hoping this patch dies in a fire"*, I have updated it with
> more details, which I now think is committable to master. Is this
> something to backpatch? Seems too rare a bug to me.
I would like to see this backpatched to be able to run the regression
tests easily on all stable branches.
I have taken your's/Thomas' patch and extended it with a few more taking
in many of the ideas in this thread:
0001 Don't clobber LD_*
This is the patch you posted. This applies cleanly all the way down to
v12. This fixes the bug and allows running most of the tests with musl -
yeah!
I also confirmed, that this will not create practical problems with
library/postgres docker image, where this is likely used the most. While
"postgres" is called by default without any arguments here, plenty of
environment variables are passed. The docker image does use LD_PRELOAD
to trick initdb, but that's not set when running the postmaster, so not
a problem here.
This use-case also shows why the proposed patch to still partially
clobber environ at this stage is better than to not clobber environ at
all - in this case, the docker image would essentially have no ps status
at all by default.-
0002 Allow setting PS_USE_NONE via CPPFLAGS
This was proposed by Andrew and applies cleanly down to v12. Thus, it
could be backpatched, too. First and foremost this would allow setting a
buildfarm animal to use this flag to make sure this code path is
actually build/tested at all. This is something that Thomas and Tom
hinted at.
0003 Don't ever clobber environ again
This is the approach I had previously posted as a PoC. This would not be
backpatched, but I suggested this could go into v17 now. This avoids the
undefined behavior and sets the table to eventually set ps status via
argv by default and remove PS_USE_NONE later.
Compared to the PoC patch, I decided not to pad argv[0], because this
will break the ps status display for the postmaster itself. Instead exec
is called with an additional argument consisting of exactly 255 spaces.
I also tried avoiding the additional exec-call if postgres was called
via pg_ctl, as suggested by Peter. This quickly turned out to be more
invasive than I would have liked this to be, though.
The current approach works very well, the environment doesn't need to be
copied anymore and the workaround for /proc/<pid>/environ in main.c can
go away, too.
0004 Default to PS_USE_CLOBBER_ARGV
This changes the default to display ps status on all other systems, too.
This could potentially go in now as well, or be delayed to the beginning
of the v18 cycle. In the unlikely event that this breaks something on a
platform not considered here and we get a bug report, we can easily
advise to compile with CPPFLAGS=-DPS_USE_NONE, which is still there at
this stage.
0005 Remove PS_USE_NONE
However, if no reports come in and no problems are detected with 0004,
then this can be entirely removed. This for "later", whenever that is.
Best,
Wolfgang
