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
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 |