On 04/12/2018 08:41 AM, Marc Hartmayer wrote:
No lock is required to access 'priv->conn' therefore we can shrink the critical sections.
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com> --- src/remote/remote_daemon_dispatch.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
While this follows other usages, that means virMutexUnlock in cleanup: may be called without holding the lock. Perhaps this and other *Register/*Deregister API's should take the lock around anything that could end up in cleanup - perhaps even the same history as the previous patch John
diff --git a/src/remote/remote_daemon_dispatch.c b/src/remote/remote_daemon_dispatch.c index 041e7c7440bf..d6699418392e 100644 --- a/src/remote/remote_daemon_dispatch.c +++ b/src/remote/remote_daemon_dispatch.c @@ -3841,8 +3841,6 @@ remoteDispatchConnectRegisterCloseCallback(virNetServerPtr server, virNetServerClientGetPrivateData(client); virNetServerProgramPtr program;
- virMutexLock(&priv->lock); - if (!priv->conn) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open")); goto cleanup; @@ -3853,6 +3851,8 @@ remoteDispatchConnectRegisterCloseCallback(virNetServerPtr server, goto cleanup; }
+ virMutexLock(&priv->lock); + if (VIR_ALLOC(callback) < 0) goto cleanup; callback->client = virObjectRef(client); @@ -3887,13 +3887,13 @@ remoteDispatchConnectUnregisterCloseCallback(virNetServerPtr server ATTRIBUTE_UN struct daemonClientPrivate *priv = virNetServerClientGetPrivateData(client);
- virMutexLock(&priv->lock); - if (!priv->conn) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open")); goto cleanup; }
+ virMutexLock(&priv->lock); + if (virConnectUnregisterCloseCallback(priv->conn, remoteRelayConnectionClosedEvent) < 0) goto cleanup;