Re: libpq's multi-threaded SSL callback handling is busted

Jan Urbański writes:

> Andres Freund writes:
>
>> On 2015-02-12 09:31:27 +0100, Jan Urbański wrote:
>>> That doesn't solve the problem of the Python deadlock, where you're not at
>>> leisure to call a C function at the beginning of your module.
>>
>> We could just never unload the hooks...
>
> That's what we did before 4e816286533dd34c10b368487d4079595a3e1418 :) And it
> got changed after http://www.postgresql.org/message-id/48620925.6070806@pws.com.au
>
>>
>>> > * If there's already callbacks set: Remember that fact and don't
>>> > overwrite. In the next major version: warn.
>>>
>>> So yeah, that was my initial approach - check if callbacks are set, don't do
>>> the dance if they are. It felt like a crutch, though, and racy at that. There's
>>> no atomic way to test-and-set those callbacks. The window for racyness is
>>> small, though.
>>
>> If you do that check during library initialization instead of every
>> connection it shouldn't be racy - if that part is run in a multithreaded
>> fashion you're doing something crazy.
>
> Yes, that's true. The problem is that there's no real libpq initialisation
> function. The docs say that:
>
> "If your application initializes libssl and/or libcrypto libraries and libpq is
> built with SSL support, you should call PQinitOpenSSL"
>
> So most apps will just not bother. The moment you know you'll need SSL is only
> when you get an 'S' message from the server...

For the sake of discussion, here's a patch to prevent stomping on
previously-set callbacks, racy as it looks.

FWIW, it does fix the Python deadlock and doesn't cause the PHP segfault...

J