Re: PostgreSQL in Windows console and Ctrl-C

Hello all,

when pg_ctl start is used to run PostgreSQL in a console window on
Windows, it runs in the background (it is terminated by closing the
window, but that is probably inevitable). There is one problem, however:
The first Ctrl-C in that window, no matter in which situation, will
cause the background postmaster to exit. If you, say, ping something,
and press Ctrl-C to stop ping, you probably don't want the database to
go away, too.

The reason is that Windows delivers the Ctrl-C event to all processes
using that console, not just to the foreground one.

Here's a patch to fix that. "pg_ctl stop" still works, and it has no
effect when running as a service, so it should be safe. It starts the
postmaster in a new process group (similar to calling setpgrp() after
fork()) that does not receive Ctrl-C events from the console window.

--
Christian

Can someone with Windows expertise comment on whether this should be
applied?

---------------------------------------------------------------------------

On Tue, Jan 7, 2014 at 12:44:33PM +0100, Christian Ullrich wrote:
> Hello all,
>
> when pg_ctl start is used to run PostgreSQL in a console window on
> Windows, it runs in the background (it is terminated by closing the
> window, but that is probably inevitable). There is one problem,
> however: The first Ctrl-C in that window, no matter in which
> situation, will cause the background postmaster to exit. If you,
> say, ping something, and press Ctrl-C to stop ping, you probably
> don't want the database to go away, too.
>
> The reason is that Windows delivers the Ctrl-C event to all
> processes using that console, not just to the foreground one.
>
> Here's a patch to fix that. "pg_ctl stop" still works, and it has no
> effect when running as a service, so it should be safe. It starts
> the postmaster in a new process group (similar to calling setpgrp()
> after fork()) that does not receive Ctrl-C events from the console
> window.
>
> --
> Christian

