Re: libpq thread safety

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(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073