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
