Hi Claudio,
sorry for the delay, I need to focuse one something else ATM !
On Mon, Sep 24, 2012 at 04:11:19PM +0200, Claudio Bley wrote:
At Wed, 12 Sep 2012 20:23:28 +0800,
Daniel Veillard wrote:
>
> > I think we must provide the free functions for all the memory allocated by
libvirt.
>
> Okay, can you work on making a patch ? To be honnest I'm very unlikely
> to have time for this in the short term,
I did notice the same thing (but was on vacation for quite some
time). I already have a fix for virDomainGetSchedulerType in my local
repository (see below).
okay i see
I'm using virFree to free the allocated memory which adds
another
level of indirection (PointerByReference). This is because JNA does
not provide access to the C free() function by itself, AFAICS.
-- 8< ---
From cb58e9538afbfd7ce09c5c2a93e403a809e77113 Mon Sep 17 00:00:00 2001
From: Claudio Bley <cbley(a)av-test.de>
Date: Fri, 24 Aug 2012 11:09:49 +0200
Subject: [PATCH] Fix memory leak for virDomainGetSchedulerType.
The string returned by virDomainGetSchedulerType must be free'd
explicitly.
JNA docs say that we have to use a Pointer as a return type and
free it ourselves. So, simply use virFree for that matter.
---
src/main/java/org/libvirt/Domain.java | 13 +++++++++----
src/main/java/org/libvirt/jna/Libvirt.java | 6 +++++-
2 files changed, 14 insertions(+), 5 deletions(-)
diff --git a/src/main/java/org/libvirt/Domain.java
b/src/main/java/org/libvirt/Domain.java
index 1f15ee8..fc1f665 100644
--- a/src/main/java/org/libvirt/Domain.java
+++ b/src/main/java/org/libvirt/Domain.java
@@ -14,7 +14,9 @@ import org.libvirt.jna.virVcpuInfo;
import com.sun.jna.Native;
import com.sun.jna.NativeLong;
+import com.sun.jna.Pointer;
import com.sun.jna.ptr.IntByReference;
+import com.sun.jna.ptr.PointerByReference;
/**
* A virtual machine defined within libvirt.
@@ -443,9 +445,11 @@ public class Domain {
public SchedParameter[] getSchedulerParameters() throws LibvirtException {
IntByReference nParams = new IntByReference();
SchedParameter[] returnValue = new SchedParameter[0];
- String scheduler = libvirt.virDomainGetSchedulerType(VDP, nParams);
+ Pointer pScheduler = libvirt.virDomainGetSchedulerType(VDP, nParams);
processError();
- if (scheduler != null) {
+ if (pScheduler != null) {
+ String scheduler = pScheduler.getString(0);
+ libvirt.virFree(new PointerByReference(pScheduler));
virSchedParameter[] nativeParams = new
virSchedParameter[nParams.getValue()];
returnValue = new SchedParameter[nParams.getValue()];
libvirt.virDomainGetSchedulerParameters(VDP, nativeParams, nParams);
@@ -470,10 +474,11 @@ public class Domain {
*/
public String[] getSchedulerType() throws LibvirtException {
IntByReference nParams = new IntByReference();
- String returnValue = libvirt.virDomainGetSchedulerType(VDP, nParams);
+ Pointer pScheduler = libvirt.virDomainGetSchedulerType(VDP, nParams);
processError();
String[] array = new String[1];
- array[0] = returnValue;
+ array[0] = pScheduler.getString(0);
+ libvirt.virFree(new PointerByReference(pScheduler));
return array;
}
diff --git a/src/main/java/org/libvirt/jna/Libvirt.java
b/src/main/java/org/libvirt/jna/Libvirt.java
index dc3a54b..e68d9ed 100644
--- a/src/main/java/org/libvirt/jna/Libvirt.java
+++ b/src/main/java/org/libvirt/jna/Libvirt.java
@@ -7,6 +7,7 @@ import com.sun.jna.NativeLong;
import com.sun.jna.Pointer;
import com.sun.jna.ptr.IntByReference;
import com.sun.jna.ptr.LongByReference;
+import com.sun.jna.ptr.PointerByReference;
/**
* The libvirt interface which is exposed via JNA. The complete API is
@@ -55,6 +56,9 @@ import com.sun.jna.ptr.LongByReference;
*
*/
public interface Libvirt extends Library {
+ // Memory management
+ void virFree(PointerByReference ptr);
+
// Callbacks
/**
* Callback interface for authorization
@@ -186,7 +190,7 @@ public interface Libvirt extends Library {
public String virDomainGetOSType(DomainPointer virDomainPtr);
public int virDomainGetSchedulerParameters(DomainPointer virDomainPtr,
virSchedParameter[] params,
IntByReference nparams);
- public String virDomainGetSchedulerType(DomainPointer virDomainPtr, IntByReference
nparams);
+ Pointer virDomainGetSchedulerType(DomainPointer virDomainPtr, IntByReference
nparams);
public int virDomainGetUUID(DomainPointer virDomainPtr, byte[] uuidString);
public int virDomainGetUUIDString(DomainPointer virDomainPtr, byte[] uuidString);
public int virDomainGetVcpus(DomainPointer virDomainPtr, virVcpuInfo[] info, int
maxInfo, byte[] cpumaps, int maplen);
Okay, that makes sense. First do you have a small pointer indicating
where in JNA that kind of native deallocation must take place, since
most of the time JNA can do the marshalling all by itself ? And second
would you have an idea how to systematically detect such leaks, the
kind of loop suggested to expose the issue is nor really practical
to chase the leaks ...
In the meantime I pushed you initial patch, thanks !
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit
http://xmlsoft.org/
daniel(a)veillard.com | Rpmfind RPM search engine
http://rpmfind.net/
http://veillard.com/ | virtualization library
http://libvirt.org/