Re: BUG #16259: Cannot Use "pg_ctl start -l logfile" on Clean Install on Windows Server 2012/2016

Hello Tom,
15.02.2020 00:46, Tom Lane wrote:
> Heath Lord <heath.lord@crunchydata.com> writes:
>>    Another interesting thing that we have found is that this issue
>> only manifests
>> itself if you are trying to launch the pg_ctl command as a privileged
>> user.  If you
>> try to run this command as a user who is not an Administrator, then we do not
>> see this issue.  This is another reason why Dory does not exhibit this behavior.
>>   With this finding, it has to be something with how pg_ctl is restricting the
>> user when running the postgres.exe command from the CMD.exe process,
>> so we are not trying to launch postgres as a privileged user.
> OOOHHH ... I think the light just went on, then.  The reason we have
> an issue is that we drop admin privileges (via CreateRestrictedProcess)
> when we launch CMD.EXE, just below this.  So we are still admin when
> the new code touches the log file, and that's why it's getting created
> with different privileges from files that the postmaster (or CMD.EXE)
> would create.
I can easily reproduce this by opening "x64 Native Tools Command Prompt
for VS 2019" with "Run as Administrator". When executing `vcregress
taptest src/bin/pg_basebackup` in this command prompt, I get:
t/010_pg_basebackup.pl ... 1/106 Bailout called.  Further testing
stopped:  pg_ctl start failed
FAILED--Further testing stopped: pg_ctl start failed
> The idea I'd had for a fix is that we don't need to actually create the
> file if it's not there yet.  All we need is to delay if it's there and
> can't be opened.  So rather than open for writing, I think we could open
> for reading, along the lines of
>
>         FILE       *fd = fopen(log_file, "r");
>
>         if (fd != NULL)     /* we may just ignore any error */
>             fclose(fd);
>
> The looping logic in pgwin32_open doesn't really care which kind
> of access we're asking for.
With the proposed change and the delay_after_unlink_pid.patch
<https://www.postgresql.org/message-id/attachment/105988/delay_after_unlink_pid.patch>
[1] applied I get the same fail on `vcregress taptest
src/bin/pg_basebackup` as before 0da33c76:
waiting for server to shut down.... done
server stopped
waiting for server to start....The process cannot access the file
because it is being used by another process.
 stopped waiting
pg_ctl: could not start server

So just the read permission is not sufficient for that check.
> BTW, we could make this at least slightly cheaper by using
> open() not fopen().
>
> Alexander, this was your patch to start with ... any thoughts?
Yes, it was my omission. Please look at the improved patch.
It works for me in both command prompts and still gives a meaningful
message on error.
 
In fact running taptests (pg_basebackup, pg_ctl) in an elevated prompt
fails anyway, but later and with other errors, e.g.:

t/001_start_stop.pl .. ok
t/002_status.pl ...... ok
t/003_promote.pl ..... 5/12 Bailout called.  Further testing stopped: 
pg_ctl start failed
FAILED--Further testing stopped: pg_ctl start failed
...
pg_ctl: could not start server
Examine the log output.
# pg_ctl start failed; logfile:
2020-02-15 07:16:05.588 MSK [1784] PANIC:  could not open file
"global/pg_control": Permission denied
Bail out!  pg_ctl start failed

But I think it's not related to this bug.

[1]
https://www.postgresql.org/message-id/e5179494-715e-f8a3-266b-0cf52adac8f4%40gmail.com

Best regards,
Alexander