[rdma-core,1/3] pyverbs: Move close_weakrefs() out of the base object

Commit Message

Noa Osherovich Dec. 18, 2019, 1:02 p.m. UTC

close_weakrefs() is called during __dealloc__(). During that time, the
object might be partially destroyed. Since close_weakrefs() doesn't use
'self', there's no prevention to move it outside of the object.

Signed-off-by: Noa Osherovich <[e-mail]>
Reviewed-by: Edward Srouji <[e-mail]>
---
 pyverbs/base.pxd   |  2 ++
 pyverbs/base.pyx   | 46 ++++++++++++++++++++++++----------------------
 pyverbs/cq.pyx     |  7 ++++---
 pyverbs/device.pyx |  7 ++++---
 pyverbs/pd.pyx     |  5 +++--
 pyverbs/xrcd.pyx   |  3 ++-
 6 files changed, 39 insertions(+), 31 deletions(-)

Patch

diff --git a/pyverbs/base.pxd b/pyverbs/base.pxd
index e85f7c020e1c..efa63236810c 100644
--- a/pyverbs/base.pxd
+++ b/pyverbs/base.pxd
@@ -9,3 +9,5 @@  cdef class PyverbsObject(object):
 
 cdef class PyverbsCM(PyverbsObject):
     cpdef close(self)
+
+cdef close_weakrefs(iterables)
diff --git a/pyverbs/base.pyx b/pyverbs/base.pyx
index 8b3e6741ae19..d69516285ece 100644
--- a/pyverbs/base.pyx
+++ b/pyverbs/base.pyx
@@ -12,6 +12,30 @@  LOG_LEVEL=logging.INFO
 LOG_FORMAT='[%(levelname)s] %(asctime)s %(filename)s:%(lineno)s: %(message)s'
 logging.basicConfig(format=LOG_FORMAT, level=LOG_LEVEL, datefmt='%d %b %Y %H:%M:%S')
 
+
+cdef close_weakrefs(iterables):
+    """
+    For each iterable element of iterables, pop each element and
+    call its close() method. This method is used when an object is being
+    closed while other objects still hold C references to it; the object
+    holds weakrefs to such other object, and closes them before trying to
+    teardown the C resources.
+    :param iterables: an array of WeakSets
+    :return: None
+    """
+    # None elements can be present if an object's close() was called more
+    # than once (e.g. GC and by another object)
+    for it in iterables:
+        if it is None:
+            continue
+        while True:
+            try:
+                tmp = it.pop()
+                tmp.close()
+            except KeyError: # popping an empty set
+                break
+
+
 cdef class PyverbsObject(object):
 
     def __cinit__(self):
@@ -20,28 +44,6 @@  cdef class PyverbsObject(object):
     def set_log_level(self, val):
         self.logger.setLevel(val)
 
