Re: PQexec() hangs on OOM

Michael Paquier <michael(dot)paquier(at)gmail(dot)com> writes:
>
>> 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.

There's still one call to pqGetErrorNotice3 that doesn't check returned
value for -2, in fe-connect.c. Shouldn't we also check it like this:

diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index e7c7a25..330b8da 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -2242,6 +2242,7 @@ keep_going: /* We will come back to here until there is
int msgLength;
int avail;
AuthRequest areq;
+ int res;

/*
* Scan the message from current point (note that if we find
@@ -2366,11 +2367,18 @@ keep_going: /* We will come back to here until there is
{
if (PG_PROTOCOL_MAJOR(conn->pversion) >= 3)
{
- if (pqGetErrorNotice3(conn, true))
+ res = pqGetErrorNotice3(conn, true);
+ if (res == -1)
{
/* We'll come back when there is more data */
return PGRES_POLLING_READING;
}
+ else if (res == -2)
+ {
+ printfPQExpBuffer(&conn->errorMessage,
+ libpq_gettext("out of memory\n"));
+ goto error_return;
+ }
}
else
{

--
Alex