Skip to content

Commit eb15f53

Browse files
committed
Improve libpq's error recovery for connection loss during COPY.
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.
1 parent a69cc9b commit eb15f53

File tree

2 files changed

+36
-12
lines changed

2 files changed

+36
-12
lines changed

src/interfaces/libpq/fe-exec.c

Lines changed: 34 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@ static int PQsendQueryGuts(PGconn *conn,
6161
const int *paramFormats,
6262
int resultFormat);
6363
static void parseInput(PGconn *conn);
64+
static PGresult *getCopyResult(PGconn *conn, ExecStatusType copytype);
6465
static bool PQexecStart(PGconn *conn);
6566
static PGresult *PQexecFinish(PGconn *conn);
6667
static int PQsendDescribe(PGconn *conn, char desc_type,
@@ -1563,22 +1564,13 @@ PQgetResult(PGconn *conn)
15631564
conn->asyncStatus = PGASYNC_BUSY;
15641565
break;
15651566
case PGASYNC_COPY_IN:
1566-
if (conn->result && conn->result->resultStatus == PGRES_COPY_IN)
1567-
res = pqPrepareAsyncResult(conn);
1568-
else
1569-
res = PQmakeEmptyPGresult(conn, PGRES_COPY_IN);
1567+
res = getCopyResult(conn, PGRES_COPY_IN);
15701568
break;
15711569
case PGASYNC_COPY_OUT:
1572-
if (conn->result && conn->result->resultStatus == PGRES_COPY_OUT)
1573-
res = pqPrepareAsyncResult(conn);
1574-
else
1575-
res = PQmakeEmptyPGresult(conn, PGRES_COPY_OUT);
1570+
res = getCopyResult(conn, PGRES_COPY_OUT);
15761571
break;
15771572
case PGASYNC_COPY_BOTH:
1578-
if (conn->result && conn->result->resultStatus == PGRES_COPY_BOTH)
1579-
res = pqPrepareAsyncResult(conn);
1580-
else
1581-
res = PQmakeEmptyPGresult(conn, PGRES_COPY_BOTH);
1573+
res = getCopyResult(conn, PGRES_COPY_BOTH);
15821574
break;
15831575
default:
15841576
printfPQExpBuffer(&conn->errorMessage,
@@ -1615,6 +1607,36 @@ PQgetResult(PGconn *conn)
16151607
return res;
16161608
}
16171609

1610+
/*
1611+
* getCopyResult
1612+
* Helper for PQgetResult: generate result for COPY-in-progress cases
1613+
*/
1614+
static PGresult *
1615+
getCopyResult(PGconn *conn, ExecStatusType copytype)
1616+
{
1617+
/*
1618+
* If the server connection has been lost, don't pretend everything is
1619+
* hunky-dory; instead return a PGRES_FATAL_ERROR result, and reset the
1620+
* asyncStatus to idle (corresponding to what we'd do if we'd detected I/O
1621+
* error in the earlier steps in PQgetResult). The text returned in the
1622+
* result is whatever is in conn->errorMessage; we hope that was filled
1623+
* with something relevant when the lost connection was detected.
1624+
*/
1625+
if (conn->status != CONNECTION_OK)
1626+
{
1627+
pqSaveErrorResult(conn);
1628+
conn->asyncStatus = PGASYNC_IDLE;
1629+
return pqPrepareAsyncResult(conn);
1630+
}
1631+
1632+
/* If we have an async result for the COPY, return that */
1633+
if (conn->result && conn->result->resultStatus == copytype)
1634+
return pqPrepareAsyncResult(conn);
1635+
1636+
/* Otherwise, invent a suitable PGresult */
1637+
return PQmakeEmptyPGresult(conn, copytype);
1638+
}
1639+
16181640

16191641
/*
16201642
* PQexec

src/interfaces/libpq/fe-misc.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -782,6 +782,8 @@ pqSendSome(PGconn *conn, int len)
782782
{
783783
printfPQExpBuffer(&conn->errorMessage,
784784
libpq_gettext("connection not open\n"));
785+
/* Discard queued data; no chance it'll ever be sent */
786+
conn->outCount = 0;
785787
return -1;
786788
}
787789

0 commit comments

Comments
 (0)