Skip to content

Commit 071bf67

Browse files
Martin Belangerigaw
authored andcommitted
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 <[email protected]>
1 parent fb4558a commit 071bf67

2 files changed

Lines changed: 11 additions & 0 deletions

File tree

libnvme/nvme.i

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -417,6 +417,9 @@ struct nvme_ns {
417417
}
418418
}
419419

420+
%pythonappend nvme_host::nvme_host(struct nvme_root *r, const char *hostnqn,
421+
const char *hostid, const char *hostsymname) {
422+
self.__parent = r # Keep a reference to parent to ensure garbage collection happens in the right order}
420423
%extend nvme_host {
421424
nvme_host(struct nvme_root *r, const char *hostnqn = NULL,
422425
const char *hostid = NULL, const char *hostsymname = NULL) {
@@ -501,6 +504,8 @@ struct nvme_ns {
501504
}
502505
}
503506

507+
%pythonappend nvme_subsystem::nvme_subsystem(struct nvme_host *host, const char *subsysnqn, const char *name) {
508+
self.__parent = host # Keep a reference to parent to ensure garbage collection happens in the right order}
504509
%extend nvme_subsystem {
505510
nvme_subsystem(struct nvme_host *host, const char *subsysnqn,
506511
const char *name = NULL) {
@@ -557,6 +562,8 @@ struct nvme_ns {
557562
}
558563
}
559564

565+
%pythonappend nvme_ctrl::connect(struct nvme_host *h, struct nvme_fabrics_config *cfg) {
566+
self.__parent = h # Keep a reference to parent to ensure garbage collection happens in the right order}
560567
%extend nvme_ctrl {
561568
nvme_ctrl(struct nvme_root *r, const char *subsysnqn, const char *transport,
562569
const char *traddr = NULL, const char *host_traddr = NULL,
@@ -720,6 +727,8 @@ struct nvme_ns {
720727
}
721728
%};
722729

730+
%pythonappend nvme_ns::nvme_ns(struct nvme_subsystem *s, unsigned int nsid) {
731+
self.__parent = s # Keep a reference to parent to ensure garbage collection happens in the right order}
723732
%extend nvme_ns {
724733
nvme_ns(struct nvme_subsystem *s, unsigned int nsid) {
725734
return nvme_subsystem_lookup_namespace(s, nsid);

src/nvme/tree.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -363,6 +363,7 @@ static void __nvme_free_ns(struct nvme_ns *n)
363363
/* Stub for SWIG */
364364
void nvme_free_ns(struct nvme_ns *n)
365365
{
366+
__nvme_free_ns(n);
366367
}
367368

368369
static void __nvme_free_subsystem(struct nvme_subsystem *s)
@@ -459,6 +460,7 @@ static void __nvme_free_host(struct nvme_host *h)
459460
/* Stub for SWIG */
460461
void nvme_free_host(struct nvme_host *h)
461462
{
463+
__nvme_free_host(h);
462464
}
463465

464466
struct nvme_host *nvme_lookup_host(nvme_root_t r, const char *hostnqn,

0 commit comments

Comments
 (0)