Re: libpq thread safety - Mailing list pgsql-hackers

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);