python: Fix segmentation fault during garbage collection

With SWIG we create proxy classes to wrap the C structures (struct).
These classes are used to create Python objects matching the C
structures such as:

`nvme.root` is used to wrap `struct nvme_root`
`nvme.host` is used to wrap `struct nvme_host`
`nvme.ctrl` is used to wrap `struct nvme_ctrl`
etc...

One thing that SWIG cannot do is figure out the dependencies
between the different objects. For example, it cannot tell that
when deleting a host object that all the subsystems, controllers,
namespaces under that host need to be deleted as well. That's an
implicit property of the libnvme driver and users need to know to
stop using objects after their parent has been deleted.

Unfotunately, with Python the Gargage Collector (GC) decides which
objects to delete and it can delete obects in any order it sees fit.
This can result in objects being deleted in the wrong order. For
example, the GC may delete a host object (`nvme_free_host`) before
deleting any of the controller objects under that host. And when the
GC finally deletes a controller by calling `nvme_free_ctrl()`, the
memory for that controller has already been freed and a segmentation
fault will result.

To enforce that objects get deleted in the right order, we need to
set dependencies between objects at the Python level. This can be
achived by having children objects maintain a reference to their
parents. This way a parent cannot be deleted before all its children
have been deleted.

Signed-off-by: Martin Belanger <martin.belanger@dell.com>

diff --git a/libnvme/nvme.i b/libnvme/nvme.i
index 0c6b73ca46a962f73821dc2071a18fee8623d23a..f0aa94b1086cf9b0936ca0240ff1f3aa40e92ece 100644 (file)
--- a/libnvme/nvme.i
+++ b/libnvme/nvme.i
@@ -417,6 +417,9 @@ struct nvme_ns {
   }
 }
 
+%pythonappend nvme_host::nvme_host(struct nvme_root *r, const char *hostnqn,
+                                  const char *hostid, const char *hostsymname) {
+    self.__parent = r  # Keep a reference to parent to ensure garbage collection happens in the right order}
 %extend nvme_host {
   nvme_host(struct nvme_root *r, const char *hostnqn = NULL,
            const char *hostid = NULL, const char *hostsymname = NULL) {
@@ -501,6 +504,8 @@ struct nvme_ns {
   }
 }
 
+%pythonappend nvme_subsystem::nvme_subsystem(struct nvme_host *host, const char *subsysnqn, const char *name) {
+    self.__parent = host  # Keep a reference to parent to ensure garbage collection happens in the right order}
 %extend nvme_subsystem {
   nvme_subsystem(struct nvme_host *host, const char *subsysnqn,
                 const char *name = NULL) {
@@ -557,6 +562,8 @@ struct nvme_ns {
   }
 }
 
+%pythonappend nvme_ctrl::connect(struct nvme_host *h, struct nvme_fabrics_config *cfg) {
+    self.__parent = h  # Keep a reference to parent to ensure garbage collection happens in the right order}
 %extend nvme_ctrl {
   nvme_ctrl(struct nvme_root *r, const char *subsysnqn, const char *transport,
            const char *traddr = NULL, const char *host_traddr = NULL,
@@ -720,6 +727,8 @@ struct nvme_ns {
   }
 %};
 
+%pythonappend nvme_ns::nvme_ns(struct nvme_subsystem *s, unsigned int nsid) {
+    self.__parent = s  # Keep a reference to parent to ensure garbage collection happens in the right order}
 %extend nvme_ns {
   nvme_ns(struct nvme_subsystem *s, unsigned int nsid) {
     return nvme_subsystem_lookup_namespace(s, nsid);

diff --git a/src/nvme/tree.c b/src/nvme/tree.c
index 18826bfedb38be29e09dfafba2a108b8296ceea1..83e35c488ce243b33ae7832d1860bccb3ceb7806 100644 (file)
--- a/src/nvme/tree.c
+++ b/src/nvme/tree.c
@@ -363,6 +363,7 @@ static void __nvme_free_ns(struct nvme_ns *n)
 /* Stub for SWIG */
 void nvme_free_ns(struct nvme_ns *n)
 {
+       __nvme_free_ns(n);
 }
 
 static void __nvme_free_subsystem(struct nvme_subsystem *s)
@@ -459,6 +460,7 @@ static void __nvme_free_host(struct nvme_host *h)
 /* Stub for SWIG */
 void nvme_free_host(struct nvme_host *h)
 {
+       __nvme_free_host(h);
 }
 
 struct nvme_host *nvme_lookup_host(nvme_root_t r, const char *hostnqn,