Re: libpq thread locking during SSL connection start

Stephen Frost wrote:
> All,
> 
>   I wanted to highlight the below commit as being a significant enough
>   change that it warrents being seen on -hackers and not just
>   -committers.  If you use SSL with libpq, particularly in a threaded
>   mode/environment, please take a look/test this change.  Prior to the
>   patch, we would crash pretty easily when using client certificates
>   with multiple threads; I was unable to get it to crash after the
>   patch.  That said, there's also risk that this patch will cause
>   slow-downs in SSL connections when using threads due to the additional
>   locking.  I'm not sure that's a heavily used case, but none-the-less,
>   if you do that, please considering testing this patch.

Now that I look at it, I think there are a couple of problems with this
patch, particularly when pthread_mutex_lock fails.  I grant you that
that is a very improbable condition, but if so then we should Assert()
against it or something like that.  (Since I am not sure what would
constitute good behavior from Assert() in libpq, I tend to think this is
not a good idea.)

pgsecure_open_client() returns -1 if it can't lock the mutex.  This is a
problem because the callers are not prepared for that return value.  I
think it should return PGRES_POLLING_FAILED instead, after setting an
appropriate error message in conn->errorMessage.

initialize_SSL() fails to set an error message.  The return code of -1
seems fine here.

-- 
Álvaro Herrera                http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services