From d8b367496eed774e317531898016613b1093d9cd Mon Sep 17 00:00:00 2001
From: Matthias Bolte <matthias.bolte@googlemail.com>
Date: Wed, 24 Nov 2010 18:10:14 +0100
Subject: [PATCH] Always close drivers when a virConnectPtr is released

virConnectClose calls virUnrefConnect which in turn closes
all open drivers when the refcount of that connection dropped
to zero. This works fine when you free all other objects that
hold a ref to the connection before you close it, because in
this case virUnrefConnect is the one that removes the last
ref to the connection.

But it doesn't work when you close the connection first before
freeing the other objects. This is because the other virUnref*
functions call virReleaseConnect when they detect that the
connection's refcount dropped to zero. In this case another
virUnref* function (different from virUnrefConnect) removes the
last ref to the connection. This results in not closing the
open drivers and leaking things that should have been cleaned
up in the driver close functions.

To fix this move the driver close calls to virReleaseConnect.
---
 src/datatypes.c | 47 +++++++++++++++++++++++------------------------
 1 file changed, 23 insertions(+), 24 deletions(-)

diff --git a/src/datatypes.c b/src/datatypes.c
index 46009aeb72..9817538a0d 100644
--- a/src/datatypes.c
+++ b/src/datatypes.c
@@ -247,6 +247,29 @@ failed:
 static void
 virReleaseConnect(virConnectPtr conn) {
     DEBUG("release connection %p", conn);
+
+    /* make sure to release the connection lock before we call the
+     * close() callbacks, otherwise we will deadlock if an error
+     * is raised by any of the callbacks */
+    virMutexUnlock(&conn->lock);
+
+    if (conn->networkDriver)
+        conn->networkDriver->close (conn);
+    if (conn->interfaceDriver)
+        conn->interfaceDriver->close (conn);
+    if (conn->storageDriver)
+        conn->storageDriver->close (conn);
+    if (conn->deviceMonitor)
+        conn->deviceMonitor->close (conn);
+    if (conn->secretDriver)
+        conn->secretDriver->close (conn);
+    if (conn->nwfilterDriver)
+        conn->nwfilterDriver->close (conn);
+    if (conn->driver)
+        conn->driver->close (conn);
+
+    virMutexLock(&conn->lock);
+
     if (conn->domains != NULL)
         virHashFree(conn->domains, (virHashDeallocator) virDomainFreeName);
     if (conn->networks != NULL)
@@ -295,30 +318,6 @@ virUnrefConnect(virConnectPtr conn) {
     conn->refs--;
     refs = conn->refs;
     if (refs == 0) {
-        /* make sure to release the connection lock before we call the
-         * close() callbacks, otherwise we will deadlock if an error
-         * is raised by any of the callbacks
-         */
-        virMutexUnlock(&conn->lock);
-        if (conn->networkDriver)
-            conn->networkDriver->close (conn);
-        if (conn->interfaceDriver)
-            conn->interfaceDriver->close (conn);
-        if (conn->storageDriver)
-            conn->storageDriver->close (conn);
-        if (conn->deviceMonitor)
-            conn->deviceMonitor->close (conn);
-        if (conn->secretDriver)
-            conn->secretDriver->close (conn);
-        if (conn->nwfilterDriver)
-            conn->nwfilterDriver->close (conn);
-        if (conn->driver)
-            conn->driver->close (conn);
-
-        /* reacquire the connection lock since virReleaseConnect expects
-         * it to already be held
-         */
-        virMutexLock(&conn->lock);
         virReleaseConnect(conn);
         /* Already unlocked mutex */
         return (0);