diff options
| author | Tom Lane <tgl@sss.pgh.pa.us> | 2018-04-13 12:53:45 -0400 |
|---|---|---|
| committer | Tom Lane <tgl@sss.pgh.pa.us> | 2018-04-13 12:53:45 -0400 |
| commit | d25c2ee9c038969eca8080177738dddf97a2cade (patch) | |
| tree | 17d486d27a45212e6aa22728c29b00d08cdd6794 | |
| parent | fafec4cce814b9b15991b62520dc5e5e84655a8a (diff) | |
| download | postgresql-d25c2ee9c038969eca8080177738dddf97a2cade.tar.gz postgresql-d25c2ee9c038969eca8080177738dddf97a2cade.zip | |
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
| -rw-r--r-- | src/interfaces/libpq/fe-protocol2.c | 10 | ||||
| -rw-r--r-- | 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 7dcef808dd7..53e5083702c 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 *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 * for stuff that is expected to be short. @@ -1038,7 +1046,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 d3ca5d25f6f..8345faface8 100644 --- a/src/interfaces/libpq/fe-protocol3.c +++ b/src/interfaces/libpq/fe-protocol3.c @@ -880,6 +880,14 @@ pqGetErrorNotice3(PGconn *conn, bool isError) 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 * for stuff that is expected to be short. We shouldn't use @@ -943,7 +951,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, |