> diff --git a/src/bin/pg_ctl/pg_ctl.c b/src/bin/pg_ctl/pg_ctl.c
> new file mode 100644
> index 50d4586..89a9544
> *** a/src/bin/pg_ctl/pg_ctl.c
> --- b/src/bin/pg_ctl/pg_ctl.c
> *************** CreateRestrictedProcess(char *cmd, PROCE
> *** 1561,1566 ****
> --- 1561,1567 ----
> HANDLE restrictedToken;
> SID_IDENTIFIER_AUTHORITY NtAuthority = {SECURITY_NT_AUTHORITY};
> SID_AND_ATTRIBUTES dropSids[2];
> + DWORD flags;
>
> /* Functions loaded dynamically */
> __CreateRestrictedToken _CreateRestrictedToken = NULL;
> *************** CreateRestrictedProcess(char *cmd, PROCE
> *** 1636,1642 ****
> AddUserToTokenDacl(restrictedToken);
> #endif
>
> ! r = CreateProcessAsUser(restrictedToken, NULL, cmd, NULL, NULL, TRUE, CREATE_SUSPENDED, NULL, NULL, &si, processInfo);
>
> Kernel32Handle = LoadLibrary("KERNEL32.DLL");
> if (Kernel32Handle != NULL)
> --- 1637,1650 ----
> AddUserToTokenDacl(restrictedToken);
> #endif
>
> ! flags = CREATE_SUSPENDED;
> !
> ! /* Protect console process from Ctrl-C */
> ! if (!as_service) {
> ! flags |= CREATE_NEW_PROCESS_GROUP;
> ! }
> !
> ! r = CreateProcessAsUser(restrictedToken, NULL, cmd, NULL, NULL, TRUE, flags, NULL, NULL, &si, processInfo);
>
> Kernel32Handle = LoadLibrary("KERNEL32.DLL");
> if (Kernel32Handle != NULL)

>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers(at)postgresql(dot)org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ Everyone has their own god. +

On Fri, Apr 11, 2014 at 7:44 AM, Bruce Momjian <bruce(at)momjian(dot)us> wrote:
>
> Can someone with Windows expertise comment on whether this should be
> applied?

I tested the same in windows and it is working as specified.
The same background running server can be closed with ctrl+break command.

> ---------------------------------------------------------------------------
>
> On Tue, Jan 7, 2014 at 12:44:33PM +0100, Christian Ullrich wrote:
>> Hello all,
>>
>> when pg_ctl start is used to run PostgreSQL in a console window on
>> Windows, it runs in the background (it is terminated by closing the
>> window, but that is probably inevitable). There is one problem,
>> however: The first Ctrl-C in that window, no matter in which
>> situation, will cause the background postmaster to exit. If you,
>> say, ping something, and press Ctrl-C to stop ping, you probably
>> don't want the database to go away, too.
>>
>> The reason is that Windows delivers the Ctrl-C event to all
>> processes using that console, not just to the foreground one.
>>
>> Here's a patch to fix that. "pg_ctl stop" still works, and it has no
>> effect when running as a service, so it should be safe. It starts
>> the postmaster in a new process group (similar to calling setpgrp()
>> after fork()) that does not receive Ctrl-C events from the console
>> window.
>>
>> --
>> Christian
>
>> diff --git a/src/bin/pg_ctl/pg_ctl.c b/src/bin/pg_ctl/pg_ctl.c
>> new file mode 100644
>> index 50d4586..89a9544
>> *** a/src/bin/pg_ctl/pg_ctl.c
>> --- b/src/bin/pg_ctl/pg_ctl.c
>> *************** CreateRestrictedProcess(char *cmd, PROCE
>> *** 1561,1566 ****
>> --- 1561,1567 ----
>> HANDLE restrictedToken;
>> SID_IDENTIFIER_AUTHORITY NtAuthority = {SECURITY_NT_AUTHORITY};
>> SID_AND_ATTRIBUTES dropSids[2];
>> + DWORD flags;
>>
>> /* Functions loaded dynamically */
>> __CreateRestrictedToken _CreateRestrictedToken = NULL;
>> *************** CreateRestrictedProcess(char *cmd, PROCE
>> *** 1636,1642 ****
>> AddUserToTokenDacl(restrictedToken);
>> #endif
>>
>> ! r = CreateProcessAsUser(restrictedToken, NULL, cmd, NULL, NULL, TRUE, CREATE_SUSPENDED, NULL, NULL, &si, processInfo);
>>
>> Kernel32Handle = LoadLibrary("KERNEL32.DLL");
>> if (Kernel32Handle != NULL)
>> --- 1637,1650 ----
>> AddUserToTokenDacl(restrictedToken);
>> #endif
>>
>> ! flags = CREATE_SUSPENDED;
>> !
>> ! /* Protect console process from Ctrl-C */
>> ! if (!as_service) {
>> ! flags |= CREATE_NEW_PROCESS_GROUP;
>> ! }
>> !
>> ! r = CreateProcessAsUser(restrictedToken, NULL, cmd, NULL, NULL, TRUE, flags, NULL, NULL, &si, processInfo);
>>
>> Kernel32Handle = LoadLibrary("KERNEL32.DLL");
>> if (Kernel32Handle != NULL)

Regards,
Hari Babu
Fujitsu Australia

On Fri, Apr 11, 2014 at 11:58:58AM +1000, Haribabu Kommi wrote:
> On Fri, Apr 11, 2014 at 7:44 AM, Bruce Momjian <bruce(at)momjian(dot)us> wrote:
> >
> > Can someone with Windows expertise comment on whether this should be
> > applied?
>
> I tested the same in windows and it is working as specified.
> The same background running server can be closed with ctrl+break command.

OK. If I apply this, are you able to test to see if the problem is
fixed?

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ Everyone has their own god. +

On Fri, Apr 11, 2014 at 12:12 PM, Bruce Momjian <bruce(at)momjian(dot)us> wrote:
> On Fri, Apr 11, 2014 at 11:58:58AM +1000, Haribabu Kommi wrote:
>> On Fri, Apr 11, 2014 at 7:44 AM, Bruce Momjian <bruce(at)momjian(dot)us> wrote:
>> >
>> > Can someone with Windows expertise comment on whether this should be
>> > applied?
>>
>> I tested the same in windows and it is working as specified.
>> The same background running server can be closed with ctrl+break command.
>
> OK. If I apply this, are you able to test to see if the problem is
> fixed?

I already tested in HEAD by applying the attached patch in the earlier mail.
with ctrl+c command the background process is not closed.
But with ctrl+break command the same can be closed.
if this behavior is fine, then we can apply patch.

Regards,
Hari Babu
Fujitsu Australia

On Fri, Apr 11, 2014 at 3:14 AM, Bruce Momjian <bruce(at)momjian(dot)us> wrote:
>
> Can someone with Windows expertise comment on whether this should be
> applied?

I don't think this is a complete fix, for example what about platform where
_CreateRestrictedToken() is not supported. For Example, the current
proposed fix will not work for below case:

if (_CreateRestrictedToken == NULL)
{
/*
* NT4 doesn't have CreateRestrictedToken, so just call ordinary
* CreateProcess
*/
write_stderr(_("%s: WARNING: cannot create restricted tokens on this
platform\n"), progname);
if (Advapi32Handle != NULL)
FreeLibrary(Advapi32Handle);
return CreateProcess(NULL, cmd, NULL, NULL, FALSE, 0, NULL, NULL, &si,
processInfo);
}

With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com

On 04/11/2014 01:35 AM, Amit Kapila wrote:
> On Fri, Apr 11, 2014 at 3:14 AM, Bruce Momjian <bruce(at)momjian(dot)us> wrote:
>> Can someone with Windows expertise comment on whether this should be
>> applied?
> I don't think this is a complete fix, for example what about platform where
> _CreateRestrictedToken() is not supported. For Example, the current
> proposed fix will not work for below case:
>
> if (_CreateRestrictedToken == NULL)
> {
> /*
> * NT4 doesn't have CreateRestrictedToken, so just call ordinary
> * CreateProcess
> */

Are we really supporting NT4 any more? Even XP is about to be at end of
support from Microsoft.

cheers

andrew

On 2014-04-11 07:12:50 -0400, Andrew Dunstan wrote:
>
> On 04/11/2014 01:35 AM, Amit Kapila wrote:
> >On Fri, Apr 11, 2014 at 3:14 AM, Bruce Momjian <bruce(at)momjian(dot)us> wrote:
> >>Can someone with Windows expertise comment on whether this should be
> >>applied?
> >I don't think this is a complete fix, for example what about platform where
> >_CreateRestrictedToken() is not supported. For Example, the current
> >proposed fix will not work for below case:
> >
> >if (_CreateRestrictedToken == NULL)
> >{
> >/*
> >* NT4 doesn't have CreateRestrictedToken, so just call ordinary
> >* CreateProcess
> >*/
>
>
> Are we really supporting NT4 any more? Even XP is about to be at end of
> support from Microsoft.

I seem to recall that win2k was already desupported?

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

On Fri, Apr 11, 2014 at 4:42 PM, Andrew Dunstan <andrew(at)dunslane(dot)net> wrote:
> On 04/11/2014 01:35 AM, Amit Kapila wrote:
>> I don't think this is a complete fix, for example what about platform
>> where
>> _CreateRestrictedToken() is not supported. For Example, the current
>> proposed fix will not work for below case:
>>
>> if (_CreateRestrictedToken == NULL)
>> {
>> /*
>> * NT4 doesn't have CreateRestrictedToken, so just call ordinary
>> * CreateProcess
>> */
> Are we really supporting NT4 any more? Even XP is about to be at end of
> support from Microsoft.

In Docs, it is mentioned as Windows (Win2000 SP4 and later).
Now what shall we do with this part of code, shall we keep it as it is and
just fix in other part of code or shall we remove this part of code?

Another thing to decide about this fix is that whether it is okay to fix it
for CTRL+C and leave the problem open for CTRL+BREAK?
(The current option used (CREATE_NEW_PROCESS_GROUP) will handle
only CTRL+C).

With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com