Thread: libpq thread safety
Hi, I've searched through libpq and looked for global or static variables as indicators of non-threadsafe code. I found: - Win32 and BeOS: there is a global "ioctlsocket_ret variable, but it seems to be a dummy variable that is always discarded. - pg_krb4_init(): Are the kerberos libraries thread safe? Additionally, setting init_done is racy. - pg_krb4_authname(): uses a static buffer. - kerberos 5: Is the library thread safe? the initialization could run twice, I'm not sure if that's intentional. - pg_krb4_authname(): relies on the global variable pg_krb5_name. - PQoidStatus: uses a static buffer. - libpq_gettext: setting already_bound is racy. - openssl: According to http://www.openssl.org/docs/crypto/threads.html libpq must register locking callbacks within openssl, otherwise there will be random corruptions. Additionally the SSL_context initialization is not properly synchronized, and SSLerrmessage relies on a static buffer. PQoidStatus is already documented as not thread safe, but what about OpenSSL and kerberos? It seems openssl needs support with callbacks, and according to google searches MIT kerberos 5 is not thread safe, and libpq must use mutexes to prevent concurrent calls into the kerberos library. -- Manfred
Manfred Spraul wrote: > Hi, > > I've searched through libpq and looked for global or static variables as > indicators of non-threadsafe code. I found: > - Win32 and BeOS: there is a global "ioctlsocket_ret variable, but it > seems to be a dummy variable that is always discarded. Right, and it is moving into a compatibility function in 7.5 where it will be a local function variable. > - pg_krb4_init(): Are the kerberos libraries thread safe? Additionally, > setting init_done is racy. No idea. > - pg_krb4_authname(): uses a static buffer. > - kerberos 5: Is the library thread safe? the initialization could run > twice, I'm not sure if that's intentional. > - pg_krb4_authname(): relies on the global variable pg_krb5_name. Seems kerberos isn't. > - PQoidStatus: uses a static buffer. Yes, known documented problem. > - libpq_gettext: setting already_bound is racy. Does that happen in different threads? > - openssl: According to > http://www.openssl.org/docs/crypto/threads.html > libpq must register locking callbacks within openssl, otherwise there > will be random corruptions. Additionally the SSL_context initialization > is not properly synchronized, and SSLerrmessage relies on a static buffer. Oh. > PQoidStatus is already documented as not thread safe, but what about > OpenSSL and kerberos? It seems openssl needs support with callbacks, and > according to google searches MIT kerberos 5 is not thread safe, and > libpq must use mutexes to prevent concurrent calls into the kerberos > library. Oh, seems like a TODO here. We already know how to do thread locking in port/thread.c so maybe we just need to add some locks in there. -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001+ If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania19073
Bruce Momjian wrote: > Manfred Spraul wrote: > > Hi, > > > > I've searched through libpq and looked for global or static variables as > > indicators of non-threadsafe code. I found: > > - Win32 and BeOS: there is a global "ioctlsocket_ret variable, but it > > seems to be a dummy variable that is always discarded. > > Right, and it is moving into a compatibility function in 7.5 where it > will be a local function variable. Done. > > > - pg_krb4_init(): Are the kerberos libraries thread safe? Additionally, > > setting init_done is racy. > > No idea. > > > - pg_krb4_authname(): uses a static buffer. > > - kerberos 5: Is the library thread safe? the initialization could run > > twice, I'm not sure if that's intentional. > > - pg_krb4_authname(): relies on the global variable pg_krb5_name. > > Seems kerberos isn't. > > > - PQoidStatus: uses a static buffer. > > Yes, known documented problem. > > > - libpq_gettext: setting already_bound is racy. > > Does that happen in different threads? > > > - openssl: According to > > http://www.openssl.org/docs/crypto/threads.html > > libpq must register locking callbacks within openssl, otherwise there > > will be random corruptions. Additionally the SSL_context initialization > > is not properly synchronized, and SSLerrmessage relies on a static buffer. > > Oh. > > > PQoidStatus is already documented as not thread safe, but what about > > OpenSSL and kerberos? It seems openssl needs support with callbacks, and > > according to google searches MIT kerberos 5 is not thread safe, and > > libpq must use mutexes to prevent concurrent calls into the kerberos > > library. > > Oh, seems like a TODO here. We already know how to do thread locking in > port/thread.c so maybe we just need to add some locks in there. What killed the idea of doing ssl or kerberos locking inside libpq was that there was no way to be sure that outside code didn't also access those routines. I have documented that SSL and Kerberos are not thread-safe in the libpq docs. Let's wait and see If we need additional work in this area. -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001+ If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania19073
Bruce Momjian wrote:
>What killed the idea of doing ssl or kerberos locking inside libpq was
>that there was no way to be sure that outside code didn't also access
>those routines.
>
A callback based implementation can handle that: libpq has a default
implementation for apps that do not use openssl or kerberos themself. If
the app wants to use the libraries, too, then it must replace the hooks
with their own locks.
I've attached a simple proposal, just for kerberos 4. If you agree on
the general approach, I'll add it to all functions that are not thread safe.
> I have documented that SSL and Kerberos are not
>thread-safe in the libpq docs. Let's wait and see If we need additional
>work in this area.
>
>
It means that multithreading is not usable: As Tom explained, the
connect string is often set directly by the end user. Setting "sslmode"
would result is races - impossible to support. In the very least,
sslmode and Kerberos would have to fail if the app is multithreaded.
--
Manfred
Index: src/interfaces/libpq/fe-auth.c
===================================================================
RCS file: /projects/cvsroot/pgsql-server/src/interfaces/libpq/fe-auth.c,v
retrieving revision 1.89
diff -u -r1.89 fe-auth.c
--- src/interfaces/libpq/fe-auth.c 7 Jan 2004 18:56:29 -0000 1.89
+++ src/interfaces/libpq/fe-auth.c 12 Mar 2004 20:07:02 -0000
@@ -590,6 +590,7 @@
case AUTH_REQ_KRB4:
#ifdef KRB4
+ pglock_thread();
if (pg_krb4_sendauth(PQerrormsg, conn->sock,
(struct sockaddr_in *) & conn->laddr.addr,
(struct sockaddr_in *) & conn->raddr.addr,
@@ -597,8 +598,10 @@
{
snprintf(PQerrormsg, PQERRORMSG_LENGTH,
libpq_gettext("Kerberos 4 authentication failed\n"));
+ pgunlock_thread();
return STATUS_ERROR;
}
+ pgunlock_thread();
break;
#else
snprintf(PQerrormsg, PQERRORMSG_LENGTH,
@@ -722,6 +725,7 @@
if (authsvc == 0)
return NULL; /* leave original error message in place */
+ pglock_thread();
#ifdef KRB4
if (authsvc == STARTUP_KRB4_MSG)
name = pg_krb4_authname(PQerrormsg);
@@ -759,5 +763,6 @@
if (name && (authn = (char *) malloc(strlen(name) + 1)))
strcpy(authn, name);
+ pgunlock_thread();
return authn;
}
Index: src/interfaces/libpq/fe-connect.c
===================================================================
RCS file: /projects/cvsroot/pgsql-server/src/interfaces/libpq/fe-connect.c,v
retrieving revision 1.268
diff -u -r1.268 fe-connect.c
--- src/interfaces/libpq/fe-connect.c 10 Mar 2004 21:12:47 -0000 1.268
+++ src/interfaces/libpq/fe-connect.c 12 Mar 2004 20:07:03 -0000
@@ -3163,4 +3163,34 @@
#undef LINELEN
}
+/*
+ * To keep the API consistent, the locking stubs are always provided, even
+ * if they are not required.
+ */
+pgthreadlock_t *g_threadlock;
+static pgthreadlock_t default_threadlock;
+static void
+default_threadlock(bool acquire)
+{
+#if defined(ENABLE_THREAD_SAFETY)
+ static pthread_mutex_t singlethread_lock = PTHREAD_MUTEX_INITIALIZER;
+ if (acquire)
+ pthread_mutex_lock(&singlethread_lock);
+ else
+ pthread_mutex_unlock(&singlethread_lock);
+#endif
+}
+
+pgthreadlock_t *
+PQregisterThreadLock(pgthreadlock_t *newhandler)
+{
+ pgthreadlock_t *prev;
+
+ prev = g_threadlock;
+ if (newhandler)
+ g_threadlock = newhandler;
+ else
+ g_threadlock = default_threadlock;
+ return prev;
+}
Index: src/interfaces/libpq/libpq-fe.h
===================================================================
RCS file: /projects/cvsroot/pgsql-server/src/interfaces/libpq/libpq-fe.h,v
retrieving revision 1.102
diff -u -r1.102 libpq-fe.h
--- src/interfaces/libpq/libpq-fe.h 9 Jan 2004 02:02:43 -0000 1.102
+++ src/interfaces/libpq/libpq-fe.h 12 Mar 2004 20:07:03 -0000
@@ -274,6 +274,22 @@
PQnoticeProcessor proc,
void *arg);
+typedef void (pgsigpipehandler_t)(bool enable, void **state);
+
+extern pgsigpipehandler_t *
+PQregisterSigpipeCallback(pgsigpipehandler_t *newhandler);
+
+/*
+ * Used to set callback that prevents concurrent access to
+ * non-thread safe functions that libpq needs.
+ * The default implementation uses a libpq internal mutex.
+ * Only required for multithreaded apps that use kerberos
+ * both within their app and for postgresql connections.
+ */
+typedef void (pgthreadlock_t)(bool acquire);
+
+extern pgthreadlock_t * PQregisterThreadLock(pgthreadlock_t *newhandler);
+
/* === in fe-exec.c === */
/* Simple synchronous query */
Index: src/interfaces/libpq/libpq-int.h
===================================================================
RCS file: /projects/cvsroot/pgsql-server/src/interfaces/libpq/libpq-int.h,v
retrieving revision 1.85
diff -u -r1.85 libpq-int.h
--- src/interfaces/libpq/libpq-int.h 5 Mar 2004 01:53:59 -0000 1.85
+++ src/interfaces/libpq/libpq-int.h 12 Mar 2004 20:07:03 -0000
@@ -359,6 +359,16 @@
extern int pqPacketSend(PGconn *conn, char pack_type,
const void *buf, size_t buf_len);
+#ifdef ENABLE_THREAD_SAFETY
+extern pgthreadlock_t *g_threadlock;
+#define pglock_thread() g_threadlock(true);
+#define pgunlock_thread() g_threadlock(false);
+#else
+#define pglock_thread() ((void)0)
+#define pgunlock_thread() ((void)0)
+ #endif
+
+
/* === in fe-exec.c === */
extern void pqSetResultError(PGresult *res, const char *msg);
Manfred Spraul wrote: > Bruce Momjian wrote: > > >What killed the idea of doing ssl or kerberos locking inside libpq was > >that there was no way to be sure that outside code didn't also access > >those routines. > > > A callback based implementation can handle that: libpq has a default > implementation for apps that do not use openssl or kerberos themself. If > the app wants to use the libraries, too, then it must replace the hooks > with their own locks. > > I've attached a simple proposal, just for kerberos 4. If you agree on > the general approach, I'll add it to all functions that are not thread safe. > > > I have documented that SSL and Kerberos are not > >thread-safe in the libpq docs. Let's wait and see If we need additional > >work in this area. > > > > > It means that multithreading is not usable: As Tom explained, the > connect string is often set directly by the end user. Setting "sslmode" > would result is races - impossible to support. In the very least, > sslmode and Kerberos would have to fail if the app is multithreaded. > I think it is even worse than you state. SSL and Kerberos is mostly controlled by pg_hba.conf, not the client connection string. Also, while we create a thread-aware libpq, we don't actually have any way to test if threads are being used by the application. Let's go in this direction for Kerberos and SSL, and I can modify the libpq docs on threading. -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001+ If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania19073
Your patch has been added to the PostgreSQL unapplied patches list at: http://momjian.postgresql.org/cgi-bin/pgpatches I will try to apply it within the next 48 hours. --------------------------------------------------------------------------- Manfred Spraul wrote: > Bruce Momjian wrote: > > >What killed the idea of doing ssl or kerberos locking inside libpq was > >that there was no way to be sure that outside code didn't also access > >those routines. > > > A callback based implementation can handle that: libpq has a default > implementation for apps that do not use openssl or kerberos themself. If > the app wants to use the libraries, too, then it must replace the hooks > with their own locks. > > I've attached a simple proposal, just for kerberos 4. If you agree on > the general approach, I'll add it to all functions that are not thread safe. > > > I have documented that SSL and Kerberos are not > >thread-safe in the libpq docs. Let's wait and see If we need additional > >work in this area. > > > > > It means that multithreading is not usable: As Tom explained, the > connect string is often set directly by the end user. Setting "sslmode" > would result is races - impossible to support. In the very least, > sslmode and Kerberos would have to fail if the app is multithreaded. > > -- > Manfred > Index: src/interfaces/libpq/fe-auth.c > =================================================================== > RCS file: /projects/cvsroot/pgsql-server/src/interfaces/libpq/fe-auth.c,v > retrieving revision 1.89 > diff -u -r1.89 fe-auth.c > --- src/interfaces/libpq/fe-auth.c 7 Jan 2004 18:56:29 -0000 1.89 > +++ src/interfaces/libpq/fe-auth.c 12 Mar 2004 20:07:02 -0000 > @@ -590,6 +590,7 @@ > > case AUTH_REQ_KRB4: > #ifdef KRB4 > + pglock_thread(); > if (pg_krb4_sendauth(PQerrormsg, conn->sock, > (struct sockaddr_in *) & conn->laddr.addr, > (struct sockaddr_in *) & conn->raddr.addr, > @@ -597,8 +598,10 @@ > { > snprintf(PQerrormsg, PQERRORMSG_LENGTH, > libpq_gettext("Kerberos 4 authentication failed\n")); > + pgunlock_thread(); > return STATUS_ERROR; > } > + pgunlock_thread(); > break; > #else > snprintf(PQerrormsg, PQERRORMSG_LENGTH, > @@ -722,6 +725,7 @@ > if (authsvc == 0) > return NULL; /* leave original error message in place */ > > + pglock_thread(); > #ifdef KRB4 > if (authsvc == STARTUP_KRB4_MSG) > name = pg_krb4_authname(PQerrormsg); > @@ -759,5 +763,6 @@ > > if (name && (authn = (char *) malloc(strlen(name) + 1))) > strcpy(authn, name); > + pgunlock_thread(); > return authn; > } > Index: src/interfaces/libpq/fe-connect.c > =================================================================== > RCS file: /projects/cvsroot/pgsql-server/src/interfaces/libpq/fe-connect.c,v > retrieving revision 1.268 > diff -u -r1.268 fe-connect.c > --- src/interfaces/libpq/fe-connect.c 10 Mar 2004 21:12:47 -0000 1.268 > +++ src/interfaces/libpq/fe-connect.c 12 Mar 2004 20:07:03 -0000 > @@ -3163,4 +3163,34 @@ > > #undef LINELEN > } > +/* > + * To keep the API consistent, the locking stubs are always provided, even > + * if they are not required. > + */ > +pgthreadlock_t *g_threadlock; > > +static pgthreadlock_t default_threadlock; > +static void > +default_threadlock(bool acquire) > +{ > +#if defined(ENABLE_THREAD_SAFETY) > + static pthread_mutex_t singlethread_lock = PTHREAD_MUTEX_INITIALIZER; > + if (acquire) > + pthread_mutex_lock(&singlethread_lock); > + else > + pthread_mutex_unlock(&singlethread_lock); > +#endif > +} > + > +pgthreadlock_t * > +PQregisterThreadLock(pgthreadlock_t *newhandler) > +{ > + pgthreadlock_t *prev; > + > + prev = g_threadlock; > + if (newhandler) > + g_threadlock = newhandler; > + else > + g_threadlock = default_threadlock; > + return prev; > +} > Index: src/interfaces/libpq/libpq-fe.h > =================================================================== > RCS file: /projects/cvsroot/pgsql-server/src/interfaces/libpq/libpq-fe.h,v > retrieving revision 1.102 > diff -u -r1.102 libpq-fe.h > --- src/interfaces/libpq/libpq-fe.h 9 Jan 2004 02:02:43 -0000 1.102 > +++ src/interfaces/libpq/libpq-fe.h 12 Mar 2004 20:07:03 -0000 > @@ -274,6 +274,22 @@ > PQnoticeProcessor proc, > void *arg); > > +typedef void (pgsigpipehandler_t)(bool enable, void **state); > + > +extern pgsigpipehandler_t * > +PQregisterSigpipeCallback(pgsigpipehandler_t *newhandler); > + > +/* > + * Used to set callback that prevents concurrent access to > + * non-thread safe functions that libpq needs. > + * The default implementation uses a libpq internal mutex. > + * Only required for multithreaded apps that use kerberos > + * both within their app and for postgresql connections. > + */ > +typedef void (pgthreadlock_t)(bool acquire); > + > +extern pgthreadlock_t * PQregisterThreadLock(pgthreadlock_t *newhandler); > + > /* === in fe-exec.c === */ > > /* Simple synchronous query */ > Index: src/interfaces/libpq/libpq-int.h > =================================================================== > RCS file: /projects/cvsroot/pgsql-server/src/interfaces/libpq/libpq-int.h,v > retrieving revision 1.85 > diff -u -r1.85 libpq-int.h > --- src/interfaces/libpq/libpq-int.h 5 Mar 2004 01:53:59 -0000 1.85 > +++ src/interfaces/libpq/libpq-int.h 12 Mar 2004 20:07:03 -0000 > @@ -359,6 +359,16 @@ > extern int pqPacketSend(PGconn *conn, char pack_type, > const void *buf, size_t buf_len); > > +#ifdef ENABLE_THREAD_SAFETY > +extern pgthreadlock_t *g_threadlock; > +#define pglock_thread() g_threadlock(true); > +#define pgunlock_thread() g_threadlock(false); > +#else > +#define pglock_thread() ((void)0) > +#define pgunlock_thread() ((void)0) > + #endif > + > + > /* === in fe-exec.c === */ > > extern void pqSetResultError(PGresult *res, const char *msg); -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001+ If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania19073
Bruce Momjian wrote: >Your patch has been added to the PostgreSQL unapplied patches list at: > > http://momjian.postgresql.org/cgi-bin/pgpatches > >I will try to apply it within the next 48 hours. > > You are too fast: the patch was a proof of concept, not really tested (actually quite buggy). Attached are two patches: - ready-sigpipe: check_sigpipe_handler skips pthread_create_key if a signal handler was installed. This is wrong - the key is always required. - ready-locking: locking around kerberos and openssl. The patches pass the regression tests on i386 linux. Kerberos is untested, ssl only partially tested due to the lack of a test setup. I'm still not sure if the new code is the right thing for the openssl initialization: libpq calls SSL_library_init() unconditionally. If the calling app uses ssl, too, this might confuse openssl. Could you replace my initial proposal with these two patches? Btw, is it intentional that THREAD_SUPPORT is not set in src/template/linux? -- Manfred Index: src/backend/libpq/md5.c =================================================================== RCS file: /projects/cvsroot/pgsql-server/src/backend/libpq/md5.c,v retrieving revision 1.22 diff -c -r1.22 md5.c *** src/backend/libpq/md5.c 29 Nov 2003 19:51:49 -0000 1.22 --- src/backend/libpq/md5.c 14 Mar 2004 10:46:54 -0000 *************** *** 271,277 **** static void bytesToHex(uint8 b[16], char *s) { ! static char *hex = "0123456789abcdef"; int q, w; --- 271,277 ---- static void bytesToHex(uint8 b[16], char *s) { ! static const char *hex = "0123456789abcdef"; int q, w; Index: src/interfaces/libpq/fe-auth.c =================================================================== RCS file: /projects/cvsroot/pgsql-server/src/interfaces/libpq/fe-auth.c,v retrieving revision 1.89 diff -c -r1.89 fe-auth.c *** src/interfaces/libpq/fe-auth.c 7 Jan 2004 18:56:29 -0000 1.89 --- src/interfaces/libpq/fe-auth.c 14 Mar 2004 10:46:55 -0000 *************** *** 590,595 **** --- 590,596 ---- case AUTH_REQ_KRB4: #ifdef KRB4 + pglock_thread(); if (pg_krb4_sendauth(PQerrormsg, conn->sock, (struct sockaddr_in *) & conn->laddr.addr, (struct sockaddr_in *) & conn->raddr.addr, *************** *** 597,604 **** --- 598,607 ---- { snprintf(PQerrormsg, PQERRORMSG_LENGTH, libpq_gettext("Kerberos 4 authentication failed\n")); + pgunlock_thread(); return STATUS_ERROR; } + pgunlock_thread(); break; #else snprintf(PQerrormsg, PQERRORMSG_LENGTH, *************** *** 608,620 **** --- 611,626 ---- case AUTH_REQ_KRB5: #ifdef KRB5 + pglock_thread(); if (pg_krb5_sendauth(PQerrormsg, conn->sock, hostname) != STATUS_OK) { snprintf(PQerrormsg, PQERRORMSG_LENGTH, libpq_gettext("Kerberos 5 authentication failed\n")); + pgunlock_thread(); return STATUS_ERROR; } + pgunlock_thread(); break; #else snprintf(PQerrormsg, PQERRORMSG_LENGTH, *************** *** 722,727 **** --- 728,734 ---- if (authsvc == 0) return NULL; /* leave original error message in place */ + pglock_thread(); #ifdef KRB4 if (authsvc == STARTUP_KRB4_MSG) name = pg_krb4_authname(PQerrormsg); *************** *** 759,763 **** --- 766,771 ---- if (name && (authn = (char *) malloc(strlen(name) + 1))) strcpy(authn, name); + pgunlock_thread(); return authn; } Index: src/interfaces/libpq/fe-connect.c =================================================================== RCS file: /projects/cvsroot/pgsql-server/src/interfaces/libpq/fe-connect.c,v retrieving revision 1.268 diff -c -r1.268 fe-connect.c *** src/interfaces/libpq/fe-connect.c 10 Mar 2004 21:12:47 -0000 1.268 --- src/interfaces/libpq/fe-connect.c 14 Mar 2004 10:46:56 -0000 *************** *** 2902,2908 **** PQsetClientEncoding(PGconn *conn, const char *encoding) { char qbuf[128]; ! static char query[] = "set client_encoding to '%s'"; PGresult *res; int status; --- 2902,2908 ---- PQsetClientEncoding(PGconn *conn, const char *encoding) { char qbuf[128]; ! static const char query[] = "set client_encoding to '%s'"; PGresult *res; int status; *************** *** 3162,3166 **** --- 3162,3207 ---- return NULL; #undef LINELEN + } + + /* + * To keep the API consistent, the locking stubs are always provided, even + * if they are not required. + */ + + void + PQenableSSLLocks(int enable) + { + #if defined(ENABLE_THREAD_SAFETY) && defined(USE_SSL) + pq_usessllocks = enable; + #endif + } + + static pgthreadlock_t default_threadlock; + static void + default_threadlock(int acquire) + { + #if defined(ENABLE_THREAD_SAFETY) + static pthread_mutex_t singlethread_lock = PTHREAD_MUTEX_INITIALIZER; + if (acquire) + pthread_mutex_lock(&singlethread_lock); + else + pthread_mutex_unlock(&singlethread_lock); + #endif + } + + pgthreadlock_t *g_threadlock = default_threadlock; + + pgthreadlock_t * + PQregisterThreadLock(pgthreadlock_t *newhandler) + { + pgthreadlock_t *prev; + + prev = g_threadlock; + if (newhandler) + g_threadlock = newhandler; + else + g_threadlock = default_threadlock; + return prev; } Index: src/interfaces/libpq/fe-secure.c =================================================================== RCS file: /projects/cvsroot/pgsql-server/src/interfaces/libpq/fe-secure.c,v retrieving revision 1.37 diff -c -r1.37 fe-secure.c *** src/interfaces/libpq/fe-secure.c 10 Feb 2004 15:21:24 -0000 1.37 --- src/interfaces/libpq/fe-secure.c 14 Mar 2004 10:46:56 -0000 *************** *** 135,145 **** static DH *load_dh_buffer(const char *, size_t); static DH *tmp_dh_cb(SSL *s, int is_export, int keylength); static int client_cert_cb(SSL *, X509 **, EVP_PKEY **); static int initialize_SSL(PGconn *); static void destroy_SSL(void); static PostgresPollingStatusType open_client_SSL(PGconn *); static void close_SSL(PGconn *); ! static const char *SSLerrmessage(void); #endif #ifdef USE_SSL --- 135,147 ---- static DH *load_dh_buffer(const char *, size_t); static DH *tmp_dh_cb(SSL *s, int is_export, int keylength); 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 *************** *** 251,259 **** !SSL_set_app_data(conn->ssl, conn) || !SSL_set_fd(conn->ssl, conn->sock)) { printfPQExpBuffer(&conn->errorMessage, libpq_gettext("could not establish SSL connection: %s\n"), ! SSLerrmessage()); close_SSL(conn); return PGRES_POLLING_FAILED; } --- 253,263 ---- !SSL_set_app_data(conn->ssl, conn) || !SSL_set_fd(conn->ssl, conn->sock)) { + char *err = SSLerrmessage(); printfPQExpBuffer(&conn->errorMessage, libpq_gettext("could not establish SSL connection: %s\n"), ! err); ! SSLerrfree(err); close_SSL(conn); return PGRES_POLLING_FAILED; } *************** *** 327,334 **** break; } case SSL_ERROR_SSL: ! printfPQExpBuffer(&conn->errorMessage, ! libpq_gettext("SSL error: %s\n"), SSLerrmessage()); /* fall through */ case SSL_ERROR_ZERO_RETURN: SOCK_ERRNO_SET(ECONNRESET); --- 331,342 ---- break; } case SSL_ERROR_SSL: ! { ! char *err = SSLerrmessage(); ! printfPQExpBuffer(&conn->errorMessage, ! libpq_gettext("SSL error: %s\n"), err); ! SSLerrfree(err); ! } /* fall through */ case SSL_ERROR_ZERO_RETURN: SOCK_ERRNO_SET(ECONNRESET); *************** *** 402,409 **** break; } case SSL_ERROR_SSL: ! printfPQExpBuffer(&conn->errorMessage, ! libpq_gettext("SSL error: %s\n"), SSLerrmessage()); /* fall through */ case SSL_ERROR_ZERO_RETURN: SOCK_ERRNO_SET(ECONNRESET); --- 410,421 ---- break; } case SSL_ERROR_SSL: ! { ! char *err = SSLerrmessage(); ! printfPQExpBuffer(&conn->errorMessage, ! libpq_gettext("SSL error: %s\n"), err); ! SSLerrfree(err); ! } /* fall through */ case SSL_ERROR_ZERO_RETURN: SOCK_ERRNO_SET(ECONNRESET); *************** *** 750,758 **** } if (PEM_read_X509(fp, x509, NULL, NULL) == NULL) { printfPQExpBuffer(&conn->errorMessage, libpq_gettext("could not read certificate (%s): %s\n"), ! fnbuf, SSLerrmessage()); fclose(fp); return -1; } --- 762,772 ---- } if (PEM_read_X509(fp, x509, NULL, NULL) == NULL) { + char *err = SSLerrmessage(); printfPQExpBuffer(&conn->errorMessage, libpq_gettext("could not read certificate (%s): %s\n"), ! fnbuf, err); ! SSLerrfree(err); fclose(fp); return -1; } *************** *** 795,803 **** } if (PEM_read_PrivateKey(fp, pkey, cb, NULL) == NULL) { printfPQExpBuffer(&conn->errorMessage, libpq_gettext("could not read private key (%s): %s\n"), ! fnbuf, SSLerrmessage()); X509_free(*x509); fclose(fp); return -1; --- 809,819 ---- } if (PEM_read_PrivateKey(fp, pkey, cb, NULL) == NULL) { + char *err = SSLerrmessage(); printfPQExpBuffer(&conn->errorMessage, libpq_gettext("could not read private key (%s): %s\n"), ! fnbuf, err); ! SSLerrfree(err); X509_free(*x509); fclose(fp); return -1; *************** *** 807,815 **** /* verify that the cert and key go together */ if (!X509_check_private_key(*x509, *pkey)) { printfPQExpBuffer(&conn->errorMessage, libpq_gettext("certificate/private key mismatch (%s): %s\n"), ! fnbuf, SSLerrmessage()); X509_free(*x509); EVP_PKEY_free(*pkey); return -1; --- 823,833 ---- /* verify that the cert and key go together */ if (!X509_check_private_key(*x509, *pkey)) { + char *err = SSLerrmessage(); printfPQExpBuffer(&conn->errorMessage, libpq_gettext("certificate/private key mismatch (%s): %s\n"), ! fnbuf, err); ! SSLerrfree(err); X509_free(*x509); EVP_PKEY_free(*pkey); return -1; *************** *** 819,838 **** #endif } ! /* ! * Initialize global SSL context. ! */ static int ! initialize_SSL(PGconn *conn) { ! #ifndef WIN32 ! struct stat buf; ! char pwdbuf[BUFSIZ]; ! struct passwd pwdstr; ! struct passwd *pwd = NULL; ! char fnbuf[2048]; ! #endif if (!SSL_context) { SSL_library_init(); --- 837,888 ---- #endif } ! #ifdef ENABLE_THREAD_SAFETY ! ! static unsigned long ! pq_threadidcallback(void) ! { ! return (unsigned long)pthread_self(); ! } ! ! static pthread_rwlock_t *pq_lockarray; ! static void ! pq_lockingcallback(int mode, int n, const char *file, int line) ! { ! if (mode & CRYPTO_LOCK) { ! pthread_rwlock_wrlock(&pq_lockarray[n]); ! } else { ! pthread_rwlock_unlock(&pq_lockarray[n]); ! } ! } ! ! bool pq_usessllocks = true; ! ! #endif /* ENABLE_THRAD_SAFETY */ ! static int ! init_ssl_system(PGconn *conn) { ! #ifdef ENABLE_THREAD_SAFETY ! static pthread_mutex_t init_mutex = PTHREAD_MUTEX_INITIALIZER; ! ! pthread_mutex_lock(&init_mutex); ! ! if (pq_usessllocks && pq_lockarray == NULL) { ! int i; ! CRYPTO_set_id_callback(pq_threadidcallback); ! ! pq_lockarray = malloc(sizeof(pthread_rwlock_t)*CRYPTO_num_locks()); ! if (!pq_lockarray) { ! pthread_mutex_unlock(&init_mutex); ! return -1; ! } ! for (i=0;i<CRYPTO_num_locks();i++) ! pthread_rwlock_init(&pq_lockarray[i], NULL); + CRYPTO_set_locking_callback(pq_lockingcallback); + } + #endif if (!SSL_context) { SSL_library_init(); *************** *** 840,851 **** SSL_context = SSL_CTX_new(TLSv1_method()); if (!SSL_context) { printfPQExpBuffer(&conn->errorMessage, libpq_gettext("could not create SSL context: %s\n"), ! SSLerrmessage()); return -1; } } #ifndef WIN32 if (pqGetpwuid(getuid(), &pwdstr, pwdbuf, sizeof(pwdbuf), &pwd) == 0) --- 890,927 ---- SSL_context = SSL_CTX_new(TLSv1_method()); if (!SSL_context) { + char *err = SSLerrmessage(); printfPQExpBuffer(&conn->errorMessage, libpq_gettext("could not create SSL context: %s\n"), ! 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 + initialize_SSL(PGconn *conn) + { + #ifndef WIN32 + struct stat buf; + char pwdbuf[BUFSIZ]; + struct passwd pwdstr; + struct passwd *pwd = NULL; + char fnbuf[2048]; + #endif + + if(!init_ssl_system(conn)) + return -1; #ifndef WIN32 if (pqGetpwuid(getuid(), &pwdstr, pwdbuf, sizeof(pwdbuf), &pwd) == 0) *************** *** 867,875 **** } if (!SSL_CTX_load_verify_locations(SSL_context, fnbuf, 0)) { printfPQExpBuffer(&conn->errorMessage, libpq_gettext("could not read root certificate list (%s): %s\n"), ! fnbuf, SSLerrmessage()); return -1; } } --- 943,953 ---- } if (!SSL_CTX_load_verify_locations(SSL_context, fnbuf, 0)) { + char *err = SSLerrmessage(); printfPQExpBuffer(&conn->errorMessage, libpq_gettext("could not read root certificate list (%s): %s\n"), ! fnbuf, err); ! SSLerrfree(err); return -1; } } *************** *** 936,945 **** return PGRES_POLLING_FAILED; } case SSL_ERROR_SSL: ! printfPQExpBuffer(&conn->errorMessage, ! libpq_gettext("SSL error: %s\n"), SSLerrmessage()); ! close_SSL(conn); ! return PGRES_POLLING_FAILED; default: printfPQExpBuffer(&conn->errorMessage, --- 1014,1027 ---- return PGRES_POLLING_FAILED; } case SSL_ERROR_SSL: ! { ! char *err = SSLerrmessage(); ! printfPQExpBuffer(&conn->errorMessage, ! libpq_gettext("SSL error: %s\n"), err); ! SSLerrfree(err); ! close_SSL(conn); ! return PGRES_POLLING_FAILED; ! } default: printfPQExpBuffer(&conn->errorMessage, *************** *** 973,981 **** conn->peer = SSL_get_peer_certificate(conn->ssl); if (conn->peer == NULL) { printfPQExpBuffer(&conn->errorMessage, libpq_gettext("certificate could not be obtained: %s\n"), ! SSLerrmessage()); close_SSL(conn); return PGRES_POLLING_FAILED; } --- 1055,1065 ---- conn->peer = SSL_get_peer_certificate(conn->ssl); if (conn->peer == NULL) { + char *err = SSLerrmessage(); printfPQExpBuffer(&conn->errorMessage, libpq_gettext("certificate could not be obtained: %s\n"), ! err); ! SSLerrfree(err); close_SSL(conn); return PGRES_POLLING_FAILED; } *************** *** 1036,1058 **** * return NULL if it doesn't recognize the error code. We don't * want to return NULL ever. */ ! static const char * SSLerrmessage(void) { unsigned long errcode; const char *errreason; ! static char errbuf[32]; errcode = ERR_get_error(); ! if (errcode == 0) ! return "No SSL error reported"; errreason = ERR_reason_error_string(errcode); ! if (errreason != NULL) ! return errreason; ! snprintf(errbuf, sizeof(errbuf), "SSL error code %lu", errcode); return errbuf; } /* * Return pointer to SSL object. */ --- 1120,1159 ---- * return NULL if it doesn't recognize the error code. We don't * want to return NULL ever. */ ! static char ssl_nomem[] = "Out of memory allocating error description"; ! #define SSL_ERR_LEN 128 ! ! static char * SSLerrmessage(void) { unsigned long errcode; const char *errreason; ! char *errbuf; + errbuf = malloc(SSL_ERR_LEN); + if (!errbuf) + return ssl_nomem; errcode = ERR_get_error(); ! if (errcode == 0) { ! strcpy(errbuf, "No SSL error reported"); ! return errbuf; ! } errreason = ERR_reason_error_string(errcode); ! if (errreason != NULL) { ! strncpy(errbuf, errreason, SSL_ERR_LEN-1); ! errbuf[SSL_ERR_LEN-1] = '\0'; ! return errbuf; ! } ! snprintf(errbuf, SSL_ERR_LEN, "SSL error code %lu", errcode); return errbuf; } + static void + SSLerrfree(char *buf) + { + if (buf != ssl_nomem) + free(buf); + } /* * Return pointer to SSL object. */ Index: src/interfaces/libpq/libpq-fe.h =================================================================== RCS file: /projects/cvsroot/pgsql-server/src/interfaces/libpq/libpq-fe.h,v retrieving revision 1.102 diff -c -r1.102 libpq-fe.h *** src/interfaces/libpq/libpq-fe.h 9 Jan 2004 02:02:43 -0000 1.102 --- src/interfaces/libpq/libpq-fe.h 14 Mar 2004 10:46:57 -0000 *************** *** 274,279 **** --- 274,293 ---- PQnoticeProcessor proc, void *arg); + /* + * Used to set callback that prevents concurrent access to + * non-thread safe functions that libpq needs. + * The default implementation uses a libpq internal mutex. + * Only required for multithreaded apps that use kerberos + * both within their app and for postgresql connections. + */ + typedef void (pgthreadlock_t)(int acquire); + + extern pgthreadlock_t * PQregisterThreadLock(pgthreadlock_t *newhandler); + + void + PQenableSSLLocks(int enable); + /* === in fe-exec.c === */ /* Simple synchronous query */ Index: src/interfaces/libpq/libpq-int.h =================================================================== RCS file: /projects/cvsroot/pgsql-server/src/interfaces/libpq/libpq-int.h,v retrieving revision 1.85 diff -c -r1.85 libpq-int.h *** src/interfaces/libpq/libpq-int.h 5 Mar 2004 01:53:59 -0000 1.85 --- src/interfaces/libpq/libpq-int.h 14 Mar 2004 10:46:57 -0000 *************** *** 359,364 **** --- 359,374 ---- extern int pqPacketSend(PGconn *conn, char pack_type, const void *buf, size_t buf_len); + #ifdef ENABLE_THREAD_SAFETY + extern pgthreadlock_t *g_threadlock; + #define pglock_thread() g_threadlock(true); + #define pgunlock_thread() g_threadlock(false); + #else + #define pglock_thread() ((void)0) + #define pgunlock_thread() ((void)0) + #endif + + /* === in fe-exec.c === */ extern void pqSetResultError(PGresult *res, const char *msg); *************** *** 448,453 **** --- 458,464 ---- #ifdef ENABLE_THREAD_SAFETY extern void check_sigpipe_handler(void); extern pthread_key_t thread_in_send; + extern bool pq_usessllocks; #endif /* Index: src/interfaces/libpq/fe-secure.c =================================================================== RCS file: /projects/cvsroot/pgsql-server/src/interfaces/libpq/fe-secure.c,v retrieving revision 1.37 diff -c -r1.37 fe-secure.c *** src/interfaces/libpq/fe-secure.c 10 Feb 2004 15:21:24 -0000 1.37 --- src/interfaces/libpq/fe-secure.c 14 Mar 2004 08:31:48 -0000 *************** *** 1077,1096 **** pqsigfunc pipehandler; /* * If the app hasn't set a SIGPIPE handler, define our own * that ignores SIGPIPE on libpq send() and does SIG_DFL * for other SIGPIPE cases. */ pipehandler = pqsignalinquire(SIGPIPE); if (pipehandler == SIG_DFL) /* not set by application */ - { - /* - * Create key first because the signal handler might be called - * right after being installed. - */ - pthread_key_create(&thread_in_send, NULL); pqsignal(SIGPIPE, sigpipe_handler_ignore_send); - } } /* --- 1077,1096 ---- pqsigfunc pipehandler; /* + * Always create the key for SIGPIPE handling - PQinSend needs + * it. Create it first because the signal handler might be called + * right after being installed. + */ + pthread_key_create(&thread_in_send, NULL); + + /* * If the app hasn't set a SIGPIPE handler, define our own * that ignores SIGPIPE on libpq send() and does SIG_DFL * for other SIGPIPE cases. */ pipehandler = pqsignalinquire(SIGPIPE); if (pipehandler == SIG_DFL) /* not set by application */ pqsignal(SIGPIPE, sigpipe_handler_ignore_send); } /*
Patch withdrawn by author.
---------------------------------------------------------------------------
Manfred Spraul wrote:
> Bruce Momjian wrote:
>
> >What killed the idea of doing ssl or kerberos locking inside libpq was
> >that there was no way to be sure that outside code didn't also access
> >those routines.
> >
> A callback based implementation can handle that: libpq has a default
> implementation for apps that do not use openssl or kerberos themself. If
> the app wants to use the libraries, too, then it must replace the hooks
> with their own locks.
>
> I've attached a simple proposal, just for kerberos 4. If you agree on
> the general approach, I'll add it to all functions that are not thread safe.
>
> > I have documented that SSL and Kerberos are not
> >thread-safe in the libpq docs. Let's wait and see If we need additional
> >work in this area.
> >
> >
> It means that multithreading is not usable: As Tom explained, the
> connect string is often set directly by the end user. Setting "sslmode"
> would result is races - impossible to support. In the very least,
> sslmode and Kerberos would have to fail if the app is multithreaded.
>
> --
> Manfred
> Index: src/interfaces/libpq/fe-auth.c
> ===================================================================
> RCS file: /projects/cvsroot/pgsql-server/src/interfaces/libpq/fe-auth.c,v
> retrieving revision 1.89
> diff -u -r1.89 fe-auth.c
> --- src/interfaces/libpq/fe-auth.c 7 Jan 2004 18:56:29 -0000 1.89
> +++ src/interfaces/libpq/fe-auth.c 12 Mar 2004 20:07:02 -0000
> @@ -590,6 +590,7 @@
>
> case AUTH_REQ_KRB4:
> #ifdef KRB4
> + pglock_thread();
> if (pg_krb4_sendauth(PQerrormsg, conn->sock,
> (struct sockaddr_in *) & conn->laddr.addr,
> (struct sockaddr_in *) & conn->raddr.addr,
> @@ -597,8 +598,10 @@
> {
> snprintf(PQerrormsg, PQERRORMSG_LENGTH,
> libpq_gettext("Kerberos 4 authentication failed\n"));
> + pgunlock_thread();
> return STATUS_ERROR;
> }
> + pgunlock_thread();
> break;
> #else
> snprintf(PQerrormsg, PQERRORMSG_LENGTH,
> @@ -722,6 +725,7 @@
> if (authsvc == 0)
> return NULL; /* leave original error message in place */
>
> + pglock_thread();
> #ifdef KRB4
> if (authsvc == STARTUP_KRB4_MSG)
> name = pg_krb4_authname(PQerrormsg);
> @@ -759,5 +763,6 @@
>
> if (name && (authn = (char *) malloc(strlen(name) + 1)))
> strcpy(authn, name);
> + pgunlock_thread();
> return authn;
> }
> Index: src/interfaces/libpq/fe-connect.c
> ===================================================================
> RCS file: /projects/cvsroot/pgsql-server/src/interfaces/libpq/fe-connect.c,v
> retrieving revision 1.268
> diff -u -r1.268 fe-connect.c
> --- src/interfaces/libpq/fe-connect.c 10 Mar 2004 21:12:47 -0000 1.268
> +++ src/interfaces/libpq/fe-connect.c 12 Mar 2004 20:07:03 -0000
> @@ -3163,4 +3163,34 @@
>
> #undef LINELEN
> }
> +/*
> + * To keep the API consistent, the locking stubs are always provided, even
> + * if they are not required.
> + */
> +pgthreadlock_t *g_threadlock;
>
> +static pgthreadlock_t default_threadlock;
> +static void
> +default_threadlock(bool acquire)
> +{
> +#if defined(ENABLE_THREAD_SAFETY)
> + static pthread_mutex_t singlethread_lock = PTHREAD_MUTEX_INITIALIZER;
> + if (acquire)
> + pthread_mutex_lock(&singlethread_lock);
> + else
> + pthread_mutex_unlock(&singlethread_lock);
> +#endif
> +}
> +
> +pgthreadlock_t *
> +PQregisterThreadLock(pgthreadlock_t *newhandler)
> +{
> + pgthreadlock_t *prev;
> +
> + prev = g_threadlock;
> + if (newhandler)
> + g_threadlock = newhandler;
> + else
> + g_threadlock = default_threadlock;
> + return prev;
> +}
> Index: src/interfaces/libpq/libpq-fe.h
> ===================================================================
> RCS file: /projects/cvsroot/pgsql-server/src/interfaces/libpq/libpq-fe.h,v
> retrieving revision 1.102
> diff -u -r1.102 libpq-fe.h
> --- src/interfaces/libpq/libpq-fe.h 9 Jan 2004 02:02:43 -0000 1.102
> +++ src/interfaces/libpq/libpq-fe.h 12 Mar 2004 20:07:03 -0000
> @@ -274,6 +274,22 @@
> PQnoticeProcessor proc,
> void *arg);
>
> +typedef void (pgsigpipehandler_t)(bool enable, void **state);
> +
> +extern pgsigpipehandler_t *
> +PQregisterSigpipeCallback(pgsigpipehandler_t *newhandler);
> +
> +/*
> + * Used to set callback that prevents concurrent access to
> + * non-thread safe functions that libpq needs.
> + * The default implementation uses a libpq internal mutex.
> + * Only required for multithreaded apps that use kerberos
> + * both within their app and for postgresql connections.
> + */
> +typedef void (pgthreadlock_t)(bool acquire);
> +
> +extern pgthreadlock_t * PQregisterThreadLock(pgthreadlock_t *newhandler);
> +
> /* === in fe-exec.c === */
>
> /* Simple synchronous query */
> Index: src/interfaces/libpq/libpq-int.h
> ===================================================================
> RCS file: /projects/cvsroot/pgsql-server/src/interfaces/libpq/libpq-int.h,v
> retrieving revision 1.85
> diff -u -r1.85 libpq-int.h
> --- src/interfaces/libpq/libpq-int.h 5 Mar 2004 01:53:59 -0000 1.85
> +++ src/interfaces/libpq/libpq-int.h 12 Mar 2004 20:07:03 -0000
> @@ -359,6 +359,16 @@
> extern int pqPacketSend(PGconn *conn, char pack_type,
> const void *buf, size_t buf_len);
>
> +#ifdef ENABLE_THREAD_SAFETY
> +extern pgthreadlock_t *g_threadlock;
> +#define pglock_thread() g_threadlock(true);
> +#define pgunlock_thread() g_threadlock(false);
> +#else
> +#define pglock_thread() ((void)0)
> +#define pgunlock_thread() ((void)0)
> + #endif
> +
> +
> /* === in fe-exec.c === */
>
> extern void pqSetResultError(PGresult *res, const char *msg);
-- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610)
359-1001+ If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square,
Pennsylvania19073
Manfred Spraul wrote: > Bruce Momjian wrote: > > >Your patch has been added to the PostgreSQL unapplied patches list at: > > > > http://momjian.postgresql.org/cgi-bin/pgpatches > > > >I will try to apply it within the next 48 hours. > > > > > You are too fast: the patch was a proof of concept, not really tested > (actually quite buggy). > Attached are two patches: > > - ready-sigpipe: check_sigpipe_handler skips pthread_create_key if a > signal handler was installed. This is wrong - the key is always required. > - ready-locking: locking around kerberos and openssl. > > The patches pass the regression tests on i386 linux. Kerberos is > untested, ssl only partially tested due to the lack of a test setup. > I'm still not sure if the new code is the right thing for the openssl > initialization: libpq calls SSL_library_init() unconditionally. If the > calling app uses ssl, too, this might confuse openssl. How can we test if libpq needs to call that? Seems that is an issue whether we are threaded or not, no? > Could you replace my initial proposal with these two patches? Done. > Btw, is it intentional that THREAD_SUPPORT is not set in src/template/linux? There is a new test program in src/tools/thread that needs to be run for every platform for 7.5. We can't use the 7.4.X tests because it didn't report individual function tests, just one general value. We need individual test reports for 7.5. Run the test program and post the results and I will get it updated. The test output on my bsd/os machine is:Make sure you have added any needed 'THREAD_CPPFLAGS' and 'THREAD_LIBS'defines to your template/$port file before compilingthis program.Add this to your template/$port file:STRERROR_THREADSAFE=yesGETPWUID_THREADSAFE=yesGETHOSTBYNAME_THREADSAFE=yes -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001+ If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania19073
Bruce Momjian wrote: >How can we test if libpq needs to call that? Seems that is an issue >whether we are threaded or not, no? > > I think it's always an issue: in the non-threaded case, it's just not fatal. At least some openssl init functions are protected with "if (done) return; done = 1;", and it the worst case, it's a memory leak. With threaded apps, it might corrupt a concurrent ssl transaction. Perhaps PQenableSSLLocks could handle that case, too - a special flag for skip SSL_library_init(). >There is a new test program in src/tools/thread that needs to be run for >every platform for 7.5. We can't use the 7.4.X tests because it didn't >report individual function tests, just one general value. We need >individual test reports for 7.5. Run the test program and post the >results and I will get it updated. The test output on my bsd/os machine >is: > > RedHat Fedora Core 1 and Debian 3.0 both report << Make sure you have added any needed 'THREAD_CPPFLAGS' and 'THREAD_LIBS' defines to your template/$port file before compiling this program. Add this to your template/$port file: STRERROR_THREADSAFE=yes GETPWUID_THREADSAFE=no GETHOSTBYNAME_THREADSAFE=no << The uname's are Linux <snip> 2.4.25-1-686 #1 Tue Feb 24 10:55:59 EST 2004 i686 unknown unknown GNU/Linux and Linux ab 2.4.22-1.2174.nptl #1 Wed Feb 18 16:38:32 EST 2004 i686 i686 i386 GNU/Linux Both glibc 2.3.2, one with nptl, one with linuxthreads as the pthread library. -- Manfred
Manfred Spraul wrote: > Bruce Momjian wrote: > > >How can we test if libpq needs to call that? Seems that is an issue > >whether we are threaded or not, no? > > > > > I think it's always an issue: in the non-threaded case, it's just not > fatal. At least some openssl init functions are protected with "if > (done) return; done = 1;", and it the worst case, it's a memory leak. > With threaded apps, it might corrupt a concurrent ssl transaction. > Perhaps PQenableSSLLocks could handle that case, too - a special flag > for skip SSL_library_init(). I see. > >There is a new test program in src/tools/thread that needs to be run for > >every platform for 7.5. We can't use the 7.4.X tests because it didn't > >report individual function tests, just one general value. We need > >individual test reports for 7.5. Run the test program and post the > >results and I will get it updated. The test output on my bsd/os machine > >is: > > > > > RedHat Fedora Core 1 and Debian 3.0 both report > > << > Make sure you have added any needed 'THREAD_CPPFLAGS' and 'THREAD_LIBS' > defines to your template/$port file before compiling this program. > > Add this to your template/$port file: > > STRERROR_THREADSAFE=yes > GETPWUID_THREADSAFE=no > GETHOSTBYNAME_THREADSAFE=no > << > The uname's are > Linux <snip> 2.4.25-1-686 #1 Tue Feb 24 10:55:59 EST 2004 i686 unknown > unknown GNU/Linux > and > Linux ab 2.4.22-1.2174.nptl #1 Wed Feb 18 16:38:32 EST 2004 i686 i686 > i386 GNU/Linux > > Both glibc 2.3.2, one with nptl, one with linuxthreads as the pthread > library. template/linux updated. -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001+ If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania19073