[libvirt] [PATCH 13/15] Fix memory leaks for libvirt functions returning newly allocated memory.

Use JNA's Native.free() method to free memory. This requires JNA version 3.3.0 or later. Remove virFree from Libvirt interface. --- src/main/java/org/libvirt/Connect.java | 109 ++++++++++++++++++---------- src/main/java/org/libvirt/Domain.java | 24 ++++-- src/main/java/org/libvirt/Interface.java | 10 ++- src/main/java/org/libvirt/Library.java | 62 ++++++++++++++++ src/main/java/org/libvirt/Network.java | 17 ++++- src/main/java/org/libvirt/jna/Libvirt.java | 37 +++++----- 6 files changed, 187 insertions(+), 72 deletions(-) diff --git a/src/main/java/org/libvirt/Connect.java b/src/main/java/org/libvirt/Connect.java index ae306d6..a775ce1 100644 --- a/src/main/java/org/libvirt/Connect.java +++ b/src/main/java/org/libvirt/Connect.java @@ -19,6 +19,7 @@ import static org.libvirt.Library.libvirt; import com.sun.jna.Memory; import com.sun.jna.NativeLong; +import com.sun.jna.Pointer; import com.sun.jna.ptr.LongByReference; /** @@ -510,8 +511,12 @@ public class Connect { * description</a> */ public String getCapabilities() throws LibvirtException { - String returnValue = libvirt.virConnectGetCapabilities(VCP); - return processError(returnValue); + Pointer ptr = processError(libvirt.virConnectGetCapabilities(VCP)); + try { + return Library.getString(ptr); + } finally { + Library.free(ptr); + } } /** @@ -545,7 +550,12 @@ public class Connect { * @throws LibvirtException */ public String getHostName() throws LibvirtException { - return processError(libvirt.virConnectGetHostname(VCP)); + Pointer ptr = processError(libvirt.virConnectGetHostname(VCP)); + try { + return Library.getString(ptr); + } finally { + Library.free(ptr); + } } /** @@ -701,11 +711,13 @@ public class Connect { */ public String[] listDefinedDomains() throws LibvirtException { int maxnames = numOfDefinedDomains(); - String[] names = new String[maxnames]; if (maxnames > 0) { - processError(libvirt.virConnectListDefinedDomains(VCP, names, maxnames)); + final Pointer[] names = new Pointer[maxnames]; + final int n = processError(libvirt.virConnectListDefinedDomains(VCP, names, maxnames)); + return Library.toStringArray(names, n); + } else { + return Library.NO_STRINGS; } - return names; } /** @@ -716,12 +728,14 @@ public class Connect { * @throws LibvirtException */ public String[] listDefinedInterfaces() throws LibvirtException { - int num = numOfDefinedInterfaces(); - String[] returnValue = new String[num]; - if (num > 0) { - processError(libvirt.virConnectListDefinedInterfaces(VCP, returnValue, num)); + final int max = numOfDefinedInterfaces(); + if (max > 0) { + final Pointer[] ifs = new Pointer[max]; + final int n = processError(libvirt.virConnectListDefinedInterfaces(VCP, ifs, max)); + return Library.toStringArray(ifs, n); + } else { + return Library.NO_STRINGS; } - return returnValue; } /** @@ -733,12 +747,13 @@ public class Connect { */ public String[] listDefinedNetworks() throws LibvirtException { int maxnames = numOfDefinedNetworks(); - String[] names = new String[maxnames]; - if (maxnames > 0) { - processError(libvirt.virConnectListDefinedNetworks(VCP, names, maxnames)); + final Pointer[] names = new Pointer[maxnames]; + final int n = processError(libvirt.virConnectListDefinedNetworks(VCP, names, maxnames)); + return Library.toStringArray(names, n); + } else { + return Library.NO_STRINGS; } - return names; } /** @@ -750,9 +765,13 @@ public class Connect { */ public String[] listDefinedStoragePools() throws LibvirtException { int num = numOfDefinedStoragePools(); - String[] returnValue = new String[num]; - processError(libvirt.virConnectListDefinedStoragePools(VCP, returnValue, num)); - return returnValue; + if (num > 0) { + Pointer[] pools = new Pointer[num]; + final int n = processError(libvirt.virConnectListDefinedStoragePools(VCP, pools, num)); + return Library.toStringArray(pools, n); + } else { + return Library.NO_STRINGS; + } } /** @@ -763,12 +782,13 @@ public class Connect { */ public String[] listDevices(String capabilityName) throws LibvirtException { int maxDevices = numOfDevices(capabilityName); - String[] names = new String[maxDevices]; - if (maxDevices > 0) { - processError(libvirt.virNodeListDevices(VCP, capabilityName, names, maxDevices, 0)); + Pointer[] names = new Pointer[maxDevices]; + final int n = processError(libvirt.virNodeListDevices(VCP, capabilityName, names, maxDevices, 0)); + return Library.toStringArray(names, n); + } else { + return Library.NO_STRINGS; } - return names; } /** @@ -796,11 +816,13 @@ public class Connect { */ public String[] listInterfaces() throws LibvirtException { int num = numOfInterfaces(); - String[] returnValue = new String[num]; if (num > 0) { - processError(libvirt.virConnectListInterfaces(VCP, returnValue, num)); + Pointer[] ifs = new Pointer[num]; + final int n = processError(libvirt.virConnectListInterfaces(VCP, ifs, num)); + return Library.toStringArray(ifs, n); + } else { + return Library.NO_STRINGS; } - return returnValue; } /** @@ -811,11 +833,13 @@ public class Connect { */ public String[] listNetworkFilters() throws LibvirtException { int maxnames = numOfNetworkFilters(); - String[] names = new String[maxnames]; if (maxnames > 0) { - processError(libvirt.virConnectListNWFilters(VCP, names, maxnames)); + Pointer[] names = new Pointer[maxnames]; + final int n = processError(libvirt.virConnectListNWFilters(VCP, names, maxnames)); + return Library.toStringArray(names, n); + } else { + return Library.NO_STRINGS; } - return names; } /** @@ -827,12 +851,13 @@ public class Connect { */ public String[] listNetworks() throws LibvirtException { int maxnames = numOfNetworks(); - String[] names = new String[maxnames]; - if (maxnames > 0) { - processError(libvirt.virConnectListNetworks(VCP, names, maxnames)); + Pointer[] names = new Pointer[maxnames]; + final int n = processError(libvirt.virConnectListNetworks(VCP, names, maxnames)); + return Library.toStringArray(names, n); + } else { + return Library.NO_STRINGS; } - return names; } /** @@ -843,9 +868,13 @@ public class Connect { */ public String[] listSecrets() throws LibvirtException { int num = numOfSecrets(); - String[] returnValue = new String[num]; - processError(libvirt.virConnectListSecrets(VCP, returnValue, num)); - return returnValue; + if (num > 0) { + Pointer[] returnValue = new Pointer[num]; + final int n = processError(libvirt.virConnectListSecrets(VCP, returnValue, num)); + return Library.toStringArray(returnValue, n); + } else { + return Library.NO_STRINGS; + } } /** @@ -857,9 +886,13 @@ public class Connect { */ public String[] listStoragePools() throws LibvirtException { int num = numOfStoragePools(); - String[] returnValue = new String[num]; - processError(libvirt.virConnectListStoragePools(VCP, returnValue, num)); - return returnValue; + if (num > 0) { + Pointer[] returnValue = new Pointer[num]; + final int n = processError(libvirt.virConnectListStoragePools(VCP, returnValue, num)); + return Library.toStringArray(returnValue, n); + } else { + return Library.NO_STRINGS; + } } /** diff --git a/src/main/java/org/libvirt/Domain.java b/src/main/java/org/libvirt/Domain.java index 45fba26..1c86bd4 100644 --- a/src/main/java/org/libvirt/Domain.java +++ b/src/main/java/org/libvirt/Domain.java @@ -426,9 +426,13 @@ public class Domain { * @throws LibvirtException */ public String getOSType() throws LibvirtException { - String returnValue = libvirt.virDomainGetOSType(VDP); + Pointer ptr = libvirt.virDomainGetOSType(VDP); processError(); - return returnValue; + try { + return Library.getString(ptr); + } finally { + Library.free(ptr); + } } /** @@ -443,8 +447,8 @@ public class Domain { Pointer pScheduler = libvirt.virDomainGetSchedulerType(VDP, nParams); processError(); if (pScheduler != null) { - String scheduler = pScheduler.getString(0); - libvirt.virFree(new PointerByReference(pScheduler)); + String scheduler = Library.getString(pScheduler); + Library.free(pScheduler); virSchedParameter[] nativeParams = new virSchedParameter[nParams.getValue()]; returnValue = new SchedParameter[nParams.getValue()]; libvirt.virDomainGetSchedulerParameters(VDP, nativeParams, nParams); @@ -472,8 +476,8 @@ public class Domain { Pointer pScheduler = libvirt.virDomainGetSchedulerType(VDP, nParams); processError(); String[] array = new String[1]; - array[0] = pScheduler.getString(0); - libvirt.virFree(new PointerByReference(pScheduler)); + array[0] = Library.getString(pScheduler); + Library.free(pScheduler); return array; } @@ -569,9 +573,13 @@ public class Domain { * Description format </a> */ public String getXMLDesc(int flags) throws LibvirtException { - String returnValue = libvirt.virDomainGetXMLDesc(VDP, flags); + Pointer ptr = libvirt.virDomainGetXMLDesc(VDP, flags); processError(); - return returnValue; + try { + return Library.getString(ptr); + } finally { + Library.free(ptr); + } } /** diff --git a/src/main/java/org/libvirt/Interface.java b/src/main/java/org/libvirt/Interface.java index 684adca..71ba3da 100644 --- a/src/main/java/org/libvirt/Interface.java +++ b/src/main/java/org/libvirt/Interface.java @@ -4,6 +4,8 @@ import org.libvirt.jna.InterfacePointer; import org.libvirt.jna.Libvirt; import static org.libvirt.Library.libvirt; +import com.sun.jna.Pointer; + /** * A device which is attached to a node */ @@ -113,9 +115,13 @@ public class Interface { * @throws LibvirtException */ public String getXMLDescription(int flags) throws LibvirtException { - String xml = libvirt.virInterfaceGetXMLDesc(VIP, flags); + Pointer xml = libvirt.virInterfaceGetXMLDesc(VIP, flags); processError(); - return xml; + try { + return Library.getString(xml); + } finally { + Library.free(xml); + } } /** diff --git a/src/main/java/org/libvirt/Library.java b/src/main/java/org/libvirt/Library.java index 035ed06..0136095 100644 --- a/src/main/java/org/libvirt/Library.java +++ b/src/main/java/org/libvirt/Library.java @@ -2,15 +2,25 @@ package org.libvirt; import org.libvirt.jna.Libvirt; +import com.sun.jna.Native; +import com.sun.jna.Pointer; + /** * This class represents an instance of the JNA mapped libvirt * library. * * The library will get loaded when first accessing this class. + * + * Additionally, this class contains internal methods to ease + * implementing the public API. */ final class Library { final static Libvirt libvirt; + // an empty string array constant + // prefer this over creating empty arrays dynamically. + final static String[] NO_STRINGS = {}; + // Load the native part static { Libvirt.INSTANCE.virInitialize(); @@ -23,4 +33,56 @@ final class Library { } private Library() {} + + /** + * Free memory pointed to by ptr. + */ + static void free(Pointer ptr) { + Native.free(Pointer.nativeValue(ptr)); + Pointer.nativeValue(ptr, 0L); + } + + /** + * Convert the data pointed to by {@code ptr} to a String. + */ + static String getString(Pointer ptr) { + final long len = ptr.indexOf(0, (byte)0); + assert (len != -1): "C-Strings must be \\0 terminated."; + + final byte[] data = ptr.getByteArray(0, (int)len); + try { + return new String(data, "utf-8"); + } catch (java.io.UnsupportedEncodingException e) { + throw new RuntimeException("Libvirt problem: UTF-8 decoding error.", e); + } + } + + /** + * Calls {@link #toStringArray(Pointer[], int)}. + */ + static String[] toStringArray(Pointer[] ptrArr) { + return toStringArray(ptrArr, ptrArr.length); + } + + /** + * Convert the given array of native pointers to "char" in + * UTF-8 encoding to an array of Strings. + * + * \note The memory used by the elements of the original array + * is freed and ptrArr is modified. + */ + static String[] toStringArray(Pointer[] ptrArr, final int size) { + try { + final String[] result = new String[size]; + for (int i = 0; i < size; ++i) { + result[i] = Library.getString(ptrArr[i]); + } + return result; + } finally { + for (int i = 0; i < size; ++i) { + Library.free(ptrArr[i]); + ptrArr[i] = null; + } + } + } } diff --git a/src/main/java/org/libvirt/Network.java b/src/main/java/org/libvirt/Network.java index bdb0d78..2244c5d 100644 --- a/src/main/java/org/libvirt/Network.java +++ b/src/main/java/org/libvirt/Network.java @@ -5,6 +5,7 @@ import org.libvirt.jna.NetworkPointer; import static org.libvirt.Library.libvirt; import com.sun.jna.Native; +import com.sun.jna.Pointer; import com.sun.jna.ptr.IntByReference; /** @@ -105,9 +106,13 @@ public class Network { * @throws LibvirtException */ public String getBridgeName() throws LibvirtException { - String returnValue = libvirt.virNetworkGetBridgeName(VNP); + final Pointer ptr = libvirt.virNetworkGetBridgeName(VNP); processError(); - return returnValue; + try { + return Library.getString(ptr); + } finally { + Library.free(ptr); + } } /** @@ -178,9 +183,13 @@ public class Network { * @throws LibvirtException */ public String getXMLDesc(int flags) throws LibvirtException { - String returnValue = libvirt.virNetworkGetXMLDesc(VNP, flags); + Pointer ptr = libvirt.virNetworkGetXMLDesc(VNP, flags); processError(); - return returnValue; + try { + return Library.getString(ptr); + } finally { + Library.free(ptr); + } } /** diff --git a/src/main/java/org/libvirt/jna/Libvirt.java b/src/main/java/org/libvirt/jna/Libvirt.java index be0de3b..5d2a291 100644 --- a/src/main/java/org/libvirt/jna/Libvirt.java +++ b/src/main/java/org/libvirt/jna/Libvirt.java @@ -56,9 +56,6 @@ import com.sun.jna.ptr.PointerByReference; * */ public interface Libvirt extends Library { - // Memory management - void virFree(PointerByReference ptr); - // Callbacks /** * Callback interface for authorization @@ -126,23 +123,23 @@ public interface Libvirt extends Library { int virConnectIsEncrypted(ConnectionPointer virConnectPtr) ; int virConnectIsSecure(ConnectionPointer virConnectPtr) ; String virConnectFindStoragePoolSources(ConnectionPointer virConnectPtr, String type, String srcSpec, int flags); - String virConnectGetCapabilities(ConnectionPointer virConnectPtr); - String virConnectGetHostname(ConnectionPointer virConnectPtr); + Pointer virConnectGetCapabilities(ConnectionPointer virConnectPtr); + Pointer virConnectGetHostname(ConnectionPointer virConnectPtr); int virConnectGetLibVersion(ConnectionPointer virConnectPtr, LongByReference libVer); int virConnectGetMaxVcpus(ConnectionPointer virConnectPtr, String type); String virConnectGetType(ConnectionPointer virConnectPtr); String virConnectGetURI(ConnectionPointer virConnectPtr); int virConnectGetVersion(ConnectionPointer virConnectPtr, LongByReference hvVer); - int virConnectListDefinedDomains(ConnectionPointer virConnectPtr, String[] name, int maxnames); - int virConnectListDefinedNetworks(ConnectionPointer virConnectPtr, String[] name, int maxnames); - int virConnectListDefinedStoragePools(ConnectionPointer virConnectPtr, String[] names, int maxnames); - int virConnectListDefinedInterfaces(ConnectionPointer virConnectPtr, String[] name, int maxNames); + int virConnectListDefinedDomains(ConnectionPointer virConnectPtr, Pointer[] name, int maxnames); + int virConnectListDefinedNetworks(ConnectionPointer virConnectPtr, Pointer[] name, int maxnames); + int virConnectListDefinedStoragePools(ConnectionPointer virConnectPtr, Pointer[] names, int maxnames); + int virConnectListDefinedInterfaces(ConnectionPointer virConnectPtr, Pointer[] name, int maxNames); int virConnectListDomains(ConnectionPointer virConnectPtr, int[] ids, int maxnames); - int virConnectListInterfaces(ConnectionPointer virConnectPtr, String[] name, int maxNames); - int virConnectListNetworks(ConnectionPointer virConnectPtr, String[] name, int maxnames); - int virConnectListNWFilters(ConnectionPointer virConnectPtr, String[] name, int maxnames); - int virConnectListSecrets(ConnectionPointer virConnectPtr, String[] uids, int maxUids); - int virConnectListStoragePools(ConnectionPointer virConnectPtr, String[] names, int maxnames); + int virConnectListInterfaces(ConnectionPointer virConnectPtr, Pointer[] name, int maxNames); + int virConnectListNetworks(ConnectionPointer virConnectPtr, Pointer[] name, int maxnames); + int virConnectListNWFilters(ConnectionPointer virConnectPtr, Pointer[] name, int maxnames); + int virConnectListSecrets(ConnectionPointer virConnectPtr, Pointer[] uids, int maxUids); + int virConnectListStoragePools(ConnectionPointer virConnectPtr, Pointer[] names, int maxnames); int virConnectNumOfDefinedDomains(ConnectionPointer virConnectPtr); int virConnectNumOfDefinedNetworks(ConnectionPointer virConnectPtr); int virConnectNumOfDefinedInterfaces(ConnectionPointer virConnectPtr); @@ -203,14 +200,14 @@ public interface Libvirt extends Library { NativeLong virDomainGetMaxMemory(DomainPointer virDomainPtr); int virDomainGetMaxVcpus(DomainPointer virDomainPtr); String virDomainGetName(DomainPointer virDomainPtr); - String virDomainGetOSType(DomainPointer virDomainPtr); + Pointer virDomainGetOSType(DomainPointer virDomainPtr); int virDomainGetSchedulerParameters(DomainPointer virDomainPtr, virSchedParameter[] params, IntByReference nparams); Pointer virDomainGetSchedulerType(DomainPointer virDomainPtr, IntByReference nparams); int virDomainGetUUID(DomainPointer virDomainPtr, byte[] uuidString); int virDomainGetUUIDString(DomainPointer virDomainPtr, byte[] uuidString); int virDomainGetVcpus(DomainPointer virDomainPtr, virVcpuInfo[] info, int maxInfo, byte[] cpumaps, int maplen); - String virDomainGetXMLDesc(DomainPointer virDomainPtr, int flags); + Pointer virDomainGetXMLDesc(DomainPointer virDomainPtr, int flags); int virDomainHasCurrentSnapshot(DomainPointer virDomainPtr, int flags); int virDomainHasManagedSaveImage(DomainPointer virDomainPtr, int flags); int virDomainInterfaceStats(DomainPointer virDomainPtr, String path, virDomainInterfaceStats stats, int size); @@ -252,11 +249,11 @@ public interface Libvirt extends Library { int virNetworkDestroy(NetworkPointer virConnectPtr); int virNetworkFree(NetworkPointer virConnectPtr); int virNetworkGetAutostart(NetworkPointer virNetworkPtr, IntByReference value); - String virNetworkGetBridgeName(NetworkPointer virNetworkPtr); + Pointer virNetworkGetBridgeName(NetworkPointer virNetworkPtr); String virNetworkGetName(NetworkPointer virNetworkPtr); int virNetworkGetUUID(NetworkPointer virNetworkPtr, byte[] uuidString); int virNetworkGetUUIDString(NetworkPointer virNetworkPtr, byte[] uuidString); - String virNetworkGetXMLDesc(NetworkPointer virNetworkPtr, int flags); + Pointer virNetworkGetXMLDesc(NetworkPointer virNetworkPtr, int flags); int virNetworkIsActive(NetworkPointer virNetworkPtr); int virNetworkIsPersistent(NetworkPointer virNetworkPtr); NetworkPointer virNetworkLookupByName(ConnectionPointer virConnectPtr, String name); @@ -273,7 +270,7 @@ public interface Libvirt extends Library { // Node/Device functions int virNodeNumOfDevices(ConnectionPointer virConnectPtr, String capabilityName, int flags); - int virNodeListDevices(ConnectionPointer virConnectPtr, String capabilityName, String[] names, int maxnames, + int virNodeListDevices(ConnectionPointer virConnectPtr, String capabilityName, Pointer[] names, int maxnames, int flags); DevicePointer virNodeDeviceLookupByName(ConnectionPointer virConnectPtr, String name); String virNodeDeviceGetName(DevicePointer virDevicePointer); @@ -337,7 +334,7 @@ public interface Libvirt extends Library { int virInterfaceFree(InterfacePointer virDevicePointer); String virInterfaceGetName(InterfacePointer virInterfacePtr); String virInterfaceGetMACString(InterfacePointer virInterfacePtr); - String virInterfaceGetXMLDesc(InterfacePointer virInterfacePtr, int flags); + Pointer virInterfaceGetXMLDesc(InterfacePointer virInterfacePtr, int flags); int virInterfaceIsActive(InterfacePointer virDevicePointer); InterfacePointer virInterfaceLookupByMACString(ConnectionPointer virConnectPtr, String mac); InterfacePointer virInterfaceLookupByName(ConnectionPointer virConnectPtr, String name); -- 1.7.9.5 -- AV-Test GmbH, Henricistraße 20, 04155 Leipzig, Germany Phone: +49 341 265 310 19 Web:<http://www.av-test.org> Eingetragen am / Registered at: Amtsgericht Stendal (HRB 114076) Geschaeftsfuehrer (CEO): Andreas Marx, Guido Habicht, Maik Morgenstern