Re: PQexec() hangs on OOM

From: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
To: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
Cc: Heikki Linnakangas <hlinnaka(at)iki(dot)fi>, Oleksandr Shulgin <oleksandr(dot)shulgin(at)zalando(dot)de>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Pg Bugs <pgsql-bugs(at)postgresql(dot)org>
Subject: Re: PQexec() hangs on OOM
Date: 2015-10-11 13:01:38
Message-ID: CAB7nPqSeZNNz4npbQq2m1sD--juQy0DQJRxSy-C1s2B8LYFHUA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox
Thread:
Lists: pgsql-bugs

On Sat, Oct 10, 2015 at 9:06 PM, Amit Kapila wrote:
> Okay, in that case, you can revert that change in your first patch and
> then that patch will be Ready for committer.

Yep. Done.

> In second patch [1], the handling is to continue on error as below:
>
> - if (getParamDescriptions(conn))
> + if (getParamDescriptions(conn, msgLength))
> return;
> - break;
> + /* getParamDescriptions() moves inStart itself */
> + continue;
>
> Now, assume there is "out of memory" error in getParamDescription(),
> the next message is 'T' which leads to a call to getRowDescriptions()
> and in that function if there is an error in below part of code, then I
> think it will just overwrite the previous "out of memory" error (I have
> tried by manual debugging and forcefully generating the below error):

> Here, the point to note is that for similar handling of error from
> getRowDescriptions(), we just skip the consecutive 'D' messages
> by checking resultStatus.
> I think overwriting of error for this case is not the right behaviour.
> [1] - 0002-Fix-OOM-error-handling-in-BIND-protocol-of-libpq-2.patch

Yeah, this behavior is caused by this piece of code:
@@ -600,7 +601,9 @@ getRowDescriptions(PGconn *conn, int msgLength)
advance_and_error:
/* Discard unsaved result, if any */
if (result && result != conn->result)
PQclear(result);
An idea may be to check for conn->result->resultStatus !=
PGRES_FATAL_ERROR to concatenate correctly the error messages using
pqCatenateResultError. But just returning the first error encountered
as you mention would be more natural. So I updated the patch this way.
Regards,
--
Michael

Attachment Content-Type Size
0001-Fix-OOM-error-handling-in-COPY-protocol-of-libpq.patch application/x-patch 7.1 KB
0002-Fix-OOM-error-handling-in-BIND-protocol-of-libpq.patch application/x-patch 4.3 KB

In response to

Responses

Browse pgsql-bugs by date

  From Date Subject
Next Message jan.magnusson0 2015-10-11 18:33:40 BUG #13673: does not install the c++ runtime version
Previous Message Amit Kapila 2015-10-10 12:06:36 Re: PQexec() hangs on OOM