Re: libpq: Process buffered SSL read bytes to support records >8kB on async API - Mailing list pgsql-hackers

From Peter Eisentraut
Subject Re: libpq: Process buffered SSL read bytes to support records >8kB on async API
Date
Msg-id 34844e8c-267c-4daf-b1e0-f26059a4a7d3@eisentraut.org
Whole thread
Responses Re: libpq: Process buffered SSL read bytes to support records >8kB on async API
List pgsql-hackers
On 07.07.26 17:58, Heikki Linnakangas wrote:
> On 03/07/2026 12:25, Heikki Linnakangas wrote:
>> On 14/05/2026 07:32, solai v wrote:
>>> Hi,
>>> i tested the latest V3 patch series on current postgreSQL HEAD with 
>>> SSL enabled.The patches applied cleanly,build succeeded,and the 
>>> server started successfully.I ran the async libpq test repeatedly and 
>>> also tested large COPY TO STDOUT operations over SSL.I did not 
>>> observe hangs,crashes,or visible regressions during testing.
>>
>> Thanks for the testing!
>>
>> Here's another patch version, with some small comment and commit 
>> message changes, and I split the changes slightly differently between 
>> the two patches. End result is otherwise the same.
>>
>> I plan to commit this in the next few days.
> 
> Committed and backpatched to all supported versions. Thanks everyone!

In pqDrainPending(), there is

     nread = pqsecure_read(conn, conn->inBuffer + conn->inEnd,
                           bytes_pending);
     conn->inEnd += nread;

But pqsecure_read() can return -1 for error.  So adding that to 
conn->inEnd at that point seems wrong.

There is error handling in the following code, but this would still kind 
of corrupt the conn->inEnd value?

     /* When there are bytes pending, the read function is not supposed 
to fail */
     if (nread != bytes_pending)
     {
         libpq_append_conn_error(conn,
                                 "drained only %zu of %zd pending bytes 
in transport buffer",
                                 nread, bytes_pending);
         return -1;
     }

I'm not sure if that comment means that a -1 return cannot happen?  Or 
just that the whole error check should not happen?

Also note that the format placeholder for nread is wrong.  If you do get 
a -1, it will print some large unsigned value.




pgsql-hackers by date:

Previous
From: Jacob Champion
Date:
Subject: Re: PG20 Minimum Dependency Thread
Next
From: Andrey Borodin
Date:
Subject: Re: injection_points: Switch wait/wakeup to use atomics rather than latches