Re: BUG #10330: pg_ctl does not correctly honor "DETACHED_PROCESS" - Mailing list 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/