From: Tom Lane <tgl@sss.pgh.pa.us>
To: =?UTF-8?Q?Juan_Jos=C3=A9_Santamar=C3=ADa_Flecha?=
 <juanjo.santamaria@gmail.com>
cc: egashira.yusuke@fujitsu.com,
        PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
Subject: Re: BUG #17254: Crash with 0xC0000409 in pg_stat_statements when
 pg_stat_tmp\pgss_query_texts.stat exceeded 2GB.
In-reply-to: <856857.1635627048@sss.pgh.pa.us>
References: <17254-a926c89dc03375c2@postgresql.org>
 <CAC+AXB21yF41p4fWgUOkG1PBA-Ezw-8U0Q=Bint9OQ56qyMPYw@mail.gmail.com>
 <CAC+AXB0fozNPMgRTP+PgFMrGcSn2zER0zgcrEbRV0NvpZqmE=A@mail.gmail.com>
 <CAC+AXB2tWxoZgj6BnoQaxiWw=vKBxUPtyBn7d8jhHRqxv7xRRw@mail.gmail.com>
 <840436.1635611203@sss.pgh.pa.us>
 <CAC+AXB2vCUOj6x8drBVUVCgjyy4ZFUu-TFBuNVW=7Emh44EC=w@mail.gmail.com>
 <856857.1635627048@sss.pgh.pa.us>
Comments: In-reply-to Tom Lane <tgl@sss.pgh.pa.us>
	message dated "Sat, 30 Oct 2021 16:50:48 -0400"
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="----- =_aaaaaaaaaa0"
Content-ID: <975781.1635700038.0@sss.pgh.pa.us>
Date: Sun, 31 Oct 2021 13:07:42 -0400
Message-ID: <975808.1635700062@sss.pgh.pa.us>
Archived-At: 
 <https://www.postgresql.org/message-id/975808.1635700062%40sss.pgh.pa.us>
Precedence: bulk

------- =_aaaaaaaaaa0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <975781.1635700038.1@sss.pgh.pa.us>
Content-Transfer-Encoding: quoted-printable

I wrote:
> =3D?UTF-8?Q?Juan_Jos=3DC3=3DA9_Santamar=3DC3=3DADa_Flecha?=3D <juanjo.sa=
ntamaria@gmail.com> writes:
>> On Sat, Oct 30, 2021 at 6:26 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>> I think instead, we need to turn the subsequent one-off read() call in=
to a
>>> loop that reads no more than INT_MAX bytes at a time.  It'd be possibl=
e
>>> to restrict that to Windows, but probably no harm in doing it the same
>>> way everywhere.

>> Seems reasonable to me, can such a change be back-patched?

> Don't see why not.

Here's a quick patch for that.  I don't have any ability to check it
on Windows, but the logic is easy to verify by reducing the arbitrary
constant to something small.  (I used 1GB, not INT_MAX, because I figured
we ought to read in multiples of a filesystem block if possible.)

			regards, tom lane


------- =_aaaaaaaaaa0
Content-Type: text/x-diff; name="avoid-windows-failure-on-huge-read.patch";
	charset="us-ascii"
Content-ID: <975781.1635700038.2@sss.pgh.pa.us>
Content-Description: avoid-windows-failure-on-huge-read.patch
Content-Transfer-Encoding: quoted-printable

diff --git a/contrib/pg_stat_statements/pg_stat_statements.c b/contrib/pg_=
stat_statements/pg_stat_statements.c
index 07fe0e7cda..726ba59e2b 100644
--- a/contrib/pg_stat_statements/pg_stat_statements.c
+++ b/contrib/pg_stat_statements/pg_stat_statements.c
@@ -2125,6 +2125,7 @@ qtext_load_file(Size *buffer_size)
 	char	   *buf;
 	int			fd;
 	struct stat stat;
+	Size		nread;
 =

 	fd =3D OpenTransientFile(PGSS_TEXT_FILE, O_RDONLY | PG_BINARY);
 	if (fd < 0)
@@ -2165,23 +2166,35 @@ qtext_load_file(Size *buffer_size)
 	}
 =

 	/*
-	 * OK, slurp in the file.  If we get a short read and errno doesn't get
-	 * set, the reason is probably that garbage collection truncated the fil=
e
-	 * since we did the fstat(), so we don't log a complaint --- but we don'=
t
-	 * return the data, either, since it's most likely corrupt due to
-	 * concurrent writes from garbage collection.
+	 * OK, slurp in the file.  Windows fails if we try to read more than
+	 * INT_MAX bytes at once, and other platforms might not like that either=
,
+	 * so read a very large file in 1GB segments.
 	 */
-	errno =3D 0;
-	if (read(fd, buf, stat.st_size) !=3D stat.st_size)
+	nread =3D 0;
+	while (nread < stat.st_size)
 	{
-		if (errno)
-			ereport(LOG,
-					(errcode_for_file_access(),
-					 errmsg("could not read file \"%s\": %m",
-							PGSS_TEXT_FILE)));
-		free(buf);
-		CloseTransientFile(fd);
-		return NULL;
+		int			toread =3D Min(1024 * 1024 * 1024, stat.st_size - nread);
+
+		/*
+		 * If we get a short read and errno doesn't get set, the reason is
+		 * probably that garbage collection truncated the file since we did
+		 * the fstat(), so we don't log a complaint --- but we don't return
+		 * the data, either, since it's most likely corrupt due to concurrent
+		 * writes from garbage collection.
+		 */
+		errno =3D 0;
+		if (read(fd, buf + nread, toread) !=3D toread)
+		{
+			if (errno)
+				ereport(LOG,
+						(errcode_for_file_access(),
+						 errmsg("could not read file \"%s\": %m",
+								PGSS_TEXT_FILE)));
+			free(buf);
+			CloseTransientFile(fd);
+			return NULL;
+		}
+		nread +=3D toread;
 	}
 =

 	if (CloseTransientFile(fd) !=3D 0)
@@ -2189,7 +2202,7 @@ qtext_load_file(Size *buffer_size)
 				(errcode_for_file_access(),
 				 errmsg("could not close file \"%s\": %m", PGSS_TEXT_FILE)));
 =

-	*buffer_size =3D stat.st_size;
+	*buffer_size =3D nread;
 	return buf;
 }
 =


------- =_aaaaaaaaaa0--