Обсуждение: BUG #10330: pg_ctl does not correctly honor "DETACHED_PROCESS"

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.

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. +

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/

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/

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. +