Thread: BUG #5697: Infinite loop inside PQexecStart function
The following bug has been logged online:
Bug reference: 5697
Logged by: Boris
Email address: admin@nyc.yamaha.com
PostgreSQL version: 8.3.5
Operating system: Linux RH ES5
Description: Infinite loop inside PQexecStart function
Details:
The infinite loop in this case occurs inside the PQexecStart() function in
pgsql driver. The following insert corresponds to the actual infinite loop.
The are several conditions that are checked "(result = PQgetResult(conn)) !=
NULL" and "result->resultStatus == PGRES_COPY_IN || result->resultStatus ==
PGRES_COPY_OUT || conn->status == CONNECTION_BAD" as an exit point.
---------------------------------------------------------
ASM code
0x0042bca5 in PQexecStart () from /usr/local/pgsql/lib/libpq.so.5
17 0x0042bca5 <PQexecStart+37>: cmp $0x3,%esi
18 0x0042bca8 <PQexecStart+40>: je 0x42bd00 <PQexecStart+128>
19 0x0042bcaa <PQexecStart+42>: cmpl $0x1,0x44(%edi)
20 0x0042bcae <PQexecStart+46>: je 0x42bcf0 <PQexecStart+112>
21 0x0042bcb0 <PQexecStart+48>: mov %edi,(%esp)
22 0x0042bcb3 <PQexecStart+51>: call 0x424fa0 <PQgetResult@plt>
23 0x0042bcb8 <PQexecStart+56>: test %eax,%eax
24 0x0042bcba <PQexecStart+58>: je 0x42bd13 <PQexecStart+147>
25 0x0042bcbc <PQexecStart+60>: mov 0x1c(%eax),%esi
26 0x0042bcbf <PQexecStart+63>: mov %eax,(%esp)
27 0x0042bcc2 <PQexecStart+66>: call 0x4255d0 <PQclear@plt>
28 0x0042bcc7 <PQexecStart+71>: cmp $0x4,%esi
29 0x0042bcca <PQexecStart+74>: jne 0x42bca5 <PQexecStart+37>
---------------------------------------------------------
The C-code corresponding to this part (short version):
while ((result = PQgetResult(conn)) != NULL){
ExecStatusType resultStatus = result->resultStatus;
PQclear(result); /* only need its status */
/* check for loss of connection, too */
if (result->resultStatus == PGRES_COPY_IN ||
result->resultStatus == PGRES_COPY_OUT ||
conn->status == CONNECTION_BAD)
break;
}
return true;
---------------------------------------------------------
These are the values mapped to the corresponding constants:
PGRES_EMPTY_QUERY = 0
PGRES_COMMAND_OK = 1
PGRES_TUPLES_OK = 2
PGRES_COPY_OUT = 3
PGRES_COPY_IN = 4
PGRES_BAD_RESPONSE = 5
PGRES_NONFATAL_ERROR = 6
PGRES_FATAL_ERROR = 7
Condition exit point is evaluated against 3 constants PGRES_COPY_IN,
PGRES_COPY_OUT, CONNECTION_BAD. Since the connection to the database is in a
"GOOD" state the only constants that are evaluated are PGRES_COPY_IN and
PGRES_COPY_OUT, but according to the debugger trace the value those are
compared against is 7, e.g. PGRES_FATAL_ERROR which has no condition and
thus the process runs forever. Please see the following insert with detailed
output of the registers.
---------------------------------------------------------
0x0042bcc7 in PQexecStart () from /usr/local/pgsql/lib/libpq.so.5
1: x/i $pc 0x42bcc7 <PQexecStart+71>: cmp $0x4,%esi
(gdb) i r
eax 0x99 153
ecx 0x1 1
edx 0x98 152
ebx 0x43a330 4432688
esp 0xbfe506d0 0xbfe506d0
ebp 0xbfe506e8 0xbfe506e8
esi 0x7 7
edi 0x98c5bb4 160193460
eip 0x42bcc7 0x42bcc7 <PQexecStart+71>
eflags 0x286 [ PF SF IF ]
cs 0x73 115
ss 0x7b 123
ds 0x7b 123
es 0x7b 123
fs 0x0 0
gs 0x33 51
"Boris" <admin@nyc.yamaha.com> writes:
> while ((result = PQgetResult(conn)) != NULL){
> ExecStatusType resultStatus = result->resultStatus;
> PQclear(result); /* only need its status */
> /* check for loss of connection, too */
> if (result->resultStatus == PGRES_COPY_IN ||
> result->resultStatus == PGRES_COPY_OUT ||
> conn->status == CONNECTION_BAD)
> break;
> }
This code is broken: once you've done PQclear() it's unsafe to access
the PGresult. I think you meant just "resultStatus" not
"result->resultStatus" in the if().
regards, tom lane
Boris Bondarenko <bbondarenko@nyc.yamaha.com> writes:
> That was a short form i used to point out the issue, the actual code
> from src/interfaces/libpq/fe-exec.c
Oh. Well, you didn't actually explain under what conditions you think
that fails. Since that code hasn't changed in quite a few releases,
and is extremely heavily exercised every day, the burden is on you
to demonstrate that there's a problem.
regards, tom lane
That was a short form i used to point out the issue, the actual code
from src/interfaces/libpq/fe-exec.c
1368 static bool
1369 PQexecStart(PGconn *conn)
1370 {
1371 PGresult *result;
1372
1373 if (!conn)
1374 return false;
1375
1376 /*
1377 * Silently discard any prior query result that
application didn't eat.
1378 * This is probably poor design, but it's here for
backward compatibility.
1379 */
1380 while ((result = PQgetResult(conn)) != NULL)
1381 {
1382 ExecStatusType resultStatus = result->resultStatus;
1383
1384 PQclear(result); /* only need
its status */
1385 if (resultStatus == PGRES_COPY_IN)
1386 {
1387 if (PG_PROTOCOL_MAJOR(conn->pversion) >= 3)
1388 {
1389 /* In protocol 3, we can get
out of a COPY IN state */
1390 if (PQputCopyEnd(conn,
1391
libpq_gettext("COPY terminated by new PQexec")) < 0)
1392 return false;
1393 /* keep waiting to swallow the
copy's failure message */
1394 }
1395 else
1396 {
1397 /* In older protocols we have
to punt */
1398
printfPQExpBuffer(&conn->errorMessage,
1399 libpq_gettext("COPY IN state
must be terminated first\n"));
1400 return false;
1401 }
1402 }
1403 else if (resultStatus == PGRES_COPY_OUT)
1404 {
1405 if (PG_PROTOCOL_MAJOR(conn->pversion) >= 3)
1406 {
1407 /*
1408 * In protocol 3, we can get
out of a COPY OUT state: we just
1409 * switch back to BUSY and
allow the remaining COPY data to be
1410 * dropped on the floor.
1411 */
1412 conn->asyncStatus = PGASYNC_BUSY;
1413 /* keep waiting to swallow the
copy's completion message */
1414 }
1415 else
1416 {
1417 /* In older protocols we have
to punt */
1418
printfPQExpBuffer(&conn->errorMessage,
1419 libpq_gettext("COPY OUT state
must be terminated first\n"));
1420 return false;
1421 }
1422 }
1423 /* check for loss of connection, too */
1424 if (conn->status == CONNECTION_BAD)
1425 return false;
1426 }
1427
1428 /* OK to send a command */
1429 return true;
1430 }
Sorry for the confusion from the shortened form.
Tom Lane wrote:
> "Boris" <admin@nyc.yamaha.com> writes:
>> while ((result = PQgetResult(conn)) != NULL){
>> ExecStatusType resultStatus = result->resultStatus;
>> PQclear(result); /* only need its status */
>> /* check for loss of connection, too */
>> if (result->resultStatus == PGRES_COPY_IN ||
>> result->resultStatus == PGRES_COPY_OUT ||
>> conn->status == CONNECTION_BAD)
>> break;
>> }
>
> This code is broken: once you've done PQclear() it's unsafe to access
> the PGresult. I think you meant just "resultStatus" not
> "result->resultStatus" in the if().
>
> regards, tom lane
>
--
Boris Bondarenko -:- bbondarenko@nyc.yamaha.com
Yamaha Music Interactive Inc.
YMIA