Re: BUG #16161: pg_ctl stop fails sometimes (on Windows)

20.12.2019 6:32, Kyotaro Horiguchi wrote:
> At Fri, 20 Dec 2019 05:40:00 +0300, Alexander Lakhin <exclusion(at)gmail(dot)com> wrote in
>> 19.12.2019 23:09, Tom Lane wrote:
>>> Alexander Lakhin <exclusion(at)gmail(dot)com> writes:
>>>> Maybe we should change the condition to 'if (stat(fileName, &st) != 0 &&
>>>> (err = GetLastError()) == ERROR_ACCESS_DENIED)' to avoid unnecessary
>>>> sleep with a loop iteration...
>>> Well, we have to loop back on file-not-found too ...
>> I think, if the file is not found on stat() we can safely return the
>> updated err (set errno with _dosmapper(err), to be exact). If we'll loop
>> back, we'll get the same err = GetLastError() after next CreateFile().
> I think Tom is mentinoing for O_CREAT case. If CreateFile returned
> ERROR_ACCESS_DENIED, then stat() returns ERROR_FILE_NOT_FOUND, it
> means the next CreateFile can success.
It seems this is a new corner case and we just failed in this case
previously (yes, with ERROR_ACCESS_DENIED instead of
ERROR_FILE_NOT_FOUND, but nonetheless). If we restrict ourselves to
solving the initial problem with 'pt_ctl stop' (and pg_ctl opens pidfile
without O_CREAT), I wouldn't develop more powerful open() now.
But if we want it, the following code passes my tests (restart &
recoverycheck) too:
                if (stat(fileName, &st) != 0 &&
                    ((err = GetLastError()) == ERROR_ACCESS_DENIED) ||
                     ((err == ERROR_FILE_NOT_FOUND) && (fileFlags &
O_CREAT)))
                {
                    pg_usleep(100000);
                    loops++;
                    continue;
                }
Maybe we still need a practical case where it's useful, to test it for sure.

Best regards,
Alexander