Re: PQexec() hangs on OOM

On Wed, Jul 8, 2015 at 1:01 AM, Heikki Linnakangas <hlinnaka@iki.fi> wrote:
> On 07/07/2015 09:32 AM, Michael Paquier wrote:
>>
>> On Tue, Jul 7, 2015 at 3:31 PM, Michael Paquier
>> <michael.paquier@gmail.com> wrote:
>>>
>>> On Tue, Jul 7, 2015 at 2:13 AM, Heikki Linnakangas wrote:
>>>>
>>>> The getParamDescriptions() changes were slightly broken. It didn't read
>>>> the
>>>> whole input message with pqGetInt() etc., so pqParseInput3() threw the
>>>> "message contents do not agree with length in message type" error. I
>>>> started
>>>> fixing that, by changing the error handling in that function to be more
>>>> like
>>>> that in getRowDescriptions(), but then I realized that all the EOF
>>>> return
>>>> cases in the pqParseInput3() subroutines are actually dead code.
>>>> pqParseInput3() always reads the whole message, before passing it on to
>>>> the
>>>> right subroutine. That was documented for getRowDescriptions() and
>>>> getAnotherTuple(), but the rest of the functions were more like the
>>>> protocol
>>>> version 2 code, prepared to deal with incomplete messages. I think it
>>>> would
>>>> be good to refactor that, removing the EOF return cases altogether. So I
>>>> left out that change for now as well.
>>>
>>>
>>> Yes, (the latter case is not actually used currently). Well, I don't
>>> mind writing the additional patch to update . On top of that The
>>> refactoring should be a master-only change, perhaps?
>>
>>
>> I pushed the send button too early.
>> I don't mind writing the additional patch for the other routines,
>> patch that should be backpatched, and the refactoring patch
>> (master-only?).
>
>
> Ok, committed and backpatched the latest patch I posted. Yeah, we'll need
> additional patches for the refactoring and the remaining issues. I'm not
> sure if it's worth trying to backpatch them; let's see how big the patch is.

So, here are two patches aimed at fixing the hangling issues with
getStartCopy and getParamDescriptions. After trying a couple of
approaches, I found out that the most elegant way to prevent the
infinite loop in parseInput is to introduce a new PGASYNC flag to
control the error and let the caller know what happened.

More refactoring could be done, and I think that the use of this new
ASYNC flag could be spread to other places as well if you think that's
a useful. For now I have focused only on fixing the existing problems
with fixes that are rather back-patchable.
--
Michael