* Stephen Frost (sfr...@snowman.net) wrote: > * Andres Freund (and...@2ndquadrant.com) wrote: > > Hm. close_SSL() first does pqsecure_destroy() which will unset the > > callbacks, and the count and then goes on to do X509_free() and > > ENGINE_finish(), ENGINE_free() if either is used. > > > > It's not implausible that one of those actually needs locking. I doubt > > engines play a role here, but, without having looked at the testcase, > > X509_free() might be a possibility. > > Unfortunately, while I can still easily get the deadlock to happen when > the hooks are reset, the hooks don't appear to ever get called when > ssl_open_connections is set to zero. You have a good point about the > additional SSL calls after the hooks are unloaded though, I wonder if > holding the ssl_config_mutex lock over all of close_SSL might be more > sensible..
I went ahead and moved the locks to be around all of close_SSL() and
haven't been able to reproduce the deadlock, so perhaps those calls are
the issue and what's happening is that another thread is dropping or
adding the hooks in a common place while the X509_free, etc, are trying
to figure out if they should be calling the locking functions or not,
but there's a race because there's no higher-level locking happening
around those.
Attached is a patch to move those and which doesn't deadlock for me.
Thoughts?
Thanks,
Stephen
diff --git a/src/interfaces/libpq/fe-secure.c b/src/interfaces/libpq/fe-secure.c
new file mode 100644
index 3bd0113..e14c301 100644
*** a/src/interfaces/libpq/fe-secure.c
--- b/src/interfaces/libpq/fe-secure.c
*************** destroy_ssl_system(void)
*** 1009,1017 ****
{
#ifdef ENABLE_THREAD_SAFETY
/* Mutex is created in initialize_ssl_system() */
- if (pthread_mutex_lock(&ssl_config_mutex))
- return;
-
if (pq_init_crypto_lib && ssl_open_connections > 0)
--ssl_open_connections;
--- 1009,1014 ----
*************** destroy_ssl_system(void)
*** 1031,1037 ****
*/
}
- pthread_mutex_unlock(&ssl_config_mutex);
#endif
}
--- 1028,1033 ----
*************** open_client_SSL(PGconn *conn)
*** 1537,1542 ****
--- 1533,1543 ----
static void
close_SSL(PGconn *conn)
{
+ #ifdef ENABLE_THREAD_SAFETY
+ if (pthread_mutex_lock(&ssl_config_mutex))
+ return;
+ #endif
+
if (conn->ssl)
{
DECLARE_SIGPIPE_INFO(spinfo);
*************** close_SSL(PGconn *conn)
*** 1565,1570 ****
--- 1566,1575 ----
conn->engine = NULL;
}
#endif
+
+ #ifdef ENABLE_THREAD_SAFETY
+ pthread_mutex_unlock(&ssl_config_mutex);
+ #endif
}
/*
signature.asc
Description: Digital signature
