Bruce Momjian wrote:
> Bruce Momjian wrote:
> > Magnus Hagander wrote:
> > > Attached patch adds some error checking to the thread locking
> > > stuff in libpq. Previously, if thread locking failed for some
> > > reason, we would just fall through and do things without locking.
> > > This patch makes us abort() instead. It's not the greatest thing
> > > probably, but our API doesn't let us pass back return values...
> >
> > I have looked over the patch and it seems fine, though I am
> > concerned about the abort() case with no output. I realize stderr
> > might be going nowhere, but in fe-print.c we do an fprintf(stderr)
> > for memory failures so for consistency I think we should do the
> > same here. If there is concern about code bloat, I suggest a macro
> > at the top of the file for thread failure exits:
> >
> > #define THEAD_FAILURE(str) \
> > do { \
> > fprintf(stderr, libpq_gettext("Thread failure:
> > %s\n")); \ exit(1); \
> > } while(0)
>
> Oh, this is Tom saying he doesn't like stderr and the added code lines
> for failure:
>
> http://archives.postgresql.org/pgsql-patches/2008-04/msg00254.php
>
> I think the macro and consistency suggest doing as I outlined.
Does this one look like what you're suggesting?
//Magnus
Index: fe-connect.c
===================================================================
RCS file: /cvsroot/pgsql/src/interfaces/libpq/fe-connect.c,v
retrieving revision 1.357
diff -c -r1.357 fe-connect.c
*** fe-connect.c 31 Mar 2008 02:43:14 -0000 1.357
--- fe-connect.c 16 May 2008 14:03:10 -0000
***************
*** 3835,3848 ****
while (InterlockedExchange(&mutex_initlock, 1) == 1)
/* loop, another thread own the lock */ ;
if (singlethread_lock == NULL)
! pthread_mutex_init(&singlethread_lock, NULL);
InterlockedExchange(&mutex_initlock, 0);
}
#endif
if (acquire)
! pthread_mutex_lock(&singlethread_lock);
else
! pthread_mutex_unlock(&singlethread_lock);
#endif
}
--- 3835,3857 ----
while (InterlockedExchange(&mutex_initlock, 1) == 1)
/* loop, another thread own the lock */ ;
if (singlethread_lock == NULL)
! {
! if (pthread_mutex_init(&singlethread_lock, NULL))
! PGTHREAD_ERROR("failed to initialize mutex");
! }
InterlockedExchange(&mutex_initlock, 0);
}
#endif
if (acquire)
! {
! if (pthread_mutex_lock(&singlethread_lock))
! PGTHREAD_ERROR("failed to lock mutex");
! }
else
! {
! if (pthread_mutex_unlock(&singlethread_lock))
! PGTHREAD_ERROR("failed to unlock mutex");
! }
#endif
}
Index: fe-secure.c
===================================================================
RCS file: /cvsroot/pgsql/src/interfaces/libpq/fe-secure.c,v
retrieving revision 1.104
diff -c -r1.104 fe-secure.c
*** fe-secure.c 31 Mar 2008 02:43:14 -0000 1.104
--- fe-secure.c 16 May 2008 14:03:11 -0000
***************
*** 796,807 ****
pq_lockingcallback(int mode, int n, const char *file, int line)
{
if (mode & CRYPTO_LOCK)
! pthread_mutex_lock(&pq_lockarray[n]);
else
! pthread_mutex_unlock(&pq_lockarray[n]);
}
#endif /* ENABLE_THREAD_SAFETY */
static int
init_ssl_system(PGconn *conn)
{
--- 796,816 ----
pq_lockingcallback(int mode, int n, const char *file, int line)
{
if (mode & CRYPTO_LOCK)
! {
! if (pthread_mutex_lock(&pq_lockarray[n]))
! PGTHREAD_ERROR("failed to lock mutex");
! }
else
! {
! if (pthread_mutex_unlock(&pq_lockarray[n]))
! PGTHREAD_ERROR("failed to unlock mutex");
! }
}
#endif /* ENABLE_THREAD_SAFETY */
+ /*
+ * Also see similar code in fe-connect.c, default_threadlock()
+ */
static int
init_ssl_system(PGconn *conn)
{
***************
*** 817,827 ****
while (InterlockedExchange(&mutex_initlock, 1) == 1)
/* loop, another thread own the lock */ ;
if (init_mutex == NULL)
! pthread_mutex_init(&init_mutex, NULL);
InterlockedExchange(&mutex_initlock, 0);
}
#endif
! pthread_mutex_lock(&init_mutex);
if (pq_initssllib && pq_lockarray == NULL)
{
--- 826,840 ----
while (InterlockedExchange(&mutex_initlock, 1) == 1)
/* loop, another thread own the lock */ ;
if (init_mutex == NULL)
! {
! if (pthread_mutex_init(&init_mutex, NULL))
! return -1;
! }
InterlockedExchange(&mutex_initlock, 0);
}
#endif
! if (pthread_mutex_lock(&init_mutex))
! return -1;
if (pq_initssllib && pq_lockarray == NULL)
{
***************
*** 836,842 ****
return -1;
}
for (i = 0; i < CRYPTO_num_locks(); i++)
! pthread_mutex_init(&pq_lockarray[i], NULL);
CRYPTO_set_locking_callback(pq_lockingcallback);
}
--- 849,858 ----
return -1;
}
for (i = 0; i < CRYPTO_num_locks(); i++)
! {
! if (pthread_mutex_init(&pq_lockarray[i], NULL))
! return -1;
! }
CRYPTO_set_locking_callback(pq_lockingcallback);
}
Index: libpq-int.h
===================================================================
RCS file: /cvsroot/pgsql/src/interfaces/libpq/libpq-int.h,v
retrieving revision 1.129
diff -c -r1.129 libpq-int.h
*** libpq-int.h 1 Jan 2008 19:46:00 -0000 1.129
--- libpq-int.h 16 May 2008 14:03:11 -0000
***************
*** 439,444 ****
--- 439,451 ----
#ifdef ENABLE_THREAD_SAFETY
extern pgthreadlock_t pg_g_threadlock;
+ #define PGTHREAD_ERROR(msg) \
+ do { \
+ fprintf(stderr, "%s\n", msg); \
+ exit(1); \
+ } while (0)
+
+
#define pglock_thread() pg_g_threadlock(true)
#define pgunlock_thread() pg_g_threadlock(false)
#else
Index: pthread-win32.c
===================================================================
RCS file: /cvsroot/pgsql/src/interfaces/libpq/pthread-win32.c,v
retrieving revision 1.15
diff -c -r1.15 pthread-win32.c
*** pthread-win32.c 1 Jan 2008 19:46:00 -0000 1.15
--- pthread-win32.c 16 May 2008 14:03:11 -0000
***************
*** 32,51 ****
return NULL;
}
! void
pthread_mutex_init(pthread_mutex_t *mp, void *attr)
{
*mp = CreateMutex(0, 0, 0);
}
! void
pthread_mutex_lock(pthread_mutex_t *mp)
{
! WaitForSingleObject(*mp, INFINITE);
}
! void
pthread_mutex_unlock(pthread_mutex_t *mp)
{
! ReleaseMutex(*mp);
}
--- 32,58 ----
return NULL;
}
! int
pthread_mutex_init(pthread_mutex_t *mp, void *attr)
{
*mp = CreateMutex(0, 0, 0);
+ if (*mp == NULL)
+ return 1;
+ return 0;
}
! int
pthread_mutex_lock(pthread_mutex_t *mp)
{
! if (WaitForSingleObject(*mp, INFINITE) != WAIT_OBJECT_0)
! return 1;
! return 0;
}
! int
pthread_mutex_unlock(pthread_mutex_t *mp)
{
! if (!ReleaseMutex(*mp))
! return 1;
! return 0;
}
--
Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches
