On Fri, Dec 1, 2023 at 9:31 AM Andres Freund <andres@anarazel.de> wrote:
> On 2023-11-30 12:44:33 -0600, Tristan Partin wrote:
> > > + /*
> > > + * Set reference point for stack-depth checking. This might seem
> > > + * redundant in !EXEC_BACKEND builds; but it's not because the postmaster
> > > + * launches its children from signal handlers, so we might be running on
> > > + * an alternative stack. XXX still true?
> > > + */
> > > + (void) set_stack_base();
> >
> > Looks like there is still this XXX left. Can't say I completely understand
> > the second sentence either.
>
> We used to start some child processes of postmaster in signal handlers. That
> was fixed in
>
> commit 7389aad6366
> Author: Thomas Munro <tmunro@postgresql.org>
> Date: 2023-01-12 12:34:23 +1300
>
> Use WaitEventSet API for postmaster's event loop.
>
>
> In some cases signal handlers run on a separate stack, which meant that the
> set_stack_base() we did in postmaster would yield a completely bogus stack
> depth estimation. So this comment should likely have been removed. Thomas?
Right, I should delete that comment in master and 16. While wondering
what to write instead, my first thought is that it is better to leave
the actual call there though, because otherwise there is a small
difference in stack reference point between EXEC_BACKEND and
!EXEC_BACKEND builds, consumed by a few postmaster stack frames. So
the new comment would just say that.
(I did idly wonder if there is a longjmp trick to zap those frames
post-fork, not looked into and probably doesn't really improve
anything we care about...)