BUG #11805: Missing SetServiceStatus call during service shutdown in pg_ctl (Windows only)

From: krystian(dot)bigaj(at)gmail(dot)com
To: pgsql-bugs(at)postgresql(dot)org
Subject: BUG #11805: Missing SetServiceStatus call during service shutdown in pg_ctl (Windows only)
Date: 2014-10-28 07:02:41
Message-ID: 20141028070241.2593.58180@wrigleys.postgresql.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-bugs pgsql-hackers

The following bug has been logged on the website:

Bug reference: 11805
Logged by: Krystian Bigaj
Email address: krystian(dot)bigaj(at)gmail(dot)com
PostgreSQL version: 9.3.5
Operating system: Windows 7 Pro x64
Description:

pg_ctl on Windows during service start/shutdown should notify service
manager about it's status by increment dwCheckPoint and call to
SetServiceStatus/pgwin32_SetServiceStatus.

However during shutdown there is a missing call to SetServiceStatus.
See src\bin\pg_ctl\pg_ctl.c:
[code]
static void WINAPI
pgwin32_ServiceMain(DWORD argc, LPTSTR *argv)
{
...
/*
* Increment the checkpoint and try again Abort after 12
* checkpoints as the postmaster has probably hung
*/
while (WaitForSingleObject(postmasterProcess, 5000) ==
WAIT_TIMEOUT && status.dwCheckPoint < 12)
status.dwCheckPoint++; <------- missing SetServiceStatus
call
break;

case (WAIT_OBJECT_0 + 1): /* postmaster went down */
break;
[/code]

As you can see there is only a dwCheckPoint increment, but there is no call
to SetServiceStatus(hStatus, (LPSERVICE_STATUS) &status);
This problem was reported before here:
http://www.postgresql.org/message-id/CAN=kAeGOEmfh_+vdg+2oD=9KhWzGn4NNqfZNdHoQ_2QOsHhuLQ@mail.gmail.com

Another problem is with above condition "status.dwCheckPoint < 12" if
service (pg_ctl) is started with -w parameter (wait for startup).
In that case test_postmaster_connection(true) increments
status.dwCheckPoint,
so during shutdown that value can be larger than 0 (and even larger than 12,
because default wait time is 60s), so there could be only one 5000ms wait
for postmaster shutdown.

Patch to fix for this bugs could looks like this:
[code]
case WAIT_OBJECT_0: /* shutdown event */
/*
* Value status.dwCheckPoint can be incremented by
test_postmaster_connection(true)
* so dwCheckPoint might not start from 0.
*/
int maxShutdownCheckPoint = status.dwCheckPoint + 12;

kill(postmasterPID, SIGINT);

/*
* Increment the checkpoint and try again Abort after 12
* checkpoints as the postmaster has probably hung
*/
while (WaitForSingleObject(postmasterProcess, 5000) ==
WAIT_TIMEOUT && status.dwCheckPoint < maxShutdownCheckPoint)
{
status.dwCheckPoint++;
SetServiceStatus(hStatus, (LPSERVICE_STATUS) &status);
}
break;
[/code]

Responses

Browse pgsql-bugs by date

  From Date Subject
Next Message Krystian Bigaj 2014-10-28 08:04:05 Re: BUG #11805: Missing SetServiceStatus call during service shutdown in pg_ctl (Windows only)
Previous Message Stephen Frost 2014-10-27 20:18:05 Re: BUG #10680: LDAP bind password leaks to log on failed authentication

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2014-10-28 07:41:01 Re: Add CREATE support to event triggers
Previous Message Adam Brightwell 2014-10-28 06:40:33 Re: alter user/role CURRENT_USER