Skip to content

Commit 335e3ef

Browse files
committed
pgconn: guard receiveMessage against closed connection
Add a status check at the top of PgConn.receiveMessage() to return an error immediately when the connection is already closed. This prevents pipeline mode from calling receiveMessage() on a dead connection when multiple FATAL errors are buffered together. Previously, the fix guarded close(cleanupDone) in the ErrorResponse handler to prevent double-close. This approach is more fundamental: it stops the second FATAL from even being processed by receiveMessage(), which is consistent with how lock() already rejects calls on closed connections. The nil-results check in Pipeline.Close() remains as a safety net for the case where FATAL errors exhaust the request queue without the server ever sending ReadyForQuery.
1 parent f4d3fb0 commit 335e3ef

1 file changed

Lines changed: 13 additions & 17 deletions

File tree

pgconn/pgconn.go

Lines changed: 13 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -584,6 +584,10 @@ func (pgConn *PgConn) peekMessage() (pgproto3.BackendMessage, error) {
584584

585585
// receiveMessage receives a message without setting up context cancellation
586586
func (pgConn *PgConn) receiveMessage() (pgproto3.BackendMessage, error) {
587+
if pgConn.status == connStatusClosed {
588+
return nil, &connLockError{status: "conn closed"}
589+
}
590+
587591
msg, err := pgConn.peekMessage()
588592
if err != nil {
589593
return nil, err
@@ -598,13 +602,9 @@ func (pgConn *PgConn) receiveMessage() (pgproto3.BackendMessage, error) {
598602
case *pgproto3.ErrorResponse:
599603
err := ErrorResponseToPgError(msg)
600604
if pgConn.config.OnPgError != nil && !pgConn.config.OnPgError(pgConn, err) {
601-
// If the connection is already being closed (e.g. by asyncClose), don't close cleanupDone again to avoid
602-
// a panic from closing an already-closed channel.
603-
if pgConn.status != connStatusClosed {
604-
pgConn.status = connStatusClosed
605-
pgConn.conn.Close() // Ignore error as the connection is already broken and there is already an error to return.
606-
close(pgConn.cleanupDone)
607-
}
605+
pgConn.status = connStatusClosed
606+
pgConn.conn.Close() // Ignore error as the connection is already broken and there is already an error to return.
607+
close(pgConn.cleanupDone)
608608
return nil, err
609609
}
610610
case *pgproto3.NoticeResponse:
@@ -1415,11 +1415,9 @@ func (pgConn *PgConn) CopyFrom(ctx context.Context, r io.Reader, sql string) (Co
14151415
// the goroutine. So instead check pgConn.bufferingReceiveErr which will have been set by the signalMessage. If an
14161416
// error is found then forcibly close the connection without sending the Terminate message.
14171417
if err := pgConn.bufferingReceiveErr; err != nil {
1418-
if pgConn.status != connStatusClosed {
1419-
pgConn.status = connStatusClosed
1420-
pgConn.conn.Close()
1421-
close(pgConn.cleanupDone)
1422-
}
1418+
pgConn.status = connStatusClosed
1419+
pgConn.conn.Close()
1420+
close(pgConn.cleanupDone)
14231421
return CommandTag{}, normalizeTimeoutError(ctx, err)
14241422
}
14251423
// peekMessage never returns err in the bufferingReceive mode - it only forwards the bufferingReceive variables.
@@ -2836,11 +2834,9 @@ func (p *Pipeline) Close() error {
28362834
break
28372835
}
28382836
} else if results == nil {
2839-
// getResults returns (nil, nil) when the request queue is exhausted (pipelineNil) but
2840-
// ExpectedReadyForQuery is still > 0. This happens when the server sends FATAL errors that
2841-
// consume queued request slots (e.g. Prepare, Sync) without ever sending ReadyForQuery --
2842-
// so expectedReadyForQueryCount is never decremented. Without this check the loop would spin
2843-
// indefinitely because there are no more requests to issue and no ReadyForQuery to receive.
2837+
// getResults returns (nil, nil) when the request queue is exhausted but
2838+
// ExpectedReadyForQuery is still > 0. This can happen when FATAL errors consume
2839+
// queued request slots without the server ever sending ReadyForQuery.
28442840
p.conn.asyncClose()
28452841
if p.err == nil {
28462842
p.err = errors.New("pipeline: no more results but expected ReadyForQuery")

0 commit comments

Comments
 (0)