Subject: Re: BUG #16154: pg_ctl restart with a logfile fails sometimes (on
 Windows)
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: pgsql-bugs@lists.postgresql.org
References: <16154-1ccf0b537b24d5e0@postgresql.org>
 <e5179494-715e-f8a3-266b-0cf52adac8f4@gmail.com>
 <6120.1575645256@sss.pgh.pa.us>
From: Alexander Lakhin <exclusion@gmail.com>
Message-ID: <e8866e38-175b-b22a-1831-45cfda6494b7@gmail.com>
Date: Fri, 6 Dec 2019 18:40:12 +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: <6120.1575645256@sss.pgh.pa.us>
Content-Type: multipart/mixed;
 boundary="------------7A79CF79C8472D11EAA8C2F6"
Content-Language: ru-RU
Precedence: bulk

This is a multi-part message in MIME format.
--------------7A79CF79C8472D11EAA8C2F6
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 8bit

06.12.2019 18:14, Tom Lane wrote:
> Alexander Lakhin <exclusion@gmail.com> writes:
>> If this file is still opened by the previous server shell (it can happen
>> when the previous server instance has unlinked it's pid file, but it's
>> CMD shell is still running), the next CMD start fails with the
>> aforementioned error message.
> Interesting.  I wonder whether this explains all of the remaining
> buildfarm failures of this sort that we've been seeing even after
> 0ba06e0bf.
>
>> To fix this issue I propose the attached patch
>> (fix_logfile_sharing_violation ).
> This seems like a pretty ugly hack ... please at least make it
> #ifdef WIN32, so that the rest of us don't have to deal with it.
> Also, if I read it correctly, it causes a pre-existing logfile
> to get truncated, which has never happened before.  Mode "a"
> would be a better choice.
This change should go under the following code:
#else                            /* WIN32 */

    /*
     * As with the Unix case, it's easiest to use the shell (CMD.EXE) to
     * handle redirection etc.  Unfortunately CMD.EXE lacks any
equivalent of
     * "exec", so we don't get to find out the postmaster's PID immediately.
     */
    PROCESS_INFORMATION pi;
    const char *comspec;

    /* Find CMD.EXE location using COMSPEC, if it's set */
    comspec = getenv("COMSPEC");
    if (comspec == NULL)
        comspec = "CMD";

So it should affect only Windows.
I've fixed the mode. Thanks for your review!

Best regards,
Alexander


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

diff --git a/src/bin/pg_ctl/pg_ctl.c b/src/bin/pg_ctl/pg_ctl.c
index 65f9fb4c0a..9bc44b2de4 100644
--- a/src/bin/pg_ctl/pg_ctl.c
+++ b/src/bin/pg_ctl/pg_ctl.c
@@ -519,8 +519,21 @@ start_postmaster(void)
 		comspec = "CMD";
 
 	if (log_file != NULL)
+	{
+		/* Check the log file availability to prevent CMD.EXE failing with
+		 * ERROR_SHARING_VIOLATION, e.g. when the log file is still opened
+		 * by the previous server shell.
+		 */
+		FILE *fd = fopen(log_file, "a");
+		if (!fd) {
+			write_stderr(_("%s: could not access log file \"%s\": %s\n"),
+						 progname, log_file, strerror(errno));
+			exit(1);
+		}
+		fclose(fd);
 		snprintf(cmd, MAXPGPATH, "\"%s\" /C \"\"%s\" %s%s < \"%s\" >> \"%s\" 2>&1\"",
 				 comspec, exec_path, pgdata_opt, post_opts, DEVNULL, log_file);
+	}
 	else
 		snprintf(cmd, MAXPGPATH, "\"%s\" /C \"\"%s\" %s%s < \"%s\" 2>&1\"",
 				 comspec, exec_path, pgdata_opt, post_opts, DEVNULL);

--------------7A79CF79C8472D11EAA8C2F6--