Re: PQexec() hangs on OOM

On Fri, Apr 10, 2015 at 2:42 AM, Heikki Linnakangas <hlinnaka@iki.fi> wrote:
> On 04/08/2015 07:27 AM, Michael Paquier wrote:
>>
>> On Wed, Apr 8, 2015 at 3:54 AM, Heikki Linnakangas wrote:
>>>
>>> On 04/07/2015 09:18 AM, Michael Paquier wrote:
>>>>
>>>> I have noticed as well that getCopyStart() in fe-protocol3.c needs to
>>>> be made a little bit smarter to make the difference between an OOM and
>>>> the not-enough-data type of problem.
>>>
>>>
>>> Yeah. getParamDescription still has the issue, with your patch.
>>
>>
>> Check.
>
>
> There are still a few parseInput subroutines that have similar issues. In
> fe-protocol2.c:
>
> * pqGetErrorNotice2 returns EOF if there is not enough data, but also on
> OOM. The caller assumes it's because not enough data.
> * getRowDescriptions() is the same, although it sets conn->errorMessage on
> OOM.

OK, I extended those two with the same logic as previously.

> * getAnotherTuple() is the same, but it also skips over the received data,
> so you won't get stuck. So maybe that's OK.

I think that it should be changed for consistency with the others, so
done this way for this function, and this way we only need to bail-out
if status == -2, a status of -1 meaning that there is not enough data.

> * getNotify() just drops any NOTIFY messages on OOM. Perhaps that's OK..

This one is different though, it directly skips the messages.

> The corresponding functions in fe-protocol3.c are pretty much identical,
> with the same issues. In addition:
>
> * getParameterStatus will act as if the parameter value was "out of memory".
> That'll be fun for something like client_encoding or
> standard_conforming_strings. Would be good to use a small char[100]
> variable, in stack, if it fits, and only use malloc for larger values. That
> would cover all the common variables that need to be machine-parsed.

Are you suggesting to replace PQExpBuffer?

So, attached is take three for all the other things above.
Regards,
--
Michael