Re: libpq thread safety

Tom Lane wrote:

>Personally I find diff -u format completely unreadable :-(.  Send
>"diff -c" if you want useful commentary.
>
>
diff -c is attached. I've removed the signal changes, they are
unrelated. I'll resent them separately.

--
    Manfred
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    11 Jan 2004 17:29:38 -0000
***************
*** 458,463 ****
--- 458,480 ----
   */
  pqbool PQinSend(void);

+ /* === 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
  }
  #endif
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 -c -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 17:29:38 -0000
***************
*** 448,453 ****
--- 448,460 ----
  #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 -c -r1.14 thread.c
*** src/port/thread.c    29 Nov 2003 22:41:31 -0000    1.14
--- src/port/thread.c    11 Jan 2004 17:29:38 -0000
***************
*** 65,70 ****
--- 65,105 ----
   *    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,96 ****
  #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);
  #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);
  #endif

      return strerrbuf;
--- 117,130 ----
  #else

  #if defined(FRONTEND) && defined(ENABLE_THREAD_SAFETY) && defined(NEED_REENTRANT_FUNCS) && !defined(HAVE_STRERROR_R)
!     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)
!     g_threadlock(false);
  #endif

      return strerrbuf;
***************
*** 118,125 ****
  #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);
  #endif

      /* no getpwuid_r() available, just use getpwuid() */
--- 152,158 ----
  #else

  #if defined(FRONTEND) && defined(ENABLE_THREAD_SAFETY) && defined(NEED_REENTRANT_FUNCS) && !defined(HAVE_GETPWUID_R)
!     g_threadlock(true);
  #endif

      /* no getpwuid_r() available, just use getpwuid() */
***************
*** 161,167 ****
          errno = ERANGE;
      }

!     pthread_mutex_unlock(&getpwuid_lock);
  #endif
  #endif
      return (*result == NULL) ? -1 : 0;
--- 194,200 ----
          errno = ERANGE;
      }

!     g_threadlock(false);
  #endif
  #endif
      return (*result == NULL) ? -1 : 0;
***************
*** 192,199 ****
  #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);
  #endif

      /* no gethostbyname_r(), just use gethostbyname() */
--- 225,231 ----
  #else

  #if defined(FRONTEND) && defined(ENABLE_THREAD_SAFETY) && defined(NEED_REENTRANT_FUNCS) &&
!defined(HAVE_GETHOSTBYNAME_R)
!     g_threadlock(true);
  #endif

      /* no gethostbyname_r(), just use gethostbyname() */
***************
*** 269,275 ****
          *herrno = h_errno;

  #if defined(FRONTEND) && defined(ENABLE_THREAD_SAFETY) && defined(NEED_REENTRANT_FUNCS) &&
!defined(HAVE_GETHOSTBYNAME_R)
!     pthread_mutex_unlock(&gethostbyname_lock);
  #endif

      if (*result != NULL)
--- 301,307 ----
          *herrno = h_errno;

  #if defined(FRONTEND) && defined(ENABLE_THREAD_SAFETY) && defined(NEED_REENTRANT_FUNCS) &&
!defined(HAVE_GETHOSTBYNAME_R)
!     g_threadlock(false);
  #endif

      if (*result != NULL)