Repository navigation
Report dead connection when libpq has given up on it (#224) - #225
Open
davecramer wants to merge 1 commit into
Open
davecramer wants to merge 1 commit into
davecramer wants to merge 1 commit into
Conversation
System.Data.Odbc closes an OdbcConnection only when SQL_ATTR_CONNECTION_DEAD returns SQL_CD_TRUE after a failed call. The driver answered from conn->status alone, so a connection that libpq had already marked CONNECTION_BAD was reported alive until some code path called CC_on_abort(). One such path: when PQsendQuery() failed, CC_send_query_append() set the error but left the connection marked up, so every later query failed while the application kept the connection open. SQL_ATTR_CONNECTION_DEAD now also returns true when PQstatus() is CONNECTION_BAD, and a PQsendQuery() failure on a CONNECTION_BAD connection now gives up the connection. New connection-dead test terminates the test connection's backend from a second connection and checks SQL_ATTR_CONNECTION_DEAD before and after the next queries fail. Assisted-By: Claude
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Refs #224.
System.Data.Odbc closes an
OdbcConnectionafter a failed call only whenSQL_ATTR_CONNECTION_DEADreturnsSQL_CD_TRUE. Otherwise it throws the driver's error and leavesStateatOpen, which is what #224 reports.Changes
SQL_ATTR_CONNECTION_DEAD, in bothPGAPI_GetConnectAttrandPGAPI_GetConnectOption, now also returns true whenPQstatus()isCONNECTION_BAD. Before this, the answer came fromconn->statusalone. A connection libpq had already given up on was reported alive until some code path calledCC_on_abort().PQsendQuery()fails inCC_send_query_append()andPQstatus()isCONNECTION_BAD, the driver now callsCC_on_abort(conn, CONN_DEAD). Before this, it set the error and kept the connection marked up, so every later query failed while the application kept the connection open.What this does not establish
This PR doesn't confirm the root cause of scenario 2 in #224. Both messages in that report, "The connection is down" and "SQLExecDirectW unable due to the connection lost", appear only when
conn->pqconnis NULL. After connect, the only code that clearspqconnisCC_on_abort(CONN_DEAD), which setsCONN_DOWNfirst, soSQL_ATTR_CONNECTION_DEADshould already have answered true. Telling which case the reporter hit needs a MyLog from a failing run, which records the status behind eachSQL_ATTR_CONNECTION_DEADanswer.Test
connection-deadterminates the test connection's backend from a second connection. It then checksSQL_ATTR_CONNECTION_DEADbefore termination and after each of two failing queries. It passes with and without the driver change, because a terminated backend is detected throughPQgetResult(), a path that already calledCC_on_abort(). So it guards the dead-connection answer against regressions rather than reproducing the change. ThePQsendQuery()failure path depends on socket timing and isn't exercised.The full suite (68 tests) passes against PostgreSQL 18 on macOS.