| From: | Peter Eisentraut <peter(at)eisentraut(dot)org> |
|---|---|
| To: | Heikki Linnakangas <hlinnaka(at)iki(dot)fi>, solai v <solai(dot)cdac(at)gmail(dot)com>, Jacob Champion <jacob(dot)champion(at)enterprisedb(dot)com> |
| Cc: | Mark Dilger <mark(dot)dilger(at)enterprisedb(dot)com>, Andres Freund <andres(at)anarazel(dot)de>, Lars Kanis <lars(at)greiz-reinsdorf(dot)de>, pgsql-hackers(at)lists(dot)postgresql(dot)org, Merlin Moncure <mmoncure(at)gmail(dot)com> |
| Subject: | Re: libpq: Process buffered SSL read bytes to support records >8kB on async API |
| Date: | 2026-07-08 19:16:18 |
| Message-ID: | 34844e8c-267c-4daf-b1e0-f26059a4a7d3@eisentraut.org |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | 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.
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Andrey Borodin | 2026-07-08 19:17:20 | Re: injection_points: Switch wait/wakeup to use atomics rather than latches |
| Previous Message | Jacob Champion | 2026-07-08 18:51:06 | Re: PG20 Minimum Dependency Thread |