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

On 2/12/15 7:28 AM, Jan Urbański wrote:
>>>>> * 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...

I don't think this patch would actually fix the problem that was
described after the original bug report
(http://www.postgresql.org/message-id/5436991B.5020708@vmware.com)
namely that another thread acquires a lock while the libpq callbacks are
set and then cannot release the lock if libpq has been shut down in the
meantime.

The only way to fix that is to never unset the callbacks. But we don't
want that or can't do that for other reasons.

I think the only way out is to declare that if there are multiple
threads and other threads might be using OpenSSL not through libpq, then
the callbacks need to be managed outside of libpq.

In environments like PHP or Python this would require some coordination
work across modules somehow, but I don't see a way around that.