[libvirt] [PATCH] Correct two memory leaks triggered by udev events
Dave Allan
dallan at redhat.com
Sat May 29 03:13:29 UTC 2010
On Tue, May 25, 2010 at 04:10:27AM +1200, Nigel Jones wrote:
> Hi List,
>
> Please find below a patch that should correct two memory leaks within
> the udev device handling code.
>
> The issue is triggered by 'add' udev calls and will slowly cause
> libvirtd to consume the majority of system memory.
>
> The first part of the patch deals with udevAddOneDevice() and frees
> the memory allocated to 'def' if the function would return -1 (an
> error condition) due to the inability to look up udev properties (for
> example, if the udev device is already removed).
>
> The second part of the patch deals with a remaining 8kB/cycle memory
> leak, in which the udev device reference isn't released at the end of
> running udevEventHandleCallback().
>
> I'm happy to answer any questions about the patch, there is also a bit
> more background at
> https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/571093
Hi Nigel,
Thanks for reporting this, and the patch. While reviewing your patch,
I found an additional leak in the remove case which I have also
addressed in the attached patch, which is otherwise just a slight
modification of yours. I was able to reproduce the leaks by running a
device add/remove cycle, and I do not see memory leakage after
applying this patch. Let me know if it fixes it for you as well.
Dave
-------------- next part --------------
>From 076666855d9a9c481ae4c983462e0d3afef92fb7 Mon Sep 17 00:00:00 2001
From: David Allan <dallan at redhat.com>
Date: Fri, 28 May 2010 22:22:05 -0400
Subject: [PATCH 1/1] Fix leaks in udev device add/remove
* This patch is a modification of a patch submitted by Nigel Jones.
It fixes several memory leaks on device addition/removal:
1. Free the virNodeDeviceDefPtr in udevAddOneDevice if the return
value is non-zero
2. The node device reference must always be released after the device
has been processed, so move the call to udev_device_unref into
udevAddOneDevice and add a call to udev_device_unref to
udevRemoveOneDevice
---
src/node_device/node_device_udev.c | 12 ++++++++++--
1 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c
index 6e3ecd7..5193f5b 100644
--- a/src/node_device/node_device_udev.c
+++ b/src/node_device/node_device_udev.c
@@ -1211,6 +1211,8 @@ static int udevRemoveOneDevice(struct udev_device *device)
}
nodeDeviceUnlock(driverState);
+ udev_device_unref(device);
+
return ret;
}
@@ -1309,13 +1311,14 @@ static int udevAddOneDevice(struct udev_device *device)
goto out;
}
+ /* If this is a device change, the old definition will be freed
+ * and the current definition will take its place. */
nodeDeviceLock(driverState);
dev = virNodeDeviceAssignDef(&driverState->devs, def);
nodeDeviceUnlock(driverState);
if (dev == NULL) {
VIR_ERROR(_("Failed to create device for '%s'"), def->name);
- virNodeDeviceDefFree(def);
goto out;
}
@@ -1324,6 +1327,12 @@ static int udevAddOneDevice(struct udev_device *device)
ret = 0;
out:
+ if (ret != 0) {
+ virNodeDeviceDefFree(def);
+ }
+
+ udev_device_unref(device);
+
return ret;
}
@@ -1343,7 +1352,6 @@ static int udevProcessDeviceListEntry(struct udev *udev,
VIR_INFO("Failed to create node device for udev device '%s'",
name);
}
- udev_device_unref(device);
ret = 0;
}
--
1.7.0.1
More information about the libvir-list
mailing list