10:08 a.m.
I hope that this is an acceptable solution. The race is caused by
referencing a disposed connection object in a callback.
In the first patch we make sure that the object reference count
is reflecting the callback registration.
The second patch prevents the invocation of a NULL callback.
The last one tries to ensure that we don't leak connection
references.
Viktor Mihajlovski (3):
libvirt: Increase connection reference count for callbacks
remote: Don't call NULL closeFreeCallback
virsh: Unregister the connection close notifier upon termination
src/libvirt.c | 5 +++++
src/remote/remote_driver.c | 3 ++-
tools/virsh.c | 23 +++++++++++++++++------
3 files changed, 24 insertions(+), 7 deletions(-)
--
1.7.9.5
10:08 a.m.
New subject: [libvirt] [PATCH 1/3] libvirt: Increase connection reference count for callbacks
By adjusting the reference count of the connection object we
prevent races between callback function and virConnectClose.
Signed-off-by: Viktor Mihajlovski <mihajlov(a)linux.vnet.ibm.com>
---
src/libvirt.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/src/libvirt.c b/src/libvirt.c
index 02d5dd9..f7df26a 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -20184,6 +20184,8 @@ int virConnectRegisterCloseCallback(virConnectPtr conn,
return -1;
}
+ virObjectRef(conn);
+
virMutexLock(&conn->lock);
virCheckNonNullArgGoto(cb, error);
@@ -20204,6 +20206,7 @@ int virConnectRegisterCloseCallback(virConnectPtr conn,
error:
virMutexUnlock(&conn->lock);
+ virObjectUnref(conn);
virDispatchError(NULL);
return -1;
}
@@ -20253,6 +20256,8 @@ int virConnectUnregisterCloseCallback(virConnectPtr conn,
virMutexUnlock(&conn->lock);
+ virObjectUnref(conn);
+
return 0;
error:
--
1.7.9.5
10:08 a.m.
New subject: [libvirt] [PATCH 2/3] remote: Don't call NULL closeFreeCallback
Check function pointer before calling.
Signed-off-by: Viktor Mihajlovski <mihajlov(a)linux.vnet.ibm.com>
---
src/remote/remote_driver.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index 3721af9..885120e 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -358,7 +358,8 @@ static void remoteClientCloseFunc(virNetClientPtr client
ATTRIBUTE_UNUSED,
closeCallback(conn, reason, closeOpaque);
virMutexLock(&conn->lock);
conn->closeDispatch = false;
- if (conn->closeUnregisterCount != closeUnregisterCount)
+ if (conn->closeUnregisterCount != closeUnregisterCount &&
+ closeFreeCallback)
closeFreeCallback(closeOpaque);
}
virMutexUnlock(&conn->lock);
--
1.7.9.5
10:08 a.m.
New subject: [libvirt] [PATCH 3/3] virsh: Unregister the connection close notifier upon termination
Before closing the connection we unregister the close callback
to prevent a reference leak. We make sure that we only unregister
if we have previously registered a callback (not the case for
virsh connect!). Further, the messages on virConnectClose != 0
are a bit more specific now.
Signed-off-by: Viktor Mihajlovski <mihajlov(a)linux.vnet.ibm.com>
---
tools/virsh.c | 23 +++++++++++++++++------
1 file changed, 17 insertions(+), 6 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c
index d822e09..4be8fa0 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -311,6 +311,8 @@ vshCatchDisconnect(virConnectPtr conn ATTRIBUTE_UNUSED,
disconnected++;
}
+static int callback_registered = 0;
+
/*
* vshReconnect:
*
@@ -336,9 +338,13 @@ vshReconnect(vshControl *ctl)
else
vshError(ctl, "%s", _("failed to connect to the
hypervisor"));
} else {
- if (virConnectRegisterCloseCallback(ctl->conn, vshCatchDisconnect,
- NULL, NULL) < 0)
- vshError(ctl, "%s", _("Unable to register disconnect
callback"));
+ if (!callback_registered) {
+ if (virConnectRegisterCloseCallback(ctl->conn, vshCatchDisconnect,
+ NULL, NULL) == 0)
+ callback_registered = 1;
+ else
+ vshError(ctl, "%s", _("Unable to register disconnect
callback"));
+ }
if (connected)
vshError(ctl, "%s", _("Reconnected to the hypervisor"));
}
@@ -2672,9 +2678,14 @@ vshDeinit(vshControl *ctl)
VIR_FREE(ctl->name);
if (ctl->conn) {
int ret;
- if ((ret = virConnectClose(ctl->conn)) != 0) {
- vshError(ctl, _("Failed to disconnect from the hypervisor, %d leaked
reference(s)"), ret);
- }
+ if (callback_registered &&
+ virConnectUnregisterCloseCallback(ctl->conn, vshCatchDisconnect) < 0)
+ vshError(ctl, "%s", _("Failed to unregister disconnect
callback"));
+ ret = virConnectClose(ctl->conn);
+ if (ret < 0)
+ vshError(ctl, "%s", _("Failed to disconnect from the
hypervisor"));
+ else if (ret > 0)
+ vshError(ctl, "%s", _("Leaked reference(s) after disconnect
from the hypervisor"));
}
virResetLastError();
--
1.7.9.5
4272
days inactive
4272
days old
3 comments
1 participants
participants (1)
-
Viktor Mihajlovski