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

2015-04-09 Thread Jan Urbański
Peter Eisentraut writes: On 2/12/15 7:28 AM, Jan Urbański wrote: +#if OPENSSL_VERSION_NUMBER 0x1000 +/* OpenSSL 1.0.0 deprecates the CRYPTO_set_id_callback function and provides a + * default implementation, so there's no need for our own. */ I have some additional concerns about

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

2015-04-09 Thread Peter Eisentraut
On 2/12/15 7:28 AM, Jan Urbański wrote: +#if OPENSSL_VERSION_NUMBER 0x1000 +/* OpenSSL 1.0.0 deprecates the CRYPTO_set_id_callback function and provides a + * default implementation, so there's no need for our own. */ I have some additional concerns about this. It is true that OpenSSL

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

2015-04-07 Thread Peter Eisentraut
On 4/3/15 7:44 AM, Jan Urbański wrote: To reiterate: I recognise that not handling the callbacks is not the right answer. But not stomping on callbacks others might have set sounds like a minimal and safe improvement. I think your patch is okay in that it is not a good idea to overwrite or

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

2015-04-03 Thread Jan Urbański
Peter Eisentraut writes: On 4/2/15 4:32 AM, Jan Urbański wrote: Peter Eisentraut writes: 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.5020...@vmware.com), namely that another thread

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

2015-04-02 Thread Jan Urbański
Peter Eisentraut writes: On 2/12/15 7:28 AM, Jan Urbański wrote: 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

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

2015-04-02 Thread Peter Eisentraut
On 4/2/15 4:32 AM, Jan Urbański wrote: Peter Eisentraut writes: On 2/12/15 7:28 AM, Jan Urbański wrote: 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

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

2015-03-31 Thread Peter Eisentraut
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

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

2015-02-12 Thread Jan Urbański
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

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

2015-02-12 Thread Jan Urbański
Andres Freund writes: On 2015-02-09 18:17:14 +0100, Jan Urbański wrote: First of all, the current behaviour is crazy. We're setting and unsetting the locking callback every time a connection is made/closed, which is not how OpenSSL is supposed to be used. The *application* using libpq should

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

2015-02-12 Thread Andres Freund
On 2015-02-12 09:31:27 +0100, Jan Urbański wrote: Andres Freund writes: On 2015-02-09 18:17:14 +0100, Jan Urbański wrote: First of all, the current behaviour is crazy. We're setting and unsetting the locking callback every time a connection is made/closed, which is not how OpenSSL

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

2015-02-12 Thread Jan Urbański
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

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

2015-02-11 Thread Jan Urbański
Jan Urbański writes: I did some more digging on bug http://www.postgresql.org/message-id/cahul3dpwyfnugdgo95ohydq4kugdnbkptjq0mnbtubhcmg4...@mail.gmail.com which describes a deadlock when using libpq with SSL in a multi-threaded environment with other threads doing SSL independently.

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

2015-02-11 Thread Andres Freund
On 2015-02-09 18:17:14 +0100, Jan Urbański wrote: We added unsetting the locking callback in 4e816286533dd34c10b368487d4079595a3e1418 due to this bug report: http://www.postgresql.org/message-id/48620925.6070...@pws.com.au Indeed, commenting out the CRYPTO_set_locking_callback(NULL) call in

[HACKERS] libpq's multi-threaded SSL callback handling is busted

2015-02-09 Thread Jan Urbański
I did some more digging on bug http://www.postgresql.org/message-id/cahul3dpwyfnugdgo95ohydq4kugdnbkptjq0mnbtubhcmg4...@mail.gmail.com which describes a deadlock when using libpq with SSL in a multi-threaded environment with other threads doing SSL independently. Attached is a reproducing Python