Re: PQexec() hangs on OOM

From: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
To: Heikki Linnakangas <hlinnaka(at)iki(dot)fi>
Cc: Oleksandr Shulgin <oleksandr(dot)shulgin(at)zalando(dot)de>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Pg Bugs <pgsql-bugs(at)postgresql(dot)org>
Subject: Re: PQexec() hangs on OOM
Date: 2015-07-04 12:40:31
Message-ID: CAB7nPqSm57dvLyQsCf7+owgiNxK_sGfyFp3OdVj=YS3kRbTMqA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-bugs

On Sat, Jul 4, 2015 at 1:32 AM, Heikki Linnakangas wrote:
> In short, the idea of returning a status code from parseInput(), instead of
> just dealing with the error, was a bad one. Sorry I didn't have this
> epiphany earlier...
>
> I came up with the attached. It fixes the few cases where we were currently
> returning "need more data" when OOM happened, to deal with the OOM error
> instead, by setting conn->errorMessage. How does this look to you?

So this relies on the fact that when errorMessage is set subsequent
messages are ignored, right? This looks neat.

At the bottom of getAnotherTuple() in fe-protocol2.c if
PQmakeEmptyPGresult returns NULL, shouldn't the error message be
enforced to "out of memory"? It is an error code path, so an error
will be set, but perhaps the message is incorrect.

- if (!res->errMsg)
- goto failure;
+ if (res)
+ {
+ res->resultStatus = isError ? PGRES_FATAL_ERROR :
PGRES_NONFATAL_ERROR;
+ res->errMsg = pqResultStrdup(res, workBuf.data);
+ }
If res->errMsg is NULL, we may have a crash later on when calling
appendPQExpBufferStr on this field. I think that we should add an
additional check on it.
--
Michael

In response to

Responses

Browse pgsql-bugs by date

  From Date Subject
Next Message jingwei_5107 2015-07-04 15:12:03 BUG #13487: GetBufferFromRing code bug
Previous Message Roberto Morelli 2015-07-04 12:07:49 Re: BUG #13485: JSONB To recordset not working with CamelCase