Bruce Momjian wrote:
> Thanks for the review, Magnus. I have adjusted the patch to use the
> same mutex every time the counter is accessed, and adjusted the
> pqsecure_destroy() call to properly decrement in the right place.
>
> Also, I renamed the libpq global destroy function to be clearer
> (the function is not exported).
Here is an updated version of the patch to match CVS HEAD.
--
Bruce Momjian <[EMAIL PROTECTED]> http://momjian.us
EnterpriseDB http://enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +
Index: src/backend/libpq/be-secure.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/libpq/be-secure.c,v
retrieving revision 1.86
diff -c -c -r1.86 be-secure.c
*** src/backend/libpq/be-secure.c 20 Nov 2008 09:29:36 -0000 1.86
--- src/backend/libpq/be-secure.c 20 Nov 2008 21:42:24 -0000
***************
*** 88,94 ****
static int verify_cb(int, X509_STORE_CTX *);
static void info_cb(const SSL *ssl, int type, int args);
static void initialize_SSL(void);
- static void destroy_SSL(void);
static int open_server_SSL(Port *);
static void close_SSL(Port *);
static const char *SSLerrmessage(void);
--- 88,93 ----
***************
*** 193,209 ****
}
/*
- * Destroy global context
- */
- void
- secure_destroy(void)
- {
- #ifdef USE_SSL
- destroy_SSL();
- #endif
- }
-
- /*
* Indicate if we have loaded the root CA store to verify certificates
*/
bool
--- 192,197 ----
***************
*** 843,853 ****
}
}
/*
! * Destroy global SSL context.
*/
static void
! destroy_SSL(void)
{
if (SSL_context)
{
--- 831,842 ----
}
}
+ #ifdef NOT_USED
/*
! * Destroy global SSL context
*/
static void
! destroy_global_SSL(void)
{
if (SSL_context)
{
***************
*** 855,860 ****
--- 844,850 ----
SSL_context = NULL;
}
}
+ #endif
/*
* Attempt to negotiate SSL connection.
Index: src/interfaces/libpq/fe-secure.c
===================================================================
RCS file: /cvsroot/pgsql/src/interfaces/libpq/fe-secure.c,v
retrieving revision 1.107
diff -c -c -r1.107 fe-secure.c
*** src/interfaces/libpq/fe-secure.c 13 Nov 2008 09:45:25 -0000 1.107
--- src/interfaces/libpq/fe-secure.c 20 Nov 2008 21:42:25 -0000
***************
*** 44,49 ****
--- 44,50 ----
#endif
#include <arpa/inet.h>
#endif
+
#include <sys/stat.h>
#ifdef ENABLE_THREAD_SAFETY
***************
*** 57,72 ****
#ifdef USE_SSL
#include <openssl/ssl.h>
#include <openssl/bio.h>
#if (SSLEAY_VERSION_NUMBER >= 0x00907000L)
#include <openssl/conf.h>
#endif
#if (SSLEAY_VERSION_NUMBER >= 0x00907000L) && !defined(OPENSSL_NO_ENGINE)
#include <openssl/engine.h>
#endif
- #endif /* USE_SSL */
-
-
- #ifdef USE_SSL
#ifndef WIN32
#define USER_CERT_FILE ".postgresql/postgresql.crt"
--- 58,70 ----
#ifdef USE_SSL
#include <openssl/ssl.h>
#include <openssl/bio.h>
+
#if (SSLEAY_VERSION_NUMBER >= 0x00907000L)
#include <openssl/conf.h>
#endif
#if (SSLEAY_VERSION_NUMBER >= 0x00907000L) && !defined(OPENSSL_NO_ENGINE)
#include <openssl/engine.h>
#endif
#ifndef WIN32
#define USER_CERT_FILE ".postgresql/postgresql.crt"
***************
*** 91,110 ****
static int verify_cb(int ok, X509_STORE_CTX *ctx);
static int client_cert_cb(SSL *, X509 **, EVP_PKEY **);
static int init_ssl_system(PGconn *conn);
static int initialize_SSL(PGconn *);
static void destroy_SSL(void);
static PostgresPollingStatusType open_client_SSL(PGconn *);
static void close_SSL(PGconn *);
static char *SSLerrmessage(void);
static void SSLerrfree(char *buf);
- #endif
- #ifdef USE_SSL
static bool pq_initssllib = true;
-
static SSL_CTX *SSL_context = NULL;
#endif
/*
* Macros to handle disabling and then restoring the state of SIGPIPE handling.
* Note that DISABLE_SIGPIPE() must appear at the start of a block.
--- 89,120 ----
static int verify_cb(int ok, X509_STORE_CTX *ctx);
static int client_cert_cb(SSL *, X509 **, EVP_PKEY **);
static int init_ssl_system(PGconn *conn);
+ static void destroy_ssl_system(void);
static int initialize_SSL(PGconn *);
static void destroy_SSL(void);
static PostgresPollingStatusType open_client_SSL(PGconn *);
static void close_SSL(PGconn *);
static char *SSLerrmessage(void);
static void SSLerrfree(char *buf);
static bool pq_initssllib = true;
static SSL_CTX *SSL_context = NULL;
+
+ #ifdef ENABLE_THREAD_SAFETY
+ static int ssl_open_connections = 0;
+
+ #ifndef WIN32
+ static pthread_mutex_t ssl_config_mutex = PTHREAD_MUTEX_INITIALIZER;
+ #else
+ static pthread_mutex_t ssl_config_mutex = NULL;
+ static long win32_ssl_create_mutex = 0;
#endif
+ #endif /* ENABLE_THREAD_SAFETY */
+
+ #endif /* SSL */
+
+
/*
* Macros to handle disabling and then restoring the state of SIGPIPE handling.
* Note that DISABLE_SIGPIPE() must appear at the start of a block.
***************
*** 725,770 ****
init_ssl_system(PGconn *conn)
{
#ifdef ENABLE_THREAD_SAFETY
! #ifndef WIN32
! static pthread_mutex_t init_mutex = PTHREAD_MUTEX_INITIALIZER;
! #else
! static pthread_mutex_t init_mutex = NULL;
! static long mutex_initlock = 0;
!
! if (init_mutex == NULL)
{
! while (InterlockedExchange(&mutex_initlock, 1) == 1)
/* loop, another thread own the lock */ ;
! if (init_mutex == NULL)
{
! if (pthread_mutex_init(&init_mutex, NULL))
return -1;
}
! InterlockedExchange(&mutex_initlock, 0);
}
#endif
! if (pthread_mutex_lock(&init_mutex))
return -1;
! if (pq_initssllib && pq_lockarray == NULL)
{
! int i;
!
! CRYPTO_set_id_callback(pq_threadidcallback);
!
! pq_lockarray = malloc(sizeof(pthread_mutex_t) * CRYPTO_num_locks());
! if (!pq_lockarray)
{
! pthread_mutex_unlock(&init_mutex);
! return -1;
}
! for (i = 0; i < CRYPTO_num_locks(); i++)
{
! if (pthread_mutex_init(&pq_lockarray[i], NULL))
! return -1;
}
-
- CRYPTO_set_locking_callback(pq_lockingcallback);
}
#endif
if (!SSL_context)
--- 735,786 ----
init_ssl_system(PGconn *conn)
{
#ifdef ENABLE_THREAD_SAFETY
! #ifdef WIN32
! if (ssl_config_mutex == NULL)
{
! while (InterlockedExchange(&win32_ssl_create_mutex, 1) == 1)
/* loop, another thread own the lock */ ;
! if (ssl_config_mutex == NULL)
{
! if (pthread_mutex_init(&ssl_config_mutex, NULL))
return -1;
}
! InterlockedExchange(&win32_ssl_create_mutex, 0);
}
#endif
! if (pthread_mutex_lock(&ssl_config_mutex))
return -1;
! if (pq_initssllib)
{
! if (pq_lockarray == NULL)
{
! int i;
!
! pq_lockarray = malloc(sizeof(pthread_mutex_t) * CRYPTO_num_locks());
! if (!pq_lockarray)
! {
! pthread_mutex_unlock(&ssl_config_mutex);
! return -1;
! }
! for (i = 0; i < CRYPTO_num_locks(); i++)
! {
! if (pthread_mutex_init(&pq_lockarray[i], NULL))
! {
! free(pq_lockarray);
! pq_lockarray = NULL;
! pthread_mutex_unlock(&ssl_config_mutex);
! return -1;
! }
! }
}
!
! if (ssl_open_connections++ == 0)
{
! /* These are only required for threaded SSL applications */
! CRYPTO_set_id_callback(pq_threadidcallback);
! CRYPTO_set_locking_callback(pq_lockingcallback);
}
}
#endif
if (!SSL_context)
***************
*** 787,804 ****
err);
SSLerrfree(err);
#ifdef ENABLE_THREAD_SAFETY
! pthread_mutex_unlock(&init_mutex);
#endif
return -1;
}
}
#ifdef ENABLE_THREAD_SAFETY
! pthread_mutex_unlock(&init_mutex);
#endif
return 0;
}
/*
* Initialize global SSL context.
*/
static int
--- 803,863 ----
err);
SSLerrfree(err);
#ifdef ENABLE_THREAD_SAFETY
! pthread_mutex_unlock(&ssl_config_mutex);
#endif
return -1;
}
}
#ifdef ENABLE_THREAD_SAFETY
! pthread_mutex_unlock(&ssl_config_mutex);
#endif
return 0;
}
/*
+ * This function is needed because if the libpq library is unloaded
+ * from the application, the callback functions will no longer exist when
+ * SSL used by other parts of the system. For this reason,
+ * we unregister the SSL callback functions when the last libpq
+ * connection is closed.
+ */
+ static void
+ destroy_ssl_system(void)
+ {
+ #ifdef ENABLE_THREAD_SAFETY
+ /* Assume mutex is already created */
+ if (pthread_mutex_lock(&ssl_config_mutex))
+ return;
+
+ if (pq_initssllib)
+ {
+ /*
+ * We never free pq_lockarray, which means we leak memory on
+ * repeated loading/unloading of this library.
+ */
+
+ if (ssl_open_connections > 0)
+ --ssl_open_connections;
+
+ if (ssl_open_connections == 0)
+ {
+ /*
+ * We need to unregister the SSL callbacks on last connection
+ * close because the libpq shared library might be unloaded,
+ * and once it is, callbacks must be removed to prevent them
+ * from being called by other SSL code.
+ */
+ CRYPTO_set_locking_callback(NULL);
+ CRYPTO_set_id_callback(NULL);
+ }
+ }
+
+ pthread_mutex_unlock(&ssl_config_mutex);
+ #endif
+ return;
+ }
+
+ /*
* Initialize global SSL context.
*/
static int
***************
*** 886,896 ****
return 0;
}
/*
! * Destroy global SSL context.
*/
static void
! destroy_SSL(void)
{
if (SSL_context)
{
--- 945,962 ----
return 0;
}
+ static void
+ destroy_SSL(void)
+ {
+ destroy_ssl_system();
+ }
+
+ #ifdef NOT_USED
/*
! * Destroy global SSL context
*/
static void
! destroy_global_SSL(void)
{
if (SSL_context)
{
***************
*** 898,903 ****
--- 964,970 ----
SSL_context = NULL;
}
}
+ #endif
/*
* Attempt to negotiate SSL connection.
***************
*** 1015,1020 ****
--- 1082,1088 ----
SSL_shutdown(conn->ssl);
SSL_free(conn->ssl);
conn->ssl = NULL;
+ pqsecure_destroy();
/* We have to assume we got EPIPE */
REMEMBER_EPIPE(true);
RESTORE_SIGPIPE();
--
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
