BUG #10330: pg_ctl does not correctly honor "DETACHED_PROCESS"

On Thu, May 15, 2014 at 03:46:20PM +0000, erica.stine@mailinator.com wrote:

The following bug has been logged on the website:

Bug reference: 10330
Logged by: Erica Stine
Email address: erica.stine@mailinator.com
PostgreSQL version: 9.3.4
Operating system: Windows 7 64-bit Ultimate
Description:

This executable:

https://github.com/cubiclesoft/createprocess-windows

Allows users to start processes in all sorts of ways that exceed the
limitations of the 'start' command by exposing the full power of the
CreateProcess() API to the command-line. The Apache web server, for
instance, has the problem of starting a hung command prompt using 'start'.
However, using the above tool and calling it with /f=DETACHED_PROCESS causes
'httpd.exe' to correctly and silently start in the background.

'pg_ctl.exe', on the other hand, does not honor DETACHED_PROCESS and
proceeds to create a new console window. There doesn't appear to be an
option to override this undesirable behavior. Trying to use 'postgres.exe'
directly with DETACTHED_PROCESS (and using the stdin, stdout, stderr
options) results in four brand new console windows. It looks like each
'postgres.exe' process gets its own console window.

The fix for this is to NOT create a console window if it doesn't exist and
pass DETACHED_PROCESS to new processes so that the OS isn't creating console
windows either based on executable type. I'm not alone in wanting a
detached startup. Those of us who want isolated installs want to run
PostgreSQL in a portable apps style format, behind the scenes, without using
the Windows Services host.

I know we work around it somehow in the script we use to start Postgres
as a Windows server. It would be nice to have these flags in the pg_ctl
executable, but none of our developers know how to write such code, or
at least I have not seen it.

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

+ Everyone has their own god. +

--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs

On Thu, May 15, 2014 at 03:46:20PM +0000,
erica(dot)stine(at)mailinator(dot)com wrote:

The following bug has been logged on the website:

Bug reference: 10330 Logged by: Erica Stine Email address:
erica(dot)stine(at)mailinator(dot)com PostgreSQL version: 9.3.4
Operating system: Windows 7 64-bit Ultimate Description:

This executable:

https://github.com/cubiclesoft/createprocess-windows

Allows users to start processes in all sorts of ways that exceed the
limitations of the 'start' command by exposing the full power of the
CreateProcess() API to the command-line. The Apache web server, for
instance, has the problem of starting a hung command prompt using
'start'. However, using the above tool and calling it with
/f=DETACHED_PROCESS causes 'httpd.exe' to correctly and silently
start in the background.

'pg_ctl.exe', on the other hand, does not honor DETACHED_PROCESS and
proceeds to create a new console window. There doesn't appear to be
an option to override this undesirable behavior. Trying to use
'postgres.exe' directly with DETACTHED_PROCESS (and using the stdin,
stdout, stderr options) results in four brand new console windows.
It looks like each 'postgres.exe' process gets its own console
window.

The fix for this is to NOT create a console window if it doesn't
exist and pass DETACHED_PROCESS to new processes so that the OS
isn't creating console windows either based on executable type. I'm
not alone in wanting a detached startup. Those of us who want
isolated installs want to run PostgreSQL in a portable apps style
format, behind the scenes, without usingthe Windows Services host.

Spent some time diagnosing the issue by digging around the source code.
Fixing 'pg_ctl.exe' is not something easily fixed, nor would it fix a
more severe problem in postmaster.

In backend/postmaster/postmaster.c, the internal_forkexec() function
attempts to emulate fork/exec. Since 'postgres.exe' is compiled as a
command-line application, the OS will create a console for it upon
startup if there isn't an attached console, unless the OS is told to do
otherwise.

Somewhere around line 4227 is this code:

if (!CreateProcess(NULL, cmdLine, NULL, NULL, TRUE, CREATE_SUSPENDED,
NULL, NULL, &si, &pi))

Adding DETACHED_PROCESS should fix the problem of the OS spawning a
bunch of console windows when 'postgres.exe' is called directly without
an attached console:

if (!CreateProcess(NULL, cmdLine, NULL, NULL, TRUE, CREATE_SUSPENDED |
DETACHED_PROCESS, NULL, NULL, &si, &pi))

That is only half of a fix though. 'pg_ctl.exe' still has to be patched
up. However, the fix for 'pg_ctl.exe' is much more extensive because
someone decided it would be a fantastic idea to call 'cmd.exe' probably
because they were too lazy to write some simple HANDLE code. The reason
extra console windows don't currently show up with the current method is
because Windows is smart enough to attach multiple processes to the
single console window that the 'cmd.exe' process creates upon startup.
The first step of the fix for 'pg_ctl.exe' is probably to change the
prototype of CreateRestrictedProcess() from:

static int
CreateRestrictedProcess(char *cmd, PROCESS_INFORMATION *processInfo,
bool as_service)

To:

static int
CreateRestrictedProcess(char *cmd, PROCESS_INFORMATION *processInfo,
STARTUPINFO *si, bool as_service)

