Subject: Re: BUG #16161: pg_ctl stop fails sometimes (on Windows)
To: pgsql-bugs@lists.postgresql.org,
 PG Bug reporting form <noreply@postgresql.org>
References: <16161-7a985d2f1bbe8f71@postgresql.org>
From: Alexander Lakhin <exclusion@gmail.com>
Message-ID: <4d28c7e1-fe91-b9fd-3227-478e549b05d5@gmail.com>
Date: Wed, 11 Dec 2019 23:30:01 +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: <16161-7a985d2f1bbe8f71@postgresql.org>
Content-Type: multipart/mixed;
 boundary="------------5E32B2FC4D694F266C93B710"
Content-Language: en-US
Precedence: bulk

This is a multi-part message in MIME format.
--------------5E32B2FC4D694F266C93B710
Content-Type: multipart/alternative;
 boundary="------------2036BFDF6F37579CBDDBF5C4"


--------------2036BFDF6F37579CBDDBF5C4
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit

11.12.2019 23:00, PG Bug reporting form wrote:
> The following bug has been logged on the website:
>
> Bug reference:      16161
> Logged by:          Alexander Lakhin
> Email address:      exclusion@gmail.com
> PostgreSQL version: 12.1
> Operating system:   Windows
> Description:        
>
> The regression tests on Windows sometimes fail with 'Permission denied'
> errors. For example:
> https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=dory&dt=2019-12-11%2007%3A45%3A33
>
> ============== shutting down postmaster               ==============
> pg_ctl: could not open PID file
> "c:/pgbuildfarm/pgbuildroot/HEAD/pgsql.build/src/test/regress/./tmp_check/data/postmaster.pid":
> Permission denied
>
> This error occurs when pg_ctl is trying to open postmaster.pid while this
> file is in "delete pending" state
> (https://stackoverflow.com/questions/3764072/).
To reproduce the issue reliably I propose a simple modification to
synchronize unlink() with open() and a simple test
(sync_pid_ops+test.patch).
With the patch applied, `vcregress taptest src/test/restart` fails for
me on iteration 47, 6, 7, 42, 51, 26, ...

I see two ways to fix the issue:
1) Unlink postmaster.pid using rename operation (adopt the solution from
https://stackoverflow.com/questions/3764072/).
2) Ignore such 'Permission denied' error and just try to open the file
once again (attached fix_open_for_unlink.patch implements this).


I'm inclined to the second approach as pgwin32_open() already handles
two transient states (Windows-only), and it could be useful not only for
postmaster.pid, but for some other files.


Best regards,
Alexander


--------------2036BFDF6F37579CBDDBF5C4
Content-Type: text/html; charset=utf-8
Content-Transfer-Encoding: 7bit

<html>
  <head>
    <meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
  </head>
  <body text="#000000" bgcolor="#FFFFFF">
    <div class="moz-cite-prefix">11.12.2019 23:00, PG Bug reporting form
      wrote:<br>
    </div>
    <blockquote type="cite"
      cite="mid:%3C16161-7a985d2f1bbe8f71@postgresql.org%3E">
      <pre class="moz-quote-pre" wrap="">The following bug has been logged on the website:

Bug reference:      16161
Logged by:          Alexander Lakhin
Email address:      <a class="moz-txt-link-abbreviated" href="mailto:exclusion@gmail.com">exclusion@gmail.com</a>
PostgreSQL version: 12.1
Operating system:   Windows
Description:        

The regression tests on Windows sometimes fail with 'Permission denied'
errors. For example:
<a class="moz-txt-link-freetext" href="https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=dory&amp;dt=2019-12-11%2007%3A45%3A33">https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=dory&amp;dt=2019-12-11%2007%3A45%3A33</a>

============== shutting down postmaster               ==============
pg_ctl: could not open PID file
"c:/pgbuildfarm/pgbuildroot/HEAD/pgsql.build/src/test/regress/./tmp_check/data/postmaster.pid":
Permission denied

This error occurs when pg_ctl is trying to open postmaster.pid while this
file is in "delete pending" state
(<a class="moz-txt-link-freetext" href="https://stackoverflow.com/questions/3764072/">https://stackoverflow.com/questions/3764072/</a>).
</pre>
    </blockquote>
    To reproduce the issue reliably I propose a simple modification to
    synchronize unlink() with open() and a simple test
    (sync_pid_ops+test.patch).<br>
    With the patch applied, `vcregress taptest src/test/restart` fails
    for me on iteration 47, 6, 7, 42, 51, 26, ...<br>
    <br>
    I see two ways to fix the issue:<br>
    1) Unlink postmaster.pid using rename operation (adopt the solution
    from <a class="moz-txt-link-freetext" href="https://stackoverflow.com/questions/3764072/">https://stackoverflow.com/questions/3764072/</a>).<br>
    2) Ignore such 'Permission denied' error and just try to open the
    file once again (attached fix_open_for_unlink.patch implements
    this).
    <p style=" margin-top:0px; margin-bottom:0px; margin-left:0px;
      margin-right:0px; -qt-block-indent:0; text-indent:0px;"><br>
    </p>
    <p style=" margin-top:0px; margin-bottom:0px; margin-left:0px;
      margin-right:0px; -qt-block-indent:0; text-indent:0px;">I'm
      inclined to the second approach as pgwin32_open() already handles
      two transient states (Windows-only), and it could be useful not
      only for postmaster.pid, but for some other files.</p>
    <p style=" margin-top:0px; margin-bottom:0px; margin-left:0px;
      margin-right:0px; -qt-block-indent:0; text-indent:0px;"><br>
    </p>
    Best regards,<br>
    Alexander<br>
    <br>
  </body>
</html>

--------------2036BFDF6F37579CBDDBF5C4--

--------------5E32B2FC4D694F266C93B710
Content-Type: text/x-patch;
 name="sync_pid_ops+test.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
 filename="sync_pid_ops+test.patch"

diff --git a/src/backend/utils/init/miscinit.c b/src/backend/utils/init/miscinit.c
index de554e28cf..0594b6649f 100644
--- a/src/backend/utils/init/miscinit.c
+++ b/src/backend/utils/init/miscinit.c
@@ -838,6 +838,13 @@ UnlinkLockFiles(int status, Datum arg)
 {
 	ListCell   *l;
 
+#ifdef WIN32
+{
+FILETIME time;
+do { GetSystemTimePreciseAsFileTime(&time); }
+while (((((ULONGLONG) time.dwHighDateTime) << 32) + time.dwLowDateTime) % 10000000L > (10000L));
+}
+#endif
 	foreach(l, lock_files)
 	{
 		char	   *curfile = (char *) lfirst(l);
diff --git a/src/bin/pg_ctl/pg_ctl.c b/src/bin/pg_ctl/pg_ctl.c
index b0858411ed..3ab1608a5d 100644
--- a/src/bin/pg_ctl/pg_ctl.c
+++ b/src/bin/pg_ctl/pg_ctl.c
@@ -276,6 +276,14 @@ get_pgpid(bool is_status_request)
 		exit(is_status_request ? 4 : 1);
 	}
 
+#ifdef WIN32
+{
+FILETIME time;
+do { GetSystemTimePreciseAsFileTime(&time); }
+while (((((ULONGLONG) time.dwHighDateTime) << 32) + time.dwLowDateTime) % 10000000L > (10000L));
+}
+#endif
+
 	pidf = fopen(pid_file, "r");
 	if (pidf == NULL)
 	{
diff --git a/src/test/restart/t/001_restart.pl b/src/test/restart/t/001_restart.pl
new file mode 100644
index 0000000000..f0621d7f27
--- /dev/null
+++ b/src/test/restart/t/001_restart.pl
@@ -0,0 +1,20 @@
+use strict;
+use warnings;
+use PostgresNode;
+use TestLib;
+use Test::More;
+use Config;
+
+my $node = get_new_node('master');
+$node->init();
+
+for ( my $i = 1; $i <= 1000; $i++ ) {
+	my ($sec,$min,$hour,$mday,$mon,$year,$wday,$yday,$isdst) = localtime();
+	diag( sprintf("%02d:%02d:%02d iteration %d", $hour, $min, $sec, $i) );
+	$node->start;
+	$node->stop;
+}
+
+ok(1, 'node start/stop test');
+
+done_testing();

--------------5E32B2FC4D694F266C93B710
Content-Type: text/x-patch;
 name="fix_open_for_unlink.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
 filename="fix_open_for_unlink.patch"

diff --git a/src/port/open.c b/src/port/open.c
index f37afc7512..dad328f000 100644
--- a/src/port/open.c
+++ b/src/port/open.c
@@ -135,6 +135,21 @@ pgwin32_open(const char *fileName, int fileFlags,...)
 				continue;
 		}
 
+		/*
+		 * ERROR_ACCESS_DENIED can be returned while the file is deleted
+		 * (Windows NT status code is STATUS_DELETE_PENDING).
+		 * https://stackoverflow.com/questions/3764072/
+		 * Try again for 1 second to check whether the "access denied"
+		 * condition persists.
+		 */
+		if (err == ERROR_ACCESS_DENIED) {
+			if (loops < 10) {
+				pg_usleep(100000);
+				loops++;
+				continue;
+			}
+		}
+
 		_dosmaperr(err);
 		return -1;
 	}

--------------5E32B2FC4D694F266C93B710--