From: | Daniel Gustafsson <daniel(at)yesql(dot)se> |
---|---|
To: | Peter Eisentraut <peter(at)eisentraut(dot)org> |
Cc: | PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: libpq OpenSSL and multithreading |
Date: | 2025-07-03 16:01:23 |
Message-ID: | F4DD42A0-6B5E-4349-A93C-F752DF1644BE@yesql.se |
Views: | Whole Thread | Raw Message | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
> On 2 Jul 2025, at 13:44, Peter Eisentraut <peter(at)eisentraut(dot)org> wrote:
>
> On 27.06.25 19:24, Daniel Gustafsson wrote:
>> The OpenSSL code in libpq have two issues for multithreading: the verify_cb
>> callback use a global variable to pass back error detail state and there is one
>> use of strerror().
>
> Slightly misleading title: This is actually about the *backend* libpq code.
Point taken, proposed commitmessage has been updated.
>> The attached fixes both, with no functional change, in order to pave the way
>> for multithreading:
>> * Rather than using a global variable the callback use a new member in the
>> Port struct for passing the string, and the Port struct is in turn passed as
>> private data in the SSL object
>
> Couldn't this also be done by making that global variable thread-local? But getting rid of it is even nicer.
It absolutely could, and I briefly considered this approach, but I prefer
getting rid of it altogether.
>> * The strerror call is replaced with a strerror_r call using the already
>> existing errorbuffer
>
> This one was already discussed some time ago at <https://www.postgresql.org/message-id/flat/daa87d79-c044-46c4-8458-8d77241ed7b0%40eisentraut.org>:
>
> > But the bigger issue is that the use of a static buffer makes
> > this not thread-safe, so having it use strerror_r to fill that
> > buffer is just putting lipstick on a pig.
>
> It looks like your patch doesn't address that?
Doh, of course. The attached v2 takes a stab at moving from using a static
buffer to a palloced buffer with the caller being responsible for freeing. In
paths which lead to proc_exit we can omit freeing.
--
Daniel Gustafsson
Attachment | Content-Type | Size |
---|---|---|
v2-0001-libpq-Make-SSL-errorhandling-in-backend-threadsaf.patch | application/octet-stream | 14.9 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Álvaro Herrera | 2025-07-03 16:07:45 | Re: Inconsistent LSN format in pg_waldump output |
Previous Message | David E. Wheeler | 2025-07-03 15:59:29 | Re: Bloom Filter improvements in postgesql |