-    def close_weakrefs(self, iterables):
-        """
-        For each iterable element of iterables, pop each element and
-        call its close() method. This method is used when an object is being
-        closed while other objects still hold C references to it; the object
-        holds weakrefs to such other object, and closes them before trying to
-        teardown the C resources.
-        :param iterables: an array of WeakSets
-        :return: None
-        """
-        # None elements can be present if an object's close() was called more
-        # than once (e.g. GC and by another object)
-        for it in iterables:
-            if it is None:
-                continue
-            while True:
-                try:
-                    tmp = it.pop()
-                    tmp.close()
-                except KeyError: # popping an empty set
-                    break
-
 
 cdef class PyverbsCM(PyverbsObject):
     """
diff --git a/pyverbs/cq.pyx b/pyverbs/cq.pyx
index ac6baa618c3d..1ea443fc6966 100755
--- a/pyverbs/cq.pyx
+++ b/pyverbs/cq.pyx
@@ -4,6 +4,7 @@  import weakref
 
 from pyverbs.pyverbs_error import PyverbsError
 from pyverbs.base import PyverbsRDMAErrno
+from pyverbs.base cimport close_weakrefs
 cimport pyverbs.libibverbs_enums as e
 from pyverbs.device cimport Context
 from pyverbs.srq cimport SRQ
@@ -35,7 +36,7 @@  cdef class CompChannel(PyverbsCM):
 
     cpdef close(self):
         self.logger.debug('Closing completion channel')
-        self.close_weakrefs([self.cqs])
+        close_weakrefs([self.cqs])
         if self.cc != NULL:
             rc = v.ibv_destroy_comp_channel(self.cc)
             if rc != 0:
@@ -108,7 +109,7 @@  cdef class CQ(PyverbsCM):
 
     cpdef close(self):
         self.logger.debug('Closing CQ')
-        self.close_weakrefs([self.qps, self.srqs])
+        close_weakrefs([self.qps, self.srqs])
         if self.cq != NULL:
             rc = v.ibv_destroy_cq(self.cq)
             if rc != 0:
@@ -292,7 +293,7 @@  cdef class CQEX(PyverbsCM):
 
     cpdef close(self):
         self.logger.debug('Closing CQEx')
-        self.close_weakrefs([self.srqs, self.qps])
+        close_weakrefs([self.srqs, self.qps])
         if self.cq != NULL:
             rc = v.ibv_destroy_cq(<v.ibv_cq*>self.cq)
             if rc != 0:
diff --git a/pyverbs/device.pyx b/pyverbs/device.pyx
index 33d133fd24da..56d7540ceb6c 100755
--- a/pyverbs/device.pyx
+++ b/pyverbs/device.pyx
@@ -12,6 +12,7 @@  from .pyverbs_error import PyverbsRDMAError, PyverbsError
 from pyverbs.cq cimport CQEX, CQ, CompChannel
 from .pyverbs_error import PyverbsUserError
 from pyverbs.base import PyverbsRDMAErrno
+from pyverbs.base cimport close_weakrefs
 cimport pyverbs.libibverbs_enums as e
 cimport pyverbs.libibverbs as v
 from pyverbs.cmid cimport CMID
@@ -144,8 +145,8 @@  cdef class Context(PyverbsCM):
 
     cpdef close(self):
         self.logger.debug('Closing Context')
-        self.close_weakrefs([self.qps, self.ccs, self.cqs, self.dms, self.pds,
-                             self.xrcds])
+        close_weakrefs([self.qps, self.ccs, self.cqs, self.dms, self.pds,
+                        self.xrcds])
         if self.context != NULL:
             rc = v.ibv_close_device(self.context)
             if rc != 0:
@@ -647,7 +648,7 @@  cdef class DM(PyverbsCM):
 
     cpdef close(self):
         self.logger.debug('Closing DM')
-        self.close_weakrefs([self.dm_mrs])
+        close_weakrefs([self.dm_mrs])
         if self.dm != NULL:
             rc = v.ibv_free_dm(self.dm)
             if rc != 0:
diff --git a/pyverbs/pd.pyx b/pyverbs/pd.pyx
index 4a33587b7df0..8c17ffcba7e8 100755
--- a/pyverbs/pd.pyx
+++ b/pyverbs/pd.pyx
@@ -4,6 +4,7 @@  import weakref
 
 from pyverbs.pyverbs_error import PyverbsUserError, PyverbsError
 from pyverbs.base import PyverbsRDMAErrno
+from pyverbs.base cimport close_weakrefs
 from pyverbs.device cimport Context
 from libc.stdint cimport uintptr_t
 from pyverbs.cmid cimport CMID
@@ -65,8 +66,8 @@  cdef class PD(PyverbsCM):
         :return: None
         """
         self.logger.debug('Closing PD')
-        self.close_weakrefs([self.parent_domains, self.qps, self.ahs, self.mws,
-                             self.mrs, self.srqs])
+        close_weakrefs([self.parent_domains, self.qps, self.ahs, self.mws,
+                        self.mrs, self.srqs])
         if self.pd != NULL:
             rc = v.ibv_dealloc_pd(self.pd)
             if rc != 0:
diff --git a/pyverbs/xrcd.pyx b/pyverbs/xrcd.pyx
index 70b0c9adbbaa..91303daf3bc0 100755
--- a/pyverbs/xrcd.pyx
+++ b/pyverbs/xrcd.pyx
@@ -4,6 +4,7 @@  import weakref
 
 from pyverbs.pyverbs_error import PyverbsRDMAError, PyverbsError
 from pyverbs.base import PyverbsRDMAErrno
+from pyverbs.base cimport close_weakrefs
 from pyverbs.device cimport Context
 from pyverbs.srq cimport SRQ
 from pyverbs.qp cimport QP
@@ -68,7 +69,7 @@  cdef class XRCD(PyverbsCM):
         :return: None
         """
         self.logger.debug('Closing XRCD')
-        self.close_weakrefs([self.qps, self.srqs])
+        close_weakrefs([self.qps, self.srqs])
         # XRCD may be deleted directly or indirectly by closing its context,
         # which leaves the Python XRCD object without the underlying C object,
         # so during destruction, need to check whether or not the C object