Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 0214a61

Browse files
committed
Fix potential deadlock with libpq non-blocking mode.
If libpq output buffer is full, pqSendSome() function tries to drain any incoming data. This avoids deadlock, if the server e.g. sends a lot of NOTICE messages, and blocks until we read them. However, pqSendSome() only did that in blocking mode. In non-blocking mode, the deadlock could still happen. To fix, take a two-pronged approach: 1. Change the documentation to instruct that when PQflush() returns 1, you should wait for both read- and write-ready, and call PQconsumeInput() if it becomes read-ready. That fixes the deadlock, but applications are not going to change overnight. 2. In pqSendSome(), drain the input buffer before returning 1. This alleviates the problem for applications that only wait for write-ready. In particular, a slow but steady stream of NOTICE messages during COPY FROM STDIN will no longer cause a deadlock. The risk remains that the server attempts to send a large burst of data and fills its output buffer, and at the same time the client also sends enough data to fill its output buffer. The application will deadlock if it goes to sleep, waiting for the socket to become write-ready, before the server's data arrives. In practice, NOTICE messages and such that the server might be sending are usually short, so it's highly unlikely that the server would fill its output buffer so quickly. Backpatch to all supported versions.
1 parent 9c15a77 commit 0214a61

File tree

2 files changed

+25
-11
lines changed

2 files changed

+25
-11
lines changed

doc/src/sgml/libpq.sgml

+8-1
Original file line numberDiff line numberDiff line change
@@ -4380,7 +4380,14 @@ int PQflush(PGconn *conn);
43804380
<para>
43814381
After sending any command or data on a nonblocking connection, call
43824382
<function>PQflush</function>. If it returns 1, wait for the socket
4383-
to be write-ready and call it again; repeat until it returns 0. Once
4383+
to become read- or write-ready. If it becomes write-ready, call
4384+
<function>PQflush</function> again. If it becomes read-ready, call
4385+
<function>PQconsumeInput</function>, then call
4386+
<function>PQflush</function> again. Repeat until
4387+
<function>PQflush</function> returns 0. (It is necessary to check for
4388+
read-ready and drain the input with <function>PQconsumeInput</function>,
4389+
because the server can block trying to send us data, e.g. NOTICE
4390+
messages, and won't read our data until we read its.) Once
43844391
<function>PQflush</function> returns 0, wait for the socket to be
43854392
read-ready and then read the response as described above.
43864393
</para>

src/interfaces/libpq/fe-misc.c

+17-10
Original file line numberDiff line numberDiff line change
@@ -904,16 +904,6 @@ pqSendSome(PGconn *conn, int len)
904904
/*
905905
* We didn't send it all, wait till we can send more.
906906
*
907-
* If the connection is in non-blocking mode we don't wait, but
908-
* return 1 to indicate that data is still pending.
909-
*/
910-
if (pqIsnonblocking(conn))
911-
{
912-
result = 1;
913-
break;
914-
}
915-
916-
/*
917907
* There are scenarios in which we can't send data because the
918908
* communications channel is full, but we cannot expect the server
919909
* to clear the channel eventually because it's blocked trying to
@@ -924,12 +914,29 @@ pqSendSome(PGconn *conn, int len)
924914
* again. Furthermore, it is possible that such incoming data
925915
* might not arrive until after we've gone to sleep. Therefore,
926916
* we wait for either read ready or write ready.
917+
*
918+
* In non-blocking mode, we don't wait here directly, but return
919+
* 1 to indicate that data is still pending. The caller should
920+
* wait for both read and write ready conditions, and call
921+
* PQconsumeInput() on read ready, but just in case it doesn't, we
922+
* call pqReadData() ourselves before returning. That's not
923+
* enough if the data has not arrived yet, but it's the best we
924+
* can do, and works pretty well in practice. (The documentation
925+
* used to say that you only need to wait for write-ready, so
926+
* there are still plenty of applications like that out there.)
927927
*/
928928
if (pqReadData(conn) < 0)
929929
{
930930
result = -1; /* error message already set up */
931931
break;
932932
}
933+
934+
if (pqIsnonblocking(conn))
935+
{
936+
result = 1;
937+
break;
938+
}
939+
933940
if (pqWait(TRUE, TRUE, conn))
934941
{
935942
result = -1;

0 commit comments

Comments
 (0)