Then, in start_postmaster(), replace the existing code with something like:

#else /* WIN32 */

/*
* On win32 we don't use system().
*/
PROCESS_INFORMATION pi;
STARTUPINFO si;
LARGE_INTEGER filepos = {0};

ZeroMemory(&si, sizeof(si));
si.cb = sizeof(si);
si.hStdInput = NULL;
if (log_file != NULL)
{
si.hStdOutput = CreateFile(log_file, GENERIC_WRITE, FILE_SHARE_READ |
FILE_SHARE_WRITE, NULL, OPEN_ALWAYS, FILE_ATTRIBUTE_NORMAL, NULL);
if (si.hStdOutput != INVALID_HANDLE_VALUE)
{
::SetFilePointerEx(si.hStdOutput, filepos, NULL, FILE_END);
}
else
{
si.hStdOutput = NULL;
}
}
else
{
si.hStdOutput = GetStdHandle(STD_OUTPUT_HANDLE);
}
si.hStdError = si.hStdOutput;
si.dwFlags |= STARTF_USESTDHANDLES;

snprintf(cmd, MAXPGPATH, "\"%s\" %s%s",
exec_path, pgdata_opt, post_opts);

if (!CreateRestrictedProcess(cmd, &pi, &si, false))
return GetLastError();

if (log_file != NULL && si.hStdOutput != NULL) CloseHandle(si.hStdOutput);
CloseHandle(pi.hProcess);
CloseHandle(pi.hThread);
return 0;

In CreateRestrictedProcess(), change:

return CreateProcess(NULL, cmd, NULL, NULL, FALSE, 0, NULL, NULL, &si,
processInfo);

To:

return CreateProcess(NULL, cmd, NULL, NULL, TRUE, 0, NULL, NULL, &si,
processInfo);

And, of course, remove the 'STARTUPINFO' structure logic from
CreateRestrictedProcess() since that's now passed into the function.
There's also a reference in pgwin32_ServiceMain() to
CreateRestrictedProcess(). Change:

if (!CreateRestrictedProcess(pgwin32_CommandLine(false), &pi, true))

To:

STARTUPINFO si;

...

ZeroMemory(&si, sizeof(si));
si.cb = sizeof(si);

...

if (!CreateRestrictedProcess(pgwin32_CommandLine(false), &pi, &si, true))

That should do the trick to eliminate 'cmd.exe' from the picture, which
eliminates one unnecessary (and undesirable) dependency. 'pg_ctl.exe'
will still not be completely free of CreateProcess() related problems,
but the 'createprocess.exe' tool can at least begin to overcome some of
the issues that remain.

--
Thomas Hruska
CubicleSoft President

I've got great, time saving software that you will find useful.

http://cubiclesoft.com/

--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs

On 5/15/2014 9:26 PM, Thomas Hruska wrote:

Somewhere around line 4227 is this code:

if (!CreateProcess(NULL, cmdLine, NULL, NULL, TRUE,
CREATE_SUSPENDED, NULL, NULL, &si, &pi))

Adding DETACHED_PROCESS should fix the problem of the OS spawning a
bunch of console windows when 'postgres.exe' is called directly without
an attached console:

if (!CreateProcess(NULL, cmdLine, NULL, NULL, TRUE,
CREATE_SUSPENDED | DETACHED_PROCESS, NULL, NULL, &si, &pi))

Minor correction. Detect the lack of a console, THEN add DETACHED_PROCESS:

if (!CreateProcess(NULL, cmdLine, NULL, NULL, TRUE, CREATE_SUSPENDED |
(GetConsoleWindow() == NULL ? DETACHED_PROCESS : 0), NULL, NULL, &si, &pi))

That should allow standard input/output handles to function as expected
when there is a console.

--
Thomas Hruska
CubicleSoft President

I've got great, time saving software that you will find useful.

http://cubiclesoft.com/

--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs

On Thu, May 15, 2014 at 09:42:27PM -0700, Thomas Hruska wrote:

On 5/15/2014 9:26 PM, Thomas Hruska wrote:

Somewhere around line 4227 is this code:

if (!CreateProcess(NULL, cmdLine, NULL, NULL, TRUE,
CREATE_SUSPENDED, NULL, NULL, &si, &pi))

Adding DETACHED_PROCESS should fix the problem of the OS spawning a
bunch of console windows when 'postgres.exe' is called directly without
an attached console:

if (!CreateProcess(NULL, cmdLine, NULL, NULL, TRUE,
CREATE_SUSPENDED | DETACHED_PROCESS, NULL, NULL, &si, &pi))

Minor correction. Detect the lack of a console, THEN add DETACHED_PROCESS:

if (!CreateProcess(NULL, cmdLine, NULL, NULL, TRUE,
CREATE_SUSPENDED | (GetConsoleWindow() == NULL ? DETACHED_PROCESS :
0), NULL, NULL, &si, &pi))

That should allow standard input/output handles to function as
expected when there is a console.

Thomas, would you like to create a complete patch for this that we could
review?

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

+ Everyone has their own god. +

--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs