On 21 August 2017 at 10:57, Craig Ringer <cr...@2ndquadrant.com> wrote:

> Hi all
> I've noticed a possible bug / design limitation where shm_mq_wait_internal
> sleep in a latch wait forever, and the postmaster gets stuck waiting for
> the bgworker the wait is running in to exit.
> This happens when the shm_mq does not have an associated bgworker handle
> registered because the other end is not known at mq creation time or is a
> normal backend not a bgworker. So a BGW handle cannot be passed.
> shm_mq_wait_internal() will CHECK_FOR_INTERRUPTS() when its latch wait is
> interrupted by a SIGTERM. But it doesn't actually respond to SIGTERM in any
> way; it just merrily resets its latch and keeps looping.
> It will bail out correctly on SIGQUIT.
> If the proc waiting to attach was known at queue creation time and was a
> bgworker, we'd pass a bgworker handle and the mq would notice it failed to
> start and stop waiting. There's only a problem if no bgworker handle can be
> supplied.
> The underlying problem is that CHECK_FOR_INTERRUPTS() doesn't care about
> SIGTERM or have any way to test for it. And we don't have any global
> management of SIGTERM like we do SIGQUIT so the shm_mq_wait_internal loop
> can't test for it.
> The only ways I can see to fix this are:
> * Generalize SIGTERM handling across postgres, so there's a global
> "got_SIGTERM" global that shm_mq_wait_internal can test to break out of its
> loop, and every backend's signal handler must set it. Lots of churn.
> * In a proc's signal handler, use globals set before entry and after exit
> from shm_mq operations to detect if we're currently in shm_mq and promote
> SIGTERM to SIGQUIT by sending a new signal to ourselves. Or set up state so
> CHECK_FOR_INTERRUPTS() will notice when the handler returns.
> * Allow passing of a *bool that tests for SIGTERM, or a function pointer
> called on each iteration to test whether looping should continue, to be
> passed to shm_mq_attach. So if you can't supply a bgw handle, you supply
> that instead. Provide a shm_mq_set_handle equivalent for it too.
> Any objections to the last approach?

BTW, you can work around it in extension code for existing versions with
something like this in your bgworker:

volatile bool                   in_shm_mq = false;


        if (in_shm_mq)
                 * shm_mq can get stuck in shm_mq_wait_internal on SIGTERM;
                 * To work around this we keep track of whether we're in
                 * and generate a fake interrupt for CHECK_FOR_INTERRUPTS()
                 * process if so.
                 * The guard around in_shm_mq may not be necessary, but
                 * it any SIGTERM will likely cause ereport(FATAL) with
                 * "terminating connection due to administrator command"
                 * which isn't ideal.
                InterruptPending = true;
                ProcDiePending = true;


inline static shm_mq_handle *
myext_shm_mq_attach(shm_mq *mq, dsm_segment *seg, BackgroundWorkerHandle
        shm_mq_handle *ret;
        in_shm_mq = true;
        ret = shm_mq_attach(mq, seg, handle);
        in_shm_mq = false;
        return ret;

/* repeated for shm_mq_receive, shm_mq_send, shm_mq_sendv,
shm_mq_wait_for_attach */

You can instead use non-blocking sends instead, and sleep on your own
latch, doing the same work as shm_mq_wait_internal yourself.

 Craig Ringer                   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services

Reply via email to