[libvirt] [PATCH Java] Cleanup close/free handling

The Connect class itself can have one ref to the internal virConnectPtr. Therefore, it has to release the VCP in its close method always. Otherwise multiple calls may release more then one ref, resulting in a ref-underflow later on. Make StoragePool's and StorageVol's free return int, like all other free methods, to return the number of remaining refs. Update Javadoc for all free methods to indicate that the return value is the number of remaining refs and that a value >= is returned on success. --- src/main/java/org/libvirt/Connect.java | 12 ++++-------- src/main/java/org/libvirt/Device.java | 2 +- src/main/java/org/libvirt/Domain.java | 2 +- src/main/java/org/libvirt/Interface.java | 2 +- src/main/java/org/libvirt/Network.java | 1 + src/main/java/org/libvirt/StoragePool.java | 14 +++++++++++--- src/main/java/org/libvirt/StorageVol.java | 12 +++++++++--- 7 files changed, 28 insertions(+), 17 deletions(-) diff --git a/src/main/java/org/libvirt/Connect.java b/src/main/java/org/libvirt/Connect.java index 3c6663e..8cec22e 100644 --- a/src/main/java/org/libvirt/Connect.java +++ b/src/main/java/org/libvirt/Connect.java @@ -219,7 +219,7 @@ public class Connect { * the object after close() will result in an exception. * * @throws LibvirtException - * @returns 0 for success, -1 for failure + * @return number of references left (>= 0) for success, -1 for failure. */ public int close() throws LibvirtException { int success = 0; @@ -227,14 +227,10 @@ public class Connect { success = libvirt.virConnectClose(VCP); processError(); // If leave an invalid pointer dangling around JVM crashes and burns - // if - // someone tries to call a method on us + // if someone tries to call a method on us // We rely on the underlying libvirt error handling to detect that - // it's - // called with a null virConnectPointer - if (success == 0) { - VCP = null; - } + // it's called with a null virConnectPointer + VCP = null; } return success; } diff --git a/src/main/java/org/libvirt/Device.java b/src/main/java/org/libvirt/Device.java index 26d8cbf..b4f9a0c 100644 --- a/src/main/java/org/libvirt/Device.java +++ b/src/main/java/org/libvirt/Device.java @@ -137,7 +137,7 @@ public class Device { * structure is freed and should not be used thereafter. * * @throws LibvirtException - * @returns 0 for success, -1 for failure. + * @return number of references left (>= 0) for success, -1 for failure. */ public int free() throws LibvirtException { int success = 0; diff --git a/src/main/java/org/libvirt/Domain.java b/src/main/java/org/libvirt/Domain.java index 8df928f..e7b24ef 100644 --- a/src/main/java/org/libvirt/Domain.java +++ b/src/main/java/org/libvirt/Domain.java @@ -167,7 +167,7 @@ public class Domain { * structure is freed and should not be used thereafter. * * @throws LibvirtException - * @returns 0 for success, -1 for failure. + * @return number of references left (>= 0) for success, -1 for failure. */ public int free() throws LibvirtException { int success = 0; diff --git a/src/main/java/org/libvirt/Interface.java b/src/main/java/org/libvirt/Interface.java index 0955cf9..44bcad5 100644 --- a/src/main/java/org/libvirt/Interface.java +++ b/src/main/java/org/libvirt/Interface.java @@ -117,7 +117,7 @@ public class Interface { * structure is freed and should not be used thereafter. * * @throws LibvirtException - * @returns 0 for success, -1 for failure. + * @return number of references left (>= 0) for success, -1 for failure. */ public int free() throws LibvirtException { int success = 0; diff --git a/src/main/java/org/libvirt/Network.java b/src/main/java/org/libvirt/Network.java index 14193fa..3c6f313 100644 --- a/src/main/java/org/libvirt/Network.java +++ b/src/main/java/org/libvirt/Network.java @@ -74,6 +74,7 @@ public class Network { * return an error. * * @throws LibvirtException + * @return number of references left (>= 0) for success, -1 for failure. */ public int free() throws LibvirtException { int success = 0; diff --git a/src/main/java/org/libvirt/StoragePool.java b/src/main/java/org/libvirt/StoragePool.java index 3256601..7de7d50 100644 --- a/src/main/java/org/libvirt/StoragePool.java +++ b/src/main/java/org/libvirt/StoragePool.java @@ -118,10 +118,18 @@ public class StoragePool { /** * Free a storage pool object, releasing all memory associated with it. Does * not change the state of the pool on the host. + * + * @throws LibvirtException + * @return number of references left (>= 0) for success, -1 for failure. */ - public void free() throws LibvirtException { - libvirt.virStoragePoolFree(VSPP); - processError(); + public int free() throws LibvirtException { + int success = 0; + if (VSPP != null) { + success = libvirt.virStoragePoolFree(VSPP); + processError(); + VSPP = null; + } + return success; } /** diff --git a/src/main/java/org/libvirt/StorageVol.java b/src/main/java/org/libvirt/StorageVol.java index 9213813..5ae5f5c 100644 --- a/src/main/java/org/libvirt/StorageVol.java +++ b/src/main/java/org/libvirt/StorageVol.java @@ -80,10 +80,16 @@ public class StorageVol { * to exist * * @throws LibvirtException + * @return number of references left (>= 0) for success, -1 for failure. */ - public void free() throws LibvirtException { - libvirt.virStorageVolFree(VSVP); - processError(); + public int free() throws LibvirtException { + int success = 0; + if (VSVP != null) { + libvirt.virStorageVolFree(VSVP); + processError(); + VSVP = null; + } + return success; } /** -- 1.6.3.3