Re: Out-of-memory error reports in libpq - Mailing list pgsql-hackers

I wrote:
> Andres Freund <andres@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);