Improve libpq's error recovery for connection loss during COPY.
authorTom Lane <[email protected]>
Wed, 12 Feb 2014 22:50:07 +0000 (17:50 -0500)
committerTom Lane <[email protected]>
Wed, 12 Feb 2014 22:50:57 +0000 (17:50 -0500)
In pqSendSome, if the connection is already closed at entry, discard any
queued output data before returning.  There is no possibility of ever
sending the data, and anyway this corresponds to what we'd do if we'd
detected a hard error while trying to send().  This avoids possible
indefinite bloat of the output buffer if the application keeps trying
to send data (or even just keeps trying to do PQputCopyEnd, as psql
indeed will).

Because PQputCopyEnd won't transition out of PGASYNC_COPY_IN state
until it's successfully queued the COPY END message, and pqPutMsgEnd
doesn't distinguish a queuing failure from a pqSendSome failure,
this omission allowed an infinite loop in psql if the connection closure
occurred when we had at least 8K queued to send.  It might be worth
refactoring so that we can make that distinction, but for the moment
the other changes made here seem to offer adequate defenses.

To guard against other variants of this scenario, do not allow
PQgetResult to return a PGRES_COPY_XXX result if the connection is
already known dead.  Make sure it returns PGRES_FATAL_ERROR instead.

Per report from Stephen Frost.  Back-patch to all active branches.

src/interfaces/libpq/fe-exec.c
src/interfaces/libpq/fe-misc.c

index 5f371b48ccc6b18711e2f20394551e8364a30031..ecf9c2131196a2fd9a291f64fdc069ca0188908d 100644 (file)
@@ -63,6 +63,7 @@ static int PQsendQueryGuts(PGconn *conn,
                                const int *paramFormats,
                                int resultFormat);
 static void parseInput(PGconn *conn);
+static PGresult *getCopyResult(PGconn *conn, ExecStatusType copytype);
 static bool PQexecStart(PGconn *conn);
 static PGresult *PQexecFinish(PGconn *conn);
 static int PQsendDescribe(PGconn *conn, char desc_type,
@@ -1734,22 +1735,13 @@ PQgetResult(PGconn *conn)
                        conn->asyncStatus = PGASYNC_BUSY;
                        break;
                case PGASYNC_COPY_IN:
-                       if (conn->result && conn->result->resultStatus == PGRES_COPY_IN)
-                               res = pqPrepareAsyncResult(conn);
-                       else
-                               res = PQmakeEmptyPGresult(conn, PGRES_COPY_IN);
+                       res = getCopyResult(conn, PGRES_COPY_IN);
                        break;
                case PGASYNC_COPY_OUT:
-                       if (conn->result && conn->result->resultStatus == PGRES_COPY_OUT)
-                               res = pqPrepareAsyncResult(conn);
-                       else
-                               res = PQmakeEmptyPGresult(conn, PGRES_COPY_OUT);
+                       res = getCopyResult(conn, PGRES_COPY_OUT);
                        break;
                case PGASYNC_COPY_BOTH:
-                       if (conn->result && conn->result->resultStatus == PGRES_COPY_BOTH)
-                               res = pqPrepareAsyncResult(conn);
-                       else
-                               res = PQmakeEmptyPGresult(conn, PGRES_COPY_BOTH);
+                       res = getCopyResult(conn, PGRES_COPY_BOTH);
                        break;
                default:
                        printfPQExpBuffer(&conn->errorMessage,
@@ -1786,6 +1778,36 @@ PQgetResult(PGconn *conn)
        return res;
 }
 
+/*
+ * getCopyResult
+ *       Helper for PQgetResult: generate result for COPY-in-progress cases
+ */
+static PGresult *
+getCopyResult(PGconn *conn, ExecStatusType copytype)
+{
+       /*
+        * If the server connection has been lost, don't pretend everything is
+        * hunky-dory; instead return a PGRES_FATAL_ERROR result, and reset the
+        * asyncStatus to idle (corresponding to what we'd do if we'd detected I/O
+        * error in the earlier steps in PQgetResult).  The text returned in the
+        * result is whatever is in conn->errorMessage; we hope that was filled
+        * with something relevant when the lost connection was detected.
+        */
+       if (conn->status != CONNECTION_OK)
+       {
+               pqSaveErrorResult(conn);
+               conn->asyncStatus = PGASYNC_IDLE;
+               return pqPrepareAsyncResult(conn);
+       }
+
+       /* If we have an async result for the COPY, return that */
+       if (conn->result && conn->result->resultStatus == copytype)
+               return pqPrepareAsyncResult(conn);
+
+       /* Otherwise, invent a suitable PGresult */
+       return PQmakeEmptyPGresult(conn, copytype);
+}
+
 
 /*
  * PQexec
index 1eb3ac627d92249138e72c70d7e24c9c8f997c58..a7afd42556225477bdb867175032c1f4fbc180fa 100644 (file)
@@ -804,6 +804,8 @@ pqSendSome(PGconn *conn, int len)
        {
                printfPQExpBuffer(&conn->errorMessage,
                                                  libpq_gettext("connection not open\n"));
+               /* Discard queued data; no chance it'll ever be sent */
+               conn->outCount = 0;
                return -1;
        }