Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unwatch and close connection on a batch write error #2240

Merged
merged 1 commit into from
Jan 25, 2025

Conversation

bonnefoa
Copy link
Contributor

@bonnefoa bonnefoa commented Jan 24, 2025

On a write error, a conn.Write simply unlock pgconn, leaving the connection as Idle and reusable while the multiResultReader would be closed. From this state, calling multiResultReader.Close won't try to receiveMessage and thus won't unwatch and close the connection since it is already closed. This leaves the connection "open" and the next time it's used, a "Watch already in progress" panic could be triggered.

This patch fixes the issue by unwatching and closing the connection on a batch write error, similar to what's done on a receiveMessage error. The same was done on Sync.Encode error even if the path is unreachable as Sync.Error never returns an error.

Previously, a conn.Write would simply unlock pgconn, leaving the
connection as Idle and reusable while the multiResultReader would be
closed. From this state, calling multiResultReader.Close won't try to
receiveMessage and thus won't unwatch and close the connection since it
is already closed. This leaves the connection "open" and the next time
it's used, a "Watch already in progress" panic could be triggered.

This patch fixes the issue by unwatching and closing the connection on a
batch write error. The same was done on Sync.Encode error even if the
path is unreachable as Sync.Error never returns an error.
@jackc jackc merged commit 1abf7d9 into jackc:master Jan 25, 2025
14 checks passed
@jackc
Copy link
Owner

jackc commented Jan 25, 2025

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants