Re: PQexec() hangs on OOM

Michael Paquier <michael.paquier@gmail.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