diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c index c96a52b..e3bf6a7 100644 --- a/src/interfaces/libpq/fe-connect.c +++ b/src/interfaces/libpq/fe-connect.c @@ -537,6 +537,10 @@ pqDropServerData(PGconn *conn) conn->last_sqlstate[0] = '\0'; conn->auth_req_received = false; conn->password_needed = false; + conn->write_failed = false; + if (conn->write_err_msg) + free(conn->write_err_msg); + conn->write_err_msg = NULL; conn->be_pid = 0; conn->be_key = 0; } @@ -3702,6 +3706,8 @@ freePGconn(PGconn *conn) /* Note that conn->Pfdebug is not ours to close or free */ if (conn->last_query) free(conn->last_query); + if (conn->write_err_msg) + free(conn->write_err_msg); if (conn->inBuffer) free(conn->inBuffer); if (conn->outBuffer) diff --git a/src/interfaces/libpq/fe-exec.c b/src/interfaces/libpq/fe-exec.c index ac969e7..93ff428 100644 --- a/src/interfaces/libpq/fe-exec.c +++ b/src/interfaces/libpq/fe-exec.c @@ -791,6 +791,32 @@ pqSaveErrorResult(PGconn *conn) } /* + * As above, and append conn->write_err_msg to whatever other error we have. + * This is used when we've detected a write failure and have exhausted our + * chances of reporting something else instead. + */ +static void +pqSaveWriteError(PGconn *conn) +{ + /* + * Ensure conn->result is an error result, and add anything in + * conn->errorMessage to it. + */ + pqSaveErrorResult(conn); + + /* + * Now append write_err_msg to that. If it's null because of previous + * strdup failure, do what we can. (It's likely our machinations here are + * all getting OOM failures as well, but ...) + */ + if (conn->write_err_msg) + pqCatenateResultError(conn->result, conn->write_err_msg); + else + pqCatenateResultError(conn->result, + libpq_gettext("write to server failed\n")); +} + +/* * This subroutine prepares an async result object for return to the caller. * If there is not already an async result object, build an error object * using whatever is in conn->errorMessage. In any case, clear the async @@ -1224,7 +1250,7 @@ PQsendQuery(PGconn *conn, const char *query) pqPuts(query, conn) < 0 || pqPutMsgEnd(conn) < 0) { - pqHandleSendFailure(conn); + /* error message should be set up already */ return 0; } @@ -1243,7 +1269,7 @@ PQsendQuery(PGconn *conn, const char *query) */ if (pqFlush(conn) < 0) { - pqHandleSendFailure(conn); + /* error message should be set up already */ return 0; } @@ -1389,7 +1415,7 @@ PQsendPrepare(PGconn *conn, return 1; sendFailed: - pqHandleSendFailure(conn); + /* error message should be set up already */ return 0; } @@ -1641,40 +1667,11 @@ PQsendQueryGuts(PGconn *conn, return 1; sendFailed: - pqHandleSendFailure(conn); + /* error message should be set up already */ return 0; } /* - * pqHandleSendFailure: try to clean up after failure to send command. - * - * Primarily, what we want to accomplish here is to process any ERROR or - * NOTICE messages that the backend might have sent just before it died. - * Since we're in IDLE state, all such messages will get sent to the notice - * processor. - * - * NOTE: this routine should only be called in PGASYNC_IDLE state. - */ -void -pqHandleSendFailure(PGconn *conn) -{ - /* - * Accept and parse any available input data, ignoring I/O errors. Note - * that if pqReadData decides the backend has closed the channel, it will - * close our side of the socket --- that's just what we want here. - */ - while (pqReadData(conn) > 0) - parseInput(conn); - - /* - * Be sure to parse available input messages even if we read no data. - * (Note: calling parseInput within the above loop isn't really necessary, - * but it prevents buffer bloat if there's a lot of data available.) - */ - parseInput(conn); -} - -/* * Select row-by-row processing mode */ int @@ -1763,8 +1760,11 @@ PQisBusy(PGconn *conn) /* Parse any available data, if our state permits. */ parseInput(conn); - /* PQgetResult will return immediately in all states except BUSY. */ - return conn->asyncStatus == PGASYNC_BUSY; + /* + * PQgetResult will return immediately in all states except BUSY, or if we + * had a write failure. + */ + return conn->asyncStatus == PGASYNC_BUSY || conn->write_failed; } @@ -1804,7 +1804,13 @@ PQgetResult(PGconn *conn) } } - /* Wait for some more data, and load it. */ + /* + * Wait for some more data, and load it. (Note: if the connection has + * been lost, pqWait should return immediately because the socket + * should be read-ready, either with the last server data or with an + * EOF indication. We expect therefore that this won't result in any + * undue delay in reporting a previous write failure.) + */ if (flushResult || pqWait(true, false, conn) || pqReadData(conn) < 0) @@ -1820,6 +1826,17 @@ PQgetResult(PGconn *conn) /* Parse it. */ parseInput(conn); + + /* + * If we had a write error, but nothing above obtained a query result + * or detected a read error, report the write error. + */ + if (conn->write_failed && conn->asyncStatus == PGASYNC_BUSY) + { + pqSaveWriteError(conn); + conn->asyncStatus = PGASYNC_IDLE; + return pqPrepareAsyncResult(conn); + } } /* Return the appropriate thing. */ @@ -2252,7 +2269,7 @@ PQsendDescribe(PGconn *conn, char desc_type, const char *desc_target) return 1; sendFailed: - pqHandleSendFailure(conn); + /* error message should be set up already */ return 0; } diff --git a/src/interfaces/libpq/fe-misc.c b/src/interfaces/libpq/fe-misc.c index e5ef8d4..718da1c 100644 --- a/src/interfaces/libpq/fe-misc.c +++ b/src/interfaces/libpq/fe-misc.c @@ -824,6 +824,13 @@ definitelyFailed: * * Return 0 on success, -1 on failure and 1 when not all data could be sent * because the socket would block and the connection is non-blocking. + * + * Upon write failure, conn->write_failed is set and the error message is + * saved in conn->write_err_msg, but we clear the output buffer and return + * zero anyway; this is because callers should soldier on until it's possible + * to read from the server and check for an error message. write_err_msg + * should be reported only when we are unable to obtain a server error first. + * (Thus, a -1 result is returned only for an internal *read* failure.) */ static int pqSendSome(PGconn *conn, int len) @@ -832,13 +839,32 @@ pqSendSome(PGconn *conn, int len) int remaining = conn->outCount; int result = 0; + /* + * If we already had a write failure, we will never again try to send data + * on that connection. Even if the kernel would let us, we've probably + * lost message boundary sync with the server. conn->write_failed + * therefore persists until the connection is reset, and we just discard + * all data presented to be written. + */ + if (conn->write_failed) + { + /* conn->write_err_msg should be set up already */ + conn->outCount = 0; + return 0; + } + if (conn->sock == PGINVALID_SOCKET) { printfPQExpBuffer(&conn->errorMessage, libpq_gettext("connection not open\n")); + conn->write_failed = true; + /* Transfer error message to conn->write_err_msg, if possible */ + /* (strdup failure is OK, we'll cope later) */ + conn->write_err_msg = strdup(conn->errorMessage.data); + resetPQExpBuffer(&conn->errorMessage); /* Discard queued data; no chance it'll ever be sent */ conn->outCount = 0; - return -1; + return 0; } /* while there's still data to send */ @@ -876,17 +902,18 @@ pqSendSome(PGconn *conn, int len) default: /* pqsecure_write set the error message for us */ + conn->write_failed = true; /* - * We used to close the socket here, but that's a bad idea - * since there might be unread data waiting (typically, a - * NOTICE message from the backend telling us it's - * committing hara-kiri...). Leave the socket open until - * pqReadData finds no more data can be read. But abandon - * attempt to send data. + * Transfer error message to conn->write_err_msg, if + * possible (strdup failure is OK, we'll cope later) */ + conn->write_err_msg = strdup(conn->errorMessage.data); + resetPQExpBuffer(&conn->errorMessage); + + /* Discard queued data; no chance it'll ever be sent */ conn->outCount = 0; - return -1; + return 0; } } else @@ -921,6 +948,9 @@ pqSendSome(PGconn *conn, int len) * can do, and works pretty well in practice. (The documentation * used to say that you only need to wait for write-ready, so * there are still plenty of applications like that out there.) + * + * Note that errors here don't result in write_failed becoming + * set. */ if (pqReadData(conn) < 0) { @@ -956,6 +986,7 @@ pqSendSome(PGconn *conn, int len) * * Return 0 on success, -1 on failure and 1 when not all data could be sent * because the socket would block and the connection is non-blocking. + * (See pqSendSome comments about how failure should be handled.) */ int pqFlush(PGconn *conn) diff --git a/src/interfaces/libpq/fe-protocol2.c b/src/interfaces/libpq/fe-protocol2.c index 1ab1421..0e36974 100644 --- a/src/interfaces/libpq/fe-protocol2.c +++ b/src/interfaces/libpq/fe-protocol2.c @@ -1450,42 +1450,30 @@ pqFunctionCall2(PGconn *conn, Oid fnid, pqPutInt(fnid, 4, conn) != 0 || /* function id */ pqPutInt(nargs, 4, conn) != 0) /* # of args */ { - pqHandleSendFailure(conn); + /* error message should be set up already */ return NULL; } for (i = 0; i < nargs; ++i) { /* len.int4 + contents */ if (pqPutInt(args[i].len, 4, conn)) - { - pqHandleSendFailure(conn); return NULL; - } if (args[i].isint) { if (pqPutInt(args[i].u.integer, 4, conn)) - { - pqHandleSendFailure(conn); return NULL; - } } else { if (pqPutnchar((char *) args[i].u.ptr, args[i].len, conn)) - { - pqHandleSendFailure(conn); return NULL; - } } } if (pqPutMsgEnd(conn) < 0 || pqFlush(conn)) - { - pqHandleSendFailure(conn); return NULL; - } for (;;) { diff --git a/src/interfaces/libpq/fe-protocol3.c b/src/interfaces/libpq/fe-protocol3.c index 47dbc31..ec51fee 100644 --- a/src/interfaces/libpq/fe-protocol3.c +++ b/src/interfaces/libpq/fe-protocol3.c @@ -1926,50 +1926,35 @@ pqFunctionCall3(PGconn *conn, Oid fnid, pqPutInt(1, 2, conn) < 0 || /* format code: BINARY */ pqPutInt(nargs, 2, conn) < 0) /* # of args */ { - pqHandleSendFailure(conn); + /* error message should be set up already */ return NULL; } for (i = 0; i < nargs; ++i) { /* len.int4 + contents */ if (pqPutInt(args[i].len, 4, conn)) - { - pqHandleSendFailure(conn); return NULL; - } if (args[i].len == -1) continue; /* it's NULL */ if (args[i].isint) { if (pqPutInt(args[i].u.integer, args[i].len, conn)) - { - pqHandleSendFailure(conn); return NULL; - } } else { if (pqPutnchar((char *) args[i].u.ptr, args[i].len, conn)) - { - pqHandleSendFailure(conn); return NULL; - } } } if (pqPutInt(1, 2, conn) < 0) /* result format code: BINARY */ - { - pqHandleSendFailure(conn); return NULL; - } if (pqPutMsgEnd(conn) < 0 || pqFlush(conn)) - { - pqHandleSendFailure(conn); return NULL; - } for (;;) { diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h index 4a93d8e..dbe0f7e 100644 --- a/src/interfaces/libpq/libpq-int.h +++ b/src/interfaces/libpq/libpq-int.h @@ -410,6 +410,8 @@ struct pg_conn bool password_needed; /* true if server demanded a password */ bool sigpipe_so; /* have we masked SIGPIPE via SO_NOSIGPIPE? */ bool sigpipe_flag; /* can we mask SIGPIPE via MSG_NOSIGNAL? */ + bool write_failed; /* have we had a write failure on sock? */ + char *write_err_msg; /* write error message, or NULL if OOM */ /* Transient state needed while establishing connection */ bool try_next_addr; /* time to advance to next address/host? */ @@ -585,7 +587,6 @@ extern void pqSaveMessageField(PGresult *res, char code, extern void pqSaveParameterStatus(PGconn *conn, const char *name, const char *value); extern int pqRowProcessor(PGconn *conn, const char **errmsgp); -extern void pqHandleSendFailure(PGconn *conn); /* === in fe-protocol2.c === */