Subject: Re: BUG #16161: pg_ctl stop fails sometimes (on Windows)
To: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Cc: tgl@sss.pgh.pa.us, pgsql-bugs@lists.postgresql.org
References: <5c7eaa87-8670-2008-3a15-337c133c12c2@gmail.com>
 <12599.1576786185@sss.pgh.pa.us>
 <f6b03b6e-59b7-1b99-0e8f-d42d72e35917@gmail.com>
 <20191220.123237.432497214732463092.horikyota.ntt@gmail.com>
From: Alexander Lakhin <exclusion@gmail.com>
Message-ID: <37483c7c-a202-fd12-7211-42aeb83bcfd7@gmail.com>
Date: Fri, 20 Dec 2019 07:40:00 +0300
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101
 Thunderbird/60.7.2
MIME-Version: 1.0
In-Reply-To: <20191220.123237.432497214732463092.horikyota.ntt@gmail.com>
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 8bit
Content-Language: en-US
Precedence: bulk

20.12.2019 6:32, Kyotaro Horiguchi wrote:
> At Fri, 20 Dec 2019 05:40:00 +0300, Alexander Lakhin <exclusion@gmail.com> wrote in 
>> 19.12.2019 23:09, Tom Lane wrote:
>>> Alexander Lakhin <exclusion@gmail.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