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

BUG #4961: pg_standby.exe crashes with no args

From
""
Date:
The following bug has been logged online:

Bug reference:      4961
Logged by:
Email address:      wader2@jcom.home.ne.jp
PostgreSQL version: 8.4.0
Operating system:   Windows XP/2000
Description:        pg_standby.exe crashes with no args
Details:

also clashes when run with recovery.conf,
after logged message below.

could not restore file "00000001.history" from archive: return code
-1073741811

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

From
Fujii Masao
Date:
Hi,

On Mon, Aug 3, 2009 at 9:18 PM, <wader2@jcom.home.ne.jp> wrote:
> also clashes when run with recovery.conf,
> after logged message below.
>
> could not restore file "00000001.history" from archive: return code
> -1073741811

Does pg_standby (e.g., pg_standby.exe --version) succeed when you
execute it alone? If not, you might have failed the installation of it.

Regards,

--
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

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

From
wader2
Date:
thx,

> Does pg_standby (e.g., pg_standby.exe --version) succeed when you

--version and --help are no problem.

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

From
Bruce Momjian
Date:
wader2 wrote:
> thx,
>
> > Does pg_standby (e.g., pg_standby.exe --version) succeed when you
>
> --version and --help are no problem.

I can't reproduce a crash here on BSD:

    $ pg_standby
    pg_standby: not enough command-line arguments

Can you show us the command and the crash text?

--
  Bruce Momjian  <bruce@momjian.us>        http://momjian.us
  EnterpriseDB                             http://enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +

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

From
wader2
Date:
Bruce Momjian wrote:
> I can't reproduce a crash here on BSD:
>
>     $ pg_standby
>     pg_standby: not enough command-line arguments
>
> Can you show us the command and the crash text?

I guess this occurs on only windows (Japanese envionment?).

C:\Program Files\PostgreSQL\8.4\bin>pg_standby.exe

results no text on command line, Windows error dialog.

AppName:pg_standby.exe AppVer:0.0.0.0 ModName:msvcr80.dll
ModVer:8.0.50727.762   Offset:000091ad

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

From
"Hiroshi Saito"
Date:
Hi.

Yes,  I also reproduce it.  It is very strange......
==
Windows-XP Home Edition Version2002 Service Pack3(Japanese)
CPU N270 @ 1.60Ghz 1GB RAM
==

C:\Program Files\PostgreSQL\8.4\bin>pg_standby.exe --help
pg_standby allows PostgreSQL warm standby servers to be configured.

Usage:
<snip>

C:\Program Files\PostgreSQL\8.4\bin>pg_standby.exe
...crash...

However,  MinGW+gcc returns a normal result.

$ pg_standby
pg_standby: not enough command-line arguments

I don't have the margin time which still investigates it.

Regards,
Hiroshi Saito

----- Original Message -----
From: "wader2" <wader2@jcom.home.ne.jp>


> Bruce Momjian wrote:
>> I can't reproduce a crash here on BSD:
>>
>> $ pg_standby
>> pg_standby: not enough command-line arguments
>>
>> Can you show us the command and the crash text?
>
> I guess this occurs on only windows (Japanese envionment?).
>
> C:\Program Files\PostgreSQL\8.4\bin>pg_standby.exe
>
> results no text on command line, Windows error dialog.
>
> AppName:pg_standby.exe AppVer:0.0.0.0 ModName:msvcr80.dll
> ModVer:8.0.50727.762   Offset:000091ad
>
> --
> Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-bugs
>

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

From
Magnus Hagander
Date:
On Mon, Aug 10, 2009 at 16:10, wader2<wader2@jcom.home.ne.jp> wrote:
> Bruce Momjian wrote:
>>
>> I can't reproduce a crash here on BSD:
>>
>>        $ pg_standby
>>        pg_standby: not enough command-line arguments
>>
>> Can you show us the command and the crash text?
>
> I guess this occurs on only windows (Japanese envionment?).
>
> C:\Program Files\PostgreSQL\8.4\bin>pg_standby.exe
>
> results no text on command line, Windows error dialog.
>
> AppName:pg_standby.exe AppVer:0.0.0.0 ModName:msvcr80.dll
> ModVer:8.0.50727.762   Offset:000091ad

I have reproduced this. The problem is:(void) signal(SIGUSR1, sighandler);(void) signal(SIGINT, sighandler);    /*
deprecated,use SIGUSR1 */ 


None of these signals exist on WIN32. I think the only reason it
compiles at all is that we bring in *some* of our signals emulation
code, but certainly not all of it.

If I just move those two lines into the #ifndef WIN32 block just
around it, it compiles and doesn't crash on running-with-no-arguments.
I haven't tried to actually use it though - can someone confirm if
this will actually make pg_standby not work properly?

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


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

From
Tom Lane
Date:
Magnus Hagander <magnus@hagander.net> writes:
> If I just move those two lines into the #ifndef WIN32 block just
> around it, it compiles and doesn't crash on running-with-no-arguments.
> I haven't tried to actually use it though - can someone confirm if
> this will actually make pg_standby not work properly?

It would mean there's no way to trigger failover via signal.

I think what we need is for pg_ctl to be able to send these signals...
        regards, tom lane


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

From
Magnus Hagander
Date:
On Mon, Aug 10, 2009 at 20:44, Tom Lane<tgl@sss.pgh.pa.us> wrote:
> Magnus Hagander <magnus@hagander.net> writes:
>> If I just move those two lines into the #ifndef WIN32 block just
>> around it, it compiles and doesn't crash on running-with-no-arguments.
>> I haven't tried to actually use it though - can someone confirm if
>> this will actually make pg_standby not work properly?
>
> It would mean there's no way to trigger failover via signal.
>
> I think what we need is for pg_ctl to be able to send these signals...

Those signals don't *exist* on Windows. The whole idea of
cross-process signals don't *exist* on Windows.

We emulate it in the main backend, by creating a background thread
that sets a global variable. That is then polled in the
CHECK_FOR_INTERRUPTS macro.  pg_ctl is perfectly capable of sending
these signals, but pg_standby can't receive them.

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.


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


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

From
wader2
Date:
I can't compile nor read source, but can tell you that
pg_standby.exe in 8.3.7 works fine.

Magnus Hagander wrote:
> On Mon, Aug 10, 2009 at 20:44, Tom Lane<tgl@sss.pgh.pa.us> wrote:
>> Magnus Hagander <magnus@hagander.net> writes:
>>> If I just move those two lines into the #ifndef WIN32 block just
>>> around it, it compiles and doesn't crash on running-with-no-arguments.
>>> I haven't tried to actually use it though - can someone confirm if
>>> this will actually make pg_standby not work properly?
>> It would mean there's no way to trigger failover via signal.
>>
>> I think what we need is for pg_ctl to be able to send these signals...
> 
> Those signals don't *exist* on Windows. The whole idea of
> cross-process signals don't *exist* on Windows.
> 
> We emulate it in the main backend, by creating a background thread
> that sets a global variable. That is then polled in the
> CHECK_FOR_INTERRUPTS macro.  pg_ctl is perfectly capable of sending
> these signals, but pg_standby can't receive them.
> 
> 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.
> 
> 



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

From
Magnus Hagander
Date:
Yeah, the patch I think breaks things isn't included in 8.3.7 - it
will be in 8.3.8 though...

//Magnus

On Wed, Aug 12, 2009 at 11:08, wader2<wader2@jcom.home.ne.jp> wrote:
> I can't compile nor read source, but can tell you that
> pg_standby.exe in 8.3.7 works fine.
>
> Magnus Hagander wrote:
>>
>> On Mon, Aug 10, 2009 at 20:44, Tom Lane<tgl@sss.pgh.pa.us> wrote:
>>>
>>> Magnus Hagander <magnus@hagander.net> writes:
>>>>
>>>> If I just move those two lines into the #ifndef WIN32 block just
>>>> around it, it compiles and doesn't crash on running-with-no-arguments.
>>>> I haven't tried to actually use it though - can someone confirm if
>>>> this will actually make pg_standby not work properly?
>>>
>>> It would mean there's no way to trigger failover via signal.
>>>
>>> I think what we need is for pg_ctl to be able to send these signals...
>>
>> Those signals don't *exist* on Windows. The whole idea of
>> cross-process signals don't *exist* on Windows.
>>
>> We emulate it in the main backend, by creating a background thread
>> that sets a global variable. That is then polled in the
>> CHECK_FOR_INTERRUPTS macro.  pg_ctl is perfectly capable of sending
>> these signals, but pg_standby can't receive them.
>>
>> 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.
>>
>>
>
>



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


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

From
Fujii Masao
Date:
Hi,

On Tue, Aug 11, 2009 at 3:10 AM, Magnus Hagander<magnus@hagander.net> wrote:
> I have reproduced this. The problem is:
>        (void) signal(SIGUSR1, sighandler);
>        (void) signal(SIGINT, sighandler);      /* deprecated, use SIGUSR1 */
>
>
> None of these signals exist on WIN32. I think the only reason it
> compiles at all is that we bring in *some* of our signals emulation
> code, but certainly not all of it.

SIGINT has been used in pg_standby for a while (e.g., v8.3.7). I wonder
why this problem arises only in v8.4.0.

Regards,

--
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center


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

From
Magnus Hagander
Date:
On Wed, Aug 12, 2009 at 19:08, Fujii Masao<masao.fujii@gmail.com> wrote:
> Hi,
>
> On Tue, Aug 11, 2009 at 3:10 AM, Magnus Hagander<magnus@hagander.net> wrote:
>> I have reproduced this. The problem is:
>>        (void) signal(SIGUSR1, sighandler);
>>        (void) signal(SIGINT, sighandler);      /* deprecated, use SIGUSR1 */
>>
>>
>> None of these signals exist on WIN32. I think the only reason it
>> compiles at all is that we bring in *some* of our signals emulation
>> code, but certainly not all of it.
>
> SIGINT has been used in pg_standby for a while (e.g., v8.3.7). I wonder
> why this problem arises only in v8.4.0.

Not sure. Potentially pure luck. SIGINT has never *worked*, though, it
just hasn't crashed.

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


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

From
Fujii Masao
Date:
Hi,

On Thu, Aug 13, 2009 at 2:24 AM, Magnus Hagander<magnus@hagander.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;
--------------------

Regards,

-- 
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center


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

From
Magnus Hagander
Date:
On Wed, Aug 19, 2009 at 08:38, Fujii Masao<masao.fujii@gmail.com> wrote:
> On Thu, Aug 13, 2009 at 2:24 AM, Magnus Hagander<magnus@hagander.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 HaganderMe: http://www.hagander.net/Work: http://www.redpill-linpro.com/


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

From
Heikki Linnakangas
Date:
Magnus Hagander wrote:
> 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?

I think we should just remove the signals support for win32. The trigger
file method still works, and the signal method has always been a bit
iffy (it doesn't work when pg_standby isn't running, for example, which
happens between processing of each WAL file, because there's no process
to signal).

Is pg_standby killed correctly when postmaster dies?

--  Heikki Linnakangas EnterpriseDB   http://www.enterprisedb.com


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

From
Magnus Hagander
Date:
On Wed, Aug 19, 2009 at 11:45, Heikki
Linnakangas<heikki.linnakangas@enterprisedb.com> wrote:
> Magnus Hagander wrote:
>> 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?
>
> I think we should just remove the signals support for win32. The trigger
> file method still works, and the signal method has always been a bit
> iffy (it doesn't work when pg_standby isn't running, for example, which
> happens between processing of each WAL file, because there's no process
> to signal).

Fair enough.


> Is pg_standby killed correctly when postmaster dies?

No idea. I can set up and env to test, but I don't have one ready.
Anybody else have tried this?

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


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

From
Fujii Masao
Date:
Hi,

Sorry for this late reply.

On Wed, Aug 19, 2009 at 6:45 PM, Heikki Linnakangas
<heikki.linnakangas@enterprisedb.com> wrote:
> Is pg_standby killed correctly when postmaster dies?

No. In my test (v8.3.7 on Win), an immediate shutdown was able to
kill postmaster, but not pg_standby and the startup process.

Though we should change those to handle the shutdown signals
correctly, this change is not imperative need. At first, we should
just get rid of the signal support from pg_standby on Win, in order
to avoid the pg_standby crash. The attached is the patch to do so.
If this patch is OK, the backport is required for 8.3.x.

Thought?

Regards,

--
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

Attachment

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

From
Heikki Linnakangas
Date:
Fujii Masao wrote:
> On Wed, Aug 19, 2009 at 6:45 PM, Heikki Linnakangas
> <heikki.linnakangas@enterprisedb.com> wrote:
>> Is pg_standby killed correctly when postmaster dies?
> 
> No. In my test (v8.3.7 on Win), an immediate shutdown was able to
> kill postmaster, but not pg_standby and the startup process.

Hmm. We should do something about that.. Magnus?

> Though we should change those to handle the shutdown signals
> correctly, this change is not imperative need. At first, we should
> just get rid of the signal support from pg_standby on Win, in order
> to avoid the pg_standby crash. The attached is the patch to do so.
> If this patch is OK, the backport is required for 8.3.x.

Ok, committed to HEAD, 8.4 and 8.3.

--  Heikki Linnakangas EnterpriseDB   http://www.enterprisedb.com


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

From
Fujii Masao
Date:
Hi,

On Wed, Nov 4, 2009 at 9:53 PM, Heikki Linnakangas
<heikki.linnakangas@enterprisedb.com> wrote:
>> Though we should change those to handle the shutdown signals
>> correctly, this change is not imperative need. At first, we should
>> just get rid of the signal support from pg_standby on Win, in order
>> to avoid the pg_standby crash. The attached is the patch to do so.
>> If this patch is OK, the backport is required for 8.3.x.
>
> Ok, committed to HEAD, 8.4 and 8.3.

Thanks a lot!

Regards,

-- 
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center