Tom Lane wrote:

Manfred Spraul <[EMAIL PROTECTED]> writes:


But what about kerberos: I'm a bit reluctant to add a forth mutex: what if kerberos calls gethostbyname or getpwuid internally?



Wouldn't help anyway, if some other part of the app also calls kerberos.


That's why I've proposed to use the system from openssl: The libpq user must implement a lock callback, and libpq calls it around the critical sections.
Attached is an untested prototype patch. What do you think?


--
   Manfred
Index: src/interfaces/libpq/fe-connect.c
===================================================================
RCS file: /projects/cvsroot/pgsql-server/src/interfaces/libpq/fe-connect.c,v
retrieving revision 1.267
diff -u -r1.267 fe-connect.c
--- src/interfaces/libpq/fe-connect.c   9 Jan 2004 02:02:43 -0000       1.267
+++ src/interfaces/libpq/fe-connect.c   11 Jan 2004 16:54:06 -0000
@@ -885,12 +885,6 @@
        struct addrinfo hint;
        const char *node = NULL;
        int                     ret;
-#ifdef ENABLE_THREAD_SAFETY
-       static pthread_once_t check_sigpipe_once = PTHREAD_ONCE_INIT;
-
-       /* Check only on first connection request */
-       pthread_once(&check_sigpipe_once, check_sigpipe_handler);
-#endif
 
        if (!conn)
                return 0;
Index: src/interfaces/libpq/fe-secure.c
===================================================================
RCS file: /projects/cvsroot/pgsql-server/src/interfaces/libpq/fe-secure.c,v
retrieving revision 1.36
diff -u -r1.36 fe-secure.c
--- src/interfaces/libpq/fe-secure.c    9 Jan 2004 02:17:15 -0000       1.36
+++ src/interfaces/libpq/fe-secure.c    11 Jan 2004 16:54:07 -0000
@@ -146,11 +146,6 @@
 static SSL_CTX *SSL_context = NULL;
 #endif
 
-#ifdef ENABLE_THREAD_SAFETY
-static void sigpipe_handler_ignore_send(int signo);
-pthread_key_t thread_in_send;
-#endif
-
 /* ------------------------------------------------------------ */
 /*                                              Hardcoded values                      
                         */
 /* ------------------------------------------------------------ */
@@ -212,6 +207,26 @@
 /* ------------------------------------------------------------ */
 
 /*
+ *     Sigpipe handling.
+ *     Dummy provided even for WIN32 to keep the API consistent
+ */
+pgsigpipehandler_t default_sigpipehandler;
+
+void default_sigpipehandler(bool enable, void **state)
+{
+#ifndef WIN32
+       if (enable) {
+               *state = (void*) pqsignal(SIGPIPE, SIG_IGN);
+       } else {
+               pqsignal(SIGPIPE, (pqsigfunc)*state);
+       }
+#endif
+}
+
+static pgsigpipehandler_t *g_sigpipehandler = default_sigpipehandler;
+
+
+/*
  *     Initialize global context
  */
 int
@@ -356,12 +371,9 @@
 {
        ssize_t         n;
 
-#ifdef ENABLE_THREAD_SAFETY
-       pthread_setspecific(thread_in_send, "t");
-#else
 #ifndef WIN32
-       pqsigfunc       oldsighandler = pqsignal(SIGPIPE, SIG_IGN);
-#endif
+       void *sigstate;
+       g_sigpipehandler(true, &sigstate);
 #endif
 
 #ifdef USE_SSL
@@ -420,12 +432,8 @@
 #endif
                n = send(conn->sock, ptr, len, 0);
 
-#ifdef ENABLE_THREAD_SAFETY
-       pthread_setspecific(thread_in_send, "f");
-#else
 #ifndef WIN32
-       pqsignal(SIGPIPE, oldsighandler);
-#endif
+       g_sigpipehandler(false, &sigstate);
 #endif
 
        return n;
@@ -1066,62 +1074,18 @@
 
 #endif   /* USE_SSL */
 
-
-#ifdef ENABLE_THREAD_SAFETY
 /*
- *     Check SIGPIPE handler and perhaps install our own.
+ *     PQregisterSigpipeCallback
  */
-void
-check_sigpipe_handler(void)
+pgsigpipehandler_t *
+PQregisterSigpipeCallback(pgsigpipehandler_t *newhandler)
 {
-       pqsigfunc pipehandler;
+       pgsigpipehandler_t *prev;
 
-       /*
-        *      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);
-       }
-}
-
-/*
- *     Threaded SIGPIPE signal handler
- */
-void
-sigpipe_handler_ignore_send(int signo)
-{
-       /*
-        *      If we have gotten a SIGPIPE outside send(), exit.
-        *      Synchronous signals are delivered to the thread
-        *      that caused the signal.
-        */
-       if (!PQinSend())
-               exit(128 + SIGPIPE);    /* typical return value for SIG_DFL */
-}
-#endif
- 
-/*
- *     Indicates whether the current thread is in send()
- *     For use by SIGPIPE signal handlers;  they should
- *     ignore SIGPIPE when libpq is in send().  This means
- *     that the backend has died unexpectedly.
- */
-pqbool
-PQinSend(void)
-{
-#ifdef ENABLE_THREAD_SAFETY
-       return (pthread_getspecific(thread_in_send) /* has it been set? */ &&
-                       *(char *)pthread_getspecific(thread_in_send) == 't') ? true : 
false;
-#else
-       return false;   /* No threading, so we can't be in send() */
-#endif
+       prev = g_sigpipehandler;
+       if (newhandler)
+               g_sigpipehandler = newhandler;
+       else
+               g_sigpipehandler = default_sigpipehandler;
+       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     11 Jan 2004 16:54:07 -0000
@@ -453,10 +453,31 @@
 /* === in fe-secure.c === */
 
 /*
- *     Indicates whether the libpq thread is in send().
- *     Used to ignore SIGPIPE if thread is in send().
+ *     Used to set callback that must protect libpq from SIGPIPE signals.
+ *     A default implementation is provided for single threaded apps.
+ *     Not required for WIN32.
  */
-pqbool PQinSend(void);
+typedef void (pgsigpipehandler_t)(bool enable, void **state);
+
+extern pgsigpipehandler_t *
+PQregisterSigpipeCallback(pgsigpipehandler_t *newhandler);
+
+/* === in thread.c === */
+
+/*
+ *     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 on platforms that
+ *     do not support the thread-safe equivalents and that want
+ *     to use the functions, too.
+ *     List of functions:
+ *     - stderror, getpwuid, gethostbyname.
+ *     TODO: the mutex must be used around kerberos calls, too.
+ */
+typedef void (pgthreadlock_t)(bool acquire);
+
+extern pgthreadlock_t * PQregisterThreadLock(pgthreadlock_t *newhandler);
 
 #ifdef __cplusplus
 }
Index: src/interfaces/libpq/libpq-int.h
===================================================================
RCS file: /projects/cvsroot/pgsql-server/src/interfaces/libpq/libpq-int.h,v
retrieving revision 1.84
diff -u -r1.84 libpq-int.h
--- src/interfaces/libpq/libpq-int.h    9 Jan 2004 02:02:43 -0000       1.84
+++ src/interfaces/libpq/libpq-int.h    11 Jan 2004 16:54:07 -0000
@@ -446,8 +446,12 @@
 extern ssize_t pqsecure_read(PGconn *, void *ptr, size_t len);
 extern ssize_t pqsecure_write(PGconn *, const void *ptr, size_t len);
 #ifdef ENABLE_THREAD_SAFETY
-extern void check_sigpipe_handler(void);
-extern pthread_key_t thread_in_send;
+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
 
 /*
Index: src/port/thread.c
===================================================================
RCS file: /projects/cvsroot/pgsql-server/src/port/thread.c,v
retrieving revision 1.14
diff -u -r1.14 thread.c
--- src/port/thread.c   29 Nov 2003 22:41:31 -0000      1.14
+++ src/port/thread.c   11 Jan 2004 16:54:07 -0000
@@ -65,6 +65,41 @@
  *     non-*_r functions.
  */
  
+#if defined(FRONTEND)
+#include "libpq-fe.h"
+#include "libpq-int.h"
+/*
+ * 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;
+}
+#endif
 
 /*
  * Wrapper around strerror and strerror_r to use the former if it is
@@ -82,15 +117,14 @@
 #else
 
 #if defined(FRONTEND) && defined(ENABLE_THREAD_SAFETY) && 
defined(NEED_REENTRANT_FUNCS) && !defined(HAVE_STRERROR_R)
-       static pthread_mutex_t strerror_lock = PTHREAD_MUTEX_INITIALIZER;
-       pthread_mutex_lock(&strerror_lock);
+       g_threadlock(true);
 #endif
 
        /* no strerror_r() available, just use strerror */
        StrNCpy(strerrbuf, strerror(errnum), buflen);
 
 #if defined(FRONTEND) && defined(ENABLE_THREAD_SAFETY) && 
defined(NEED_REENTRANT_FUNCS) && !defined(HAVE_STRERROR_R)
-       pthread_mutex_unlock(&strerror_lock);
+       g_threadlock(false);
 #endif
 
        return strerrbuf;
@@ -118,8 +152,7 @@
 #else
 
 #if defined(FRONTEND) && defined(ENABLE_THREAD_SAFETY) && 
defined(NEED_REENTRANT_FUNCS) && !defined(HAVE_GETPWUID_R)
-       static pthread_mutex_t getpwuid_lock = PTHREAD_MUTEX_INITIALIZER;
-       pthread_mutex_lock(&getpwuid_lock);
+       g_threadlock(true);
 #endif
 
        /* no getpwuid_r() available, just use getpwuid() */
@@ -161,7 +194,7 @@
                errno = ERANGE;
        }
 
-       pthread_mutex_unlock(&getpwuid_lock);
+       g_threadlock(false);
 #endif
 #endif
        return (*result == NULL) ? -1 : 0;
@@ -192,8 +225,7 @@
 #else
 
 #if defined(FRONTEND) && defined(ENABLE_THREAD_SAFETY) && 
defined(NEED_REENTRANT_FUNCS) && !defined(HAVE_GETHOSTBYNAME_R)
-       static pthread_mutex_t gethostbyname_lock = PTHREAD_MUTEX_INITIALIZER;
-       pthread_mutex_lock(&gethostbyname_lock);
+       g_threadlock(true);
 #endif
 
        /* no gethostbyname_r(), just use gethostbyname() */
@@ -269,7 +301,7 @@
                *herrno = h_errno;
                
 #if defined(FRONTEND) && defined(ENABLE_THREAD_SAFETY) && 
defined(NEED_REENTRANT_FUNCS) && !defined(HAVE_GETHOSTBYNAME_R)
-       pthread_mutex_unlock(&gethostbyname_lock);
+       g_threadlock(false);
 #endif
 
        if (*result != NULL)
---------------------------(end of broadcast)---------------------------
TIP 3: if posting/reading through Usenet, please send an appropriate
      subscribe-nomail command to [EMAIL PROTECTED] so that your
      message can get through to the mailing list cleanly

Reply via email to