From d014b38df60fe7f486bfaeb9b55a4851bf32c86f Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Fri, 13 Apr 2018 12:53:45 -0400 Subject: [PATCH] In libpq, free any partial query result before collecting a server error. We'd throw away the partial result anyway after parsing the error message. Throwing it away beforehand costs nothing and reduces the risk of out-of-memory failure. Also, at least in systems that behave like glibc/Linux, if the partial result was very large then the error PGresult would get allocated at high heap addresses, preventing the heap storage used by the partial result from being released to the OS until the error PGresult is freed. In psql >= 9.6, we hold onto the error PGresult until another error is received (for \errverbose), so that this behavior causes a seeming memory leak to persist for awhile, as in a recent complaint from Darafei Praliaskouski. This is a potential performance regression from older versions, justifying back-patching at least that far. But similar behavior may occur in other client applications, so it seems worth just back-patching to all supported branches. Discussion: https://postgr.es/m/CAC8Q8tJ=7cOkPePyAbJE_Pf691t8nDFhJp0KZxHvnq_uicfyVg@mail.gmail.com --- src/interfaces/libpq/fe-protocol2.c | 10 +++++++++- src/interfaces/libpq/fe-protocol3.c | 10 +++++++++- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/src/interfaces/libpq/fe-protocol2.c b/src/interfaces/libpq/fe-protocol2.c index a58f701e18b..2bc7e6e8c7c 100644 --- a/src/interfaces/libpq/fe-protocol2.c +++ b/src/interfaces/libpq/fe-protocol2.c @@ -967,6 +967,14 @@ pqGetErrorNotice2(PGconn *conn, bool isError) char *startp; char *splitp; + /* + * If this is an error message, pre-emptively clear any incomplete query + * result we may have. We'd just throw it away below anyway, and + * releasing it before collecting the error might avoid out-of-memory. + */ + if (isError) + pqClearAsyncResult(conn); + /* * Since the message might be pretty long, we create a temporary * PQExpBuffer rather than using conn->workBuffer. workBuffer is intended @@ -1039,7 +1047,7 @@ pqGetErrorNotice2(PGconn *conn, bool isError) */ if (isError) { - pqClearAsyncResult(conn); + pqClearAsyncResult(conn); /* redundant, but be safe */ conn->result = res; resetPQExpBuffer(&conn->errorMessage); if (res && !PQExpBufferDataBroken(workBuf) && res->errMsg) diff --git a/src/interfaces/libpq/fe-protocol3.c b/src/interfaces/libpq/fe-protocol3.c index a484fe80a15..51055be88d7 100644 --- a/src/interfaces/libpq/fe-protocol3.c +++ b/src/interfaces/libpq/fe-protocol3.c @@ -880,6 +880,14 @@ pqGetErrorNotice3(PGconn *conn, bool isError) PQExpBufferData workBuf; char id; + /* + * If this is an error message, pre-emptively clear any incomplete query + * result we may have. We'd just throw it away below anyway, and + * releasing it before collecting the error might avoid out-of-memory. + */ + if (isError) + pqClearAsyncResult(conn); + /* * Since the fields might be pretty long, we create a temporary * PQExpBuffer rather than using conn->workBuffer. workBuffer is intended @@ -944,7 +952,7 @@ pqGetErrorNotice3(PGconn *conn, bool isError) { if (res) res->errMsg = pqResultStrdup(res, workBuf.data); - pqClearAsyncResult(conn); + pqClearAsyncResult(conn); /* redundant, but be safe */ conn->result = res; if (PQExpBufferDataBroken(workBuf)) printfPQExpBuffer(&conn->errorMessage,