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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
