Re: Out-of-memory error reports in libpq

[Tom Lane]

I wrote:
> Andres Freund <and...@anarazel.de> writes:
>> Hm. It seems we should be able to guarantee that the recovery path can print
>> something, at least in the PGconn case. Is it perhaps worth pre-sizing
>> PGConn->errorMessage so it'd fit an error like this?
>> But perhaps that's more effort than it's worth.

> Yeah.  I considered changing things so that oom_buffer contains
> "out of memory\n" rather than an empty string, but I'm afraid
> that that's making unsupportable assumptions about what PQExpBuffers
> are used for.

Actually, wait a minute.  There are only a couple of places that ever
read out the value of conn->errorMessage, so let's make those places
responsible for dealing with OOM scenarios.  That leads to a nicely
small patch, as attached, and it looks to me like it makes us quite
bulletproof against such scenarios.

It might still be worth doing the "pqReportOOM" changes to save a
few bytes of code space, but I'm less excited about that now.

                        regards, tom lane


diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index e950b41374..49eec3e835 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -6739,6 +6739,14 @@ PQerrorMessage(const PGconn *conn)
 	if (!conn)
 		return libpq_gettext("connection pointer is NULL\n");
 
+	/*
+	 * The errorMessage buffer might be marked "broken" due to having
+	 * previously failed to allocate enough memory for the message.  In that
+	 * case, tell the application we ran out of memory.
+	 */
+	if (PQExpBufferBroken(&conn->errorMessage))
+		return libpq_gettext("out of memory\n");
+
 	return conn->errorMessage.data;
 }
 
diff --git a/src/interfaces/libpq/fe-exec.c b/src/interfaces/libpq/fe-exec.c
index aca81890bb..87f348f3dc 100644
--- a/src/interfaces/libpq/fe-exec.c
+++ b/src/interfaces/libpq/fe-exec.c
@@ -191,7 +191,7 @@ PQmakeEmptyPGresult(PGconn *conn, ExecStatusType status)
 				/* non-error cases */
 				break;
 			default:
-				pqSetResultError(result, conn->errorMessage.data);
+				pqSetResultError(result, &conn->errorMessage);
 				break;
 		}
 
@@ -662,14 +662,28 @@ pqResultStrdup(PGresult *res, const char *str)
  *		assign a new error message to a PGresult
  */
 void
-pqSetResultError(PGresult *res, const char *msg)
+pqSetResultError(PGresult *res, PQExpBuffer errorMessage)
 {
+	char	   *msg;
+
 	if (!res)
 		return;
-	if (msg && *msg)
-		res->errMsg = pqResultStrdup(res, msg);
+
+	/*
+	 * We handle two OOM scenarios here.  The errorMessage buffer might be
+	 * marked "broken" due to having previously failed to allocate enough
+	 * memory for the message, or it might be fine but pqResultStrdup fails
+	 * and returns NULL.  In either case, just make res->errMsg point directly
+	 * at a constant "out of memory" string.
+	 */
+	if (!PQExpBufferBroken(errorMessage))
+		msg = pqResultStrdup(res, errorMessage->data);
+	else
+		msg = NULL;
+	if (msg)
+		res->errMsg = msg;
 	else
-		res->errMsg = NULL;
+		res->errMsg = libpq_gettext("out of memory\n");
 }
 
 /*
@@ -2122,7 +2136,7 @@ PQgetResult(PGconn *conn)
 				appendPQExpBuffer(&conn->errorMessage,
 								  libpq_gettext("PGEventProc \"%s\" failed during PGEVT_RESULTCREATE event\n"),
 								  res->events[i].name);
-				pqSetResultError(res, conn->errorMessage.data);
+				pqSetResultError(res, &conn->errorMessage);
 				res->resultStatus = PGRES_FATAL_ERROR;
 				break;
 			}
diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h
index e9f214b61b..490458adef 100644
--- a/src/interfaces/libpq/libpq-int.h
+++ b/src/interfaces/libpq/libpq-int.h
@@ -637,7 +637,7 @@ extern pgthreadlock_t pg_g_threadlock;
 
 /* === in fe-exec.c === */
 
-extern void pqSetResultError(PGresult *res, const char *msg);
+extern void pqSetResultError(PGresult *res, PQExpBuffer errorMessage);
 extern void *pqResultAlloc(PGresult *res, size_t nBytes, bool isBinary);
 extern char *pqResultStrdup(PGresult *res, const char *str);
 extern void pqClearAsyncResult(PGconn *conn);