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.6070...@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
diff --git a/src/interfaces/libpq/fe-secure-openssl.c b/src/interfaces/libpq/fe-secure-openssl.c
index a32af34..54312a5 100644
--- a/src/interfaces/libpq/fe-secure-openssl.c
+++ b/src/interfaces/libpq/fe-secure-openssl.c
@@ -710,6 +710,9 @@ verify_peer_name_matches_certificate(PGconn *conn)
* Callback functions for OpenSSL internal locking
*/
+#if OPENSSL_VERSION_NUMBER < 0x10000000
+/* OpenSSL 1.0.0 deprecates the CRYPTO_set_id_callback function and provides a
+ * default implementation, so there's no need for our own. */
static unsigned long
pq_threadidcallback(void)
{
@@ -720,6 +723,7 @@ pq_threadidcallback(void)
*/
return (unsigned long) pthread_self();
}
+#endif /* OPENSSL_VERSION_NUMBER < 0x10000000 */
static pthread_mutex_t *pq_lockarray;
@@ -806,9 +810,14 @@ pgtls_init(PGconn *conn)
if (ssl_open_connections++ == 0)
{
- /* These are only required for threaded libcrypto applications */
- CRYPTO_set_id_callback(pq_threadidcallback);
- CRYPTO_set_locking_callback(pq_lockingcallback);
+ /* These are only required for threaded libcrypto applications, but
+ * make sure we don't stomp on them if they're already set. */
+#if OPENSSL_VERSION_NUMBER < 0x10000000
+ if (CRYPTO_get_id_callback() == NULL)
+ CRYPTO_set_id_callback(pq_threadidcallback);
+#endif
+ if (CRYPTO_get_locking_callback() == NULL)
+ CRYPTO_set_locking_callback(pq_lockingcallback);
}
}
#endif /* ENABLE_THREAD_SAFETY */
@@ -885,9 +894,14 @@ destroy_ssl_system(void)
if (pq_init_crypto_lib && ssl_open_connections == 0)
{
- /* No connections left, unregister libcrypto callbacks */
- CRYPTO_set_locking_callback(NULL);
- CRYPTO_set_id_callback(NULL);
+ /* No connections left, unregister libcrypto callbacks, if no one
+ * registered different ones in the meantime. */
+#if OPENSSL_VERSION_NUMBER < 0x10000000
+ if (CRYPTO_get_id_callback() == pq_threadidcallback)
+ CRYPTO_set_id_callback(NULL);
+#endif
+ if (CRYPTO_get_locking_callback() == pq_lockingcallback)
+ CRYPTO_set_locking_callback(NULL);
/*
* We don't free the lock array or the SSL_context. If we get another
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
