Thread: PQexec infinite loop

we have some perl code that used a DBD::Pg statement handle after
the parent database handle had been disconnected.
by doing this we could consistently put libpq into a spin:

PQexec (conn=0x829e120,
    query=0x82a2b80 "SELECT \"login\"\n\t       FROM \"account\"\n", ' ' <repeats 15 times>, "WHERE \"id\" = '1'\n", '
'<repeats 14 times>) at fe-exec.c:1207 

(gdb) p *result
$5 = {ntups = 0, numAttributes = 0, attDescs = 0x0, tuples = 0x0,
  tupArrSize = 0, resultStatus = PGRES_FATAL_ERROR,
  cmdStatus = "\000\000\000\000 \000\000\000ÿÿÿÿgetdatabaseencoding\000\a\000\000\000 ,*\b", binary = 0, xconn =
0x829e120,noticeHook = 0, noticeArg = 0x0,  
  client_encoding = 0,
  errMsg = 0x82a2c54 "PQgetResult: Unexpected asyncStatus 145\n",
  null_field = "", curBlock = 0x82a2c50, curOffset = 45, spaceLeft = 2003}

looping in fe-exec.c:PQexec:
    while ((result = PQgetResult(conn)) != NULL)
    {
        if (result->resultStatus == PGRES_COPY_IN ||
            result->resultStatus == PGRES_COPY_OUT)
        {
            PQclear(result);
            printfPQExpBuffer(&conn->errorMessage,
                "PQexec: you gotta get out of a COPY state yourself.\n");
            /* restore blocking status */
            goto errout;
        }
        PQclear(result);
    }


result is EmptyPGresult with PGRES_FATAL_ERROR, so never NULL


PQgetResult(conn) exits at default case of:

    /* Return the appropriate thing. */
    switch (conn->asyncStatus)
    {
        case PGASYNC_IDLE:
            res = NULL;            /* query is complete */
            break;
        case PGASYNC_READY:
            res = prepareAsyncResult(conn);
            /* Set the state back to BUSY, allowing parsing to proceed. */
            conn->asyncStatus = PGASYNC_BUSY;
            break;
        case PGASYNC_COPY_IN:
            res = PQmakeEmptyPGresult(conn, PGRES_COPY_IN);
            break;
        case PGASYNC_COPY_OUT:
            res = PQmakeEmptyPGresult(conn, PGRES_COPY_OUT);
            break;
        default:
            printfPQExpBuffer(&conn->errorMessage,
                              "PQgetResult: Unexpected asyncStatus %d\n",
                              (int) conn->asyncStatus);
            res = PQmakeEmptyPGresult(conn, PGRES_FATAL_ERROR);
            break;
    }

    return res;


no idea how asyncStatus came to be 145:
(this looks like garbage to me. scribbled on by DBD::Pg ?)

(gdb) p *conn
$8 = {
  pghost = 0x40169928 " \231\026@
\231\026@\030á)\b\030á)\b0\231\026@0\231\026@8\231\026@8\231\026@@\231\026@@\231\026@H\231\026@H\231\026@P\231\026@P\231\026@X\231\026@X\231\026@`\231\026@`\231\026@h\231\026@h\231\026@p\231\026@p\231\026@x\231\026@x\231\026@\200\231\026@\200\231\026@\210\231\026@\210\231\026@\220\231\026@\220\231\026@\230\231\026@\230\231\026@ \231\026@ \231\026@¨\231\026@¨\231\026@°\231\026@°\231\026@¸\231\026@¸\231\026@À\231\026@À\231\026@È\231\026@È\231\026@Ð\231\026@Ð\231\026@Ø\231\026@Ø\231\026@à\231\026@à\231\026@"...,

  pghostaddr = 0x40169928 " \231\026@
\231\026@\030á)\b\030á)\b0\231\026@0\231\026@8\231\026@8\231\026@@\231\026@@\231\026@H\231\026@H\231\026@P\231\026@P\231\026@X\231\026@X\231\026@`\231\026@`\231\026@h\231\026@h\231\026@p\231\026@p\231\026@x\231\026@x\231\026@\200\231\026@\200\231\026@\210\231\026@\210\231\026@\220\231\026@\220\231\026@\230\231\026@\230\231\026@ \231\026@ \231\026@¨\231\026@¨\231\026@°\231\026@°\231\026@¸\231\026@¸\231\026@À\231\026@À\231\026@È\231\026@È\231\026@Ð\231\026@Ð\231\026@Ø\231\026@Ø\231\026@à\231\026@à\231\026@"...,

  pgport = 0x8290fb8 " \017)\b\200â\031\b\f )\b¸\017)\b\230â\031\b",
  pgunixsocket = 0x0, pgtty = 0x0,
  pgoptions = 0x8290fc4 "¸\017)\b\230â\031\b",
  dbName = 0x8290fac "Ä\017)\bH¼\"\b\030 )\b \017)\b\200â\031\b\f )\b¸\017)\b\230â\031\b", pguser = 0x8290fd0
"¬\017)\bhâ\031\b$ )\b",pgpass = 0x0,  
  Pfdebug = 0x0, noticeHook = 0, noticeArg = 0x0, status = CONNECTION_BAD,
  asyncStatus = 145, notifyList = 0x401698f0, sock = 1075222768, laddr = {
    sa = {sa_family = 7, sa_data = '\000' <repeats 13 times>}, in = {
      sin_family = 7, sin_port = 0, sin_addr = {s_addr = 0},
      sin_zero = "\000\000\000\000\000\000\000"}, un = {sun_family = 7,
      sun_path = '\000' <repeats 22 times>, "ÿÿÿÿ", '\000' <repeats 16 times>, "Y\000\000\000¸\230\026@¸\230\026@",
'\000'<repeats 20 times>, "\224\017)\b", '\000' <repeats 20 times>, "!\000\000\000àÝ)\b\200\230"}}, raddr = {sa = { 
      sa_family = 2, sa_data = "\0258\021\000\000\000 ã)\bhi*\b"}, in = {
      sin_family = 2, sin_port = 14357, sin_addr = {s_addr = 17},
      sin_zero = " ã)\bhi*\b"}, un = {sun_family = 2,
      sun_path = "\0258\021\000\000\000 ã)\bhi*\bÈ\000\000\000\020\000\000\000DBD::Pg::db\0009", '\000' <repeats 51
times>,"8\000\000\000\021\000\000\000test\000\230\026@\000\000\000\000\021"}}, raddr_len = 0, be_pid = 0, be_key = 16,  
  salt = "\021", lobjfuncs = 0x40169800,
  inBuffer = 0x40169868
"H\213*\b`\230\026@P4*\bP4*\bèa*\b ã)\bx\230\026@x\230\026@àÝ)\bàÝ)\b\210i*\b\210i*\b\220\230\026@\220\230\026@è\213)\bè\213)\b(Ý)\b(Ý)\b\030å)\b\030å)\b°\230\026@°\230\026@¸ß)\b¸ß)\bÀ\230\026@À\230\026@È\230\026@È\230\026@Ð\230\026@Ð\230\026@Ø\230\026@Ø\230\026@à\230\026@à\230\026@è\230\026@è\230\026@ð\230\026@ð\230\026@ø\230\026@ø\230\026@",
inBufSize= 16384,  
  inStart = 57, inCursor = 57, inEnd = 4, nonblocking = 0,
  outBuffer = 0xb <Address 0xb out of bounds>, outBufSize = 0, outCount = 0,
  result = 0x829e4c8, curTuple = 0x81d2b98, setenv_state = 4294967295,
  next_eo = 0x0, allow_ssl_try = 0 '\000', require_ssl = 0 '\000', ssl = 0x0,
  errorMessage = {
    data = 0x82a6770 "PQgetResult: Unexpected asyncStatus 145\n", len = 40,
    maxlen = 977551940}, workBuffer = {
    data = 0x3a67503a <Address 0x3a67503a out of bounds>, len = 6448186,
    maxlen = 57}, client_encoding = 0}



is it correct to just do this?
(should BAD_RESPONSE and NONFATAL_ERROR also be checked?)

    while ((result = PQgetResult(conn)) != NULL)
    {
        if (result->resultStatus == PGRES_COPY_IN ||
            result->resultStatus == PGRES_COPY_OUT)
        {
            PQclear(result);
            printfPQExpBuffer(&conn->errorMessage,
                "PQexec: you gotta get out of a COPY state yourself.\n");
            /* restore blocking status */
            goto errout;
        }
+        else if (result->resultStatus == PGRES_FATAL_ERROR)
+        {
+            PQclear(result);
+            goto errout;
+        }
        PQclear(result);
    }

(yes i know -- if DBD::Pg screwed up then it isn't PQexec's fault, but
afaics PQexec should still be checking for errors)

--
 - Gus

Angus Lees <gus@switchonline.com.au> writes:
> by doing this we could consistently put libpq into a spin:

Hmmm ... the patch you give is no good as-is, since it will break the
behavior for "normal" errors.  If we want to work around this, we'll
need some way to distinguish persistent inside-libpq failure conditions
from plain old backend error reports.  Looking at resultStatus isn't
enough to tell the difference.

The particular scenario you describe is not something that libpq can
reasonably be expected to defend against --- once the database handle
is disconnected, the "conn" structure is freed, and so that memory
could be reallocated and filled with completely arbitrary contents.
libpq could hardly even be expected to guarantee no core dump, let
alone nice friendly easily-interpreted behavior.  But it does seem that
there might be similar scenarios with more-legitimate causes in which
PQgetResult returns a non-transient error condition, and so the loop
in PQexec never exits.  Maybe another flag field in PGconn is needed?

            regards, tom lane