* Heikki Linnakangas (hlinnakan...@vmware.com) wrote: > Umm, with that patch, pqsecure_destroy() is never called. The "if > (conn->ssl)" test that's now at the end of the close_SSL function is > never true, because conn->ssl is set to NULL earlier.
Yeah, got ahead of myself, as Andres pointed out.
> I'm afraid the "move_locks.diff" patch you posted earlier is also
> broken; close_SSL() is called in error scenarios from
> pqsecure_open_client(), while already holding the mutex. So it will
> deadlock with itself if the connection cannot be established.
Sorry, I was really just tossing it up to see if it really did avoid
this particular deadlock. I'm running some tests now with the
attached to see if I can get it to deadlock now.
Thanks,
Stephen
diff --git a/src/interfaces/libpq/fe-secure.c b/src/interfaces/libpq/fe-secure.c
new file mode 100644
index 3bd0113..c008dcf 100644
*** a/src/interfaces/libpq/fe-secure.c
--- b/src/interfaces/libpq/fe-secure.c
*************** open_client_SSL(PGconn *conn)
*** 1537,1542 ****
--- 1537,1544 ----
static void
close_SSL(PGconn *conn)
{
+ bool destroy_needed = conn->ssl ? true : false;
+
if (conn->ssl)
{
DECLARE_SIGPIPE_INFO(spinfo);
*************** close_SSL(PGconn *conn)
*** 1545,1551 ****
SSL_shutdown(conn->ssl);
SSL_free(conn->ssl);
conn->ssl = NULL;
- pqsecure_destroy();
/* We have to assume we got EPIPE */
REMEMBER_EPIPE(spinfo, true);
RESTORE_SIGPIPE(conn, spinfo);
--- 1547,1552 ----
*************** close_SSL(PGconn *conn)
*** 1565,1570 ****
--- 1566,1582 ----
conn->engine = NULL;
}
#endif
+
+ /*
+ * This will remove our SSL locking hooks, if this is the last SSL
+ * connection, which means we must wait to call it until after all
+ * SSL calls have been made, otherwise we can end up with a race
+ * condition and possible deadlocks.
+ *
+ * See comments above destroy_ssl_system().
+ */
+ if (destroy_needed)
+ pqsecure_destroy();
}
/*
signature.asc
Description: Digital signature
