Re: BUG #4961: pg_standby.exe crashes with no args

On Wed, Aug 19, 2009 at 08:38, Fujii Masao<masao(dot)fujii(at)gmail(dot)com> wrote:
> On Thu, Aug 13, 2009 at 2:24 AM, Magnus Hagander<magnus(at)hagander(dot)net> wrote:
>> Not sure. Potentially pure luck. SIGINT has never *worked*, though, it
>> just hasn't crashed.
>
> OK.
>
>> We could implement the same type of check in pg_standby, but it
>> requires something like CHECK_FOR_INTERRUPTS. And these interrupts
>> won't, by default, cause any kind of interruption of the process. In
>> the backend, we interrupt socket calls because we have the socket
>> wrapper layer, and nothing else. I don't know how doable this would be
>> in pg_standby - does it always block on a single thing where we could
>> stick some win32 synchronization code? If it's a single, or limited,
>> places we could implement something similar to the backend. But if we
>> need to interrupt at arbitrary locations, that's just not possible.
>
> I think that CHECK_FOR_INTERRUPTS should be placed just before
> checking the flag 'signaled' which may be enabled by the signal handler.
> Here is the pseudo-code.
>
> --------------------
>        {
>                /* Check for trigger file or signal first */
>                CheckForExternalTrigger();
> + #ifdef WIN32
> +               CHECK_FOR_INTERRUPTS();
> + #endif /* WIN32 */
>                if (signaled)
>                {
>                        Failover = FastFailover;
> --------------------

It's going to take a lot more than that, really. I looked a bit more
at it, and I think the best way is to do a win32 specific thing all
the way, not just emulate signals. Given that we only have a couple of
signal() calls anyway. I think that's going to be much clearer in what
it does, and also a lot simpler code in the end. The reason we have
the full emulation layer is that we use signals all over the place in
the backend.

From what I can tell, we don't actually need to interrupt anything
other than the sleep call on line 810, because we only check for the
signaled=true state in that loop anyway. Can someone more
knowledgeable in the pg_standby code comment on that?

This would amount to fairly major surgery for pg_standby on Win32. Is
that something we'd want to backpatch, or do we want to backpatch just
the removal of the signal() calls which would amount to not supporting
signals in pg_standby on win32?

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/