Re: PQexec() hangs on OOM

On Mon, Dec 14, 2015 at 10:41 PM, Heikki Linnakangas <hlinnaka(at)iki(dot)fi>
wrote:

> On 16/10/15 05:00, Michael Paquier wrote:
>
>> On Thu, Oct 15, 2015 at 11:28 PM, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
>> wrote:
>>
>>> On Sun, Oct 11, 2015 at 6:31 PM, Michael Paquier <
>>> michael(dot)paquier(at)gmail(dot)com>
>>> wrote:
>>>
>>>>
>>>> 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.
>>>>
>>>
> I pushed the ParamDescription fix. I'm not quite sure on the details of
> the COPY patch. Do you remember what this change in PQexecFinish is for:
>
> @@ -1991,6 +1995,9 @@ PQexecFinish(PGconn *conn)
>> * We have to stop if we see copy in/out/both, however. We will
>> resume
>> * parsing after application performs the data transfer.
>> *
>> + * Stop if we are in copy mode and error has occurred, the
>> pending results
>> + * will be discarded during next execution in PQexecStart.
>> + *
>> * Also stop if the connection is lost (else we'll loop
>> infinitely).
>> */
>> lastResult = NULL;
>> @@ -2020,6 +2027,11 @@ PQexecFinish(PGconn *conn)
>> result->resultStatus == PGRES_COPY_BOTH ||
>> conn->status == CONNECTION_BAD)
>> break;
>> + else if ((conn->asyncStatus == PGASYNC_COPY_IN ||
>> + conn->asyncStatus == PGASYNC_COPY_OUT
>> ||
>> + conn->asyncStatus == PGASYNC_COPY_BOTH)
>> &&
>> + result->resultStatus ==
>> PGRES_FATAL_ERROR)
>> + break;
>> }
>>
>> return lastResult;
>>
>
> I don't immediately see why that's needed. I ran the little tester program
> I wrote earlier (
> http://www.postgresql.org/message-id/55FC501A.5000409@iki.fi) with and
> without that change, and it behaved the same.
>
>
Without this change, it can ignore the OOM error for copy command.
As an example, for command like "copy t1 from stdin;", when the
flow reaches getCopyStart() in debugger, I have manually ensured
that PQmakeEmptyPGresult() return NULL by overwriting the return
value of malloc to zero and then it enters the error path
(advance_and_error) and then I just press continue and what I observed
is without above change it just doesn't show OOM error and with the
above change it properly reports OOM error.

With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com