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.
A single Connect("connstring") may actually make multiple connection
requests due to TLS or HA configuration. Previously, when all attempts
failed only the last error was returned. This could be confusing.
Now details of all failed attempts are included.
For example, the following connection string:
host=localhost,127.0.0.1,foo.invalid port=1,2,3
Will now return an error like the following:
failed to connect to `user=postgres database=pgx_test`:
lookup foo.invalid: no such host
[::1]:1 (localhost): dial error: dial tcp [::1]:1: connect: connection refused
127.0.0.1:1 (localhost): dial error: dial tcp 127.0.0.1:1: connect: connection refused
127.0.0.1:2 (127.0.0.1): dial error: dial tcp 127.0.0.1:2: connect: connection refused
https://github.com/jackc/pgx/issues/1929
The PostgreSQL server will reject messages greater than ~1 GB anyway.
However, worse than that is that a message that is larger than 4 GB
could wrap the 32-bit integer message size and be interpreted by the
server as multiple messages. This could allow a malicious client to
inject arbitrary protocol messages.
https://github.com/jackc/pgx/security/advisories/GHSA-mrww-27vc-gghv
Otherwise, it might be possible to panic when closing the pipeline if it
tries to read a connection that should be closed but still has a fatal
error on the wire.
https://github.com/jackc/pgx/issues/1920
OnPGError is called on every error response received from Postgres and can
be used to close connections on specific errors. Defaults to closing on
FATAL-severity errors.
Fixes#1803
stdlib_test.TestConnConcurrency had been flickering on CI deadline /
timeout errors. This was extremely confusing because the test deadline
was set for 2 minutes and the errors would occur much quicker.
The problem only manifested in an extremely specific and timing
sensitive situation.
1. The watchdog timer for deadlocked writes starts the goroutine to
start the background reader
2. The background reader is stopped
3. The next operation is a read without a preceding write (AFAIK only
CheckConn does this)
4. The deadline is set to interrupt the read
5. The goroutine from 1 actually starts the background reader
6. The background reader gets an error reading the connection with the
deadline
7. The deadline is cleared
8. The next read on the connection will get the timeout error
Felix Röhrich
Anthonin Bonnefoy
Jack Christensen
zenkovev
EinoPlasma
Oleksandr Redko
zenkovev
Alexander Rumyantsev
Kevin Biju
Yann Soubeyrand
Hans-Joachim Kliemeck
Oleksandr Redko
vahid-sohrabloo
James Hartig
Ryan Fowler
Samuel Stauffer
Simon Paredes
Ivan Posazhennikov
Anton Levakin