controller: eliminate weakref of datapath.ev_q.aux

Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>

3 files changed, 26 insertions, 11 deletions

diff --git a/ryu/controller/controller.py b/ryu/controller/controller.py
index 45cffe3f..46179a12 100644
--- a/ryu/controller/controller.py
+++ b/ryu/controller/controller.py
@@ -13,11 +13,11 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
+import contextlib
 import gflags
 import logging
 import gevent
 import random
-import weakref
 from gevent.server import StreamServer
 from gevent.queue import Queue
 
@@ -85,17 +85,25 @@ class Datapath(object):
         self.recv_q = Queue()
         self.send_q = Queue()
 
-        # weakref: qv_q.aux refers to aux = self
-        # self.ev_q.aux == weakref.ref(self)
+        # circular reference self.ev_q.aux == self
         self.ev_q = dispatcher.EventQueue(handler.QUEUE_NAME_OFP_MSG,
                                           handler.HANDSHAKE_DISPATCHER,
-                                          weakref.ref(self))
+                                          self)
 
         self.set_version(max(self.supported_ofp_version))
         self.xid = random.randint(0, self.ofproto.MAX_XID)
         self.id = None  # datapath_id is unknown yet
         self.ports = None
 
+    def close(self):
+        """
+        Call this before discarding this datapath object
+        The circular refernce as self.ev_q.aux == self must be broken.
+        """
+        # tell this datapath is dead
+        self.ev_q.set_dispatcher(handler.DEAD_DISPATCHER)
+        self.ev_q.close()
+
     def set_version(self, version):
         assert version in self.supported_ofp_version
         self.ofproto, self.ofproto_parser = self.supported_ofp_version[version]
@@ -218,10 +226,5 @@ class Datapath(object):
 
 def datapath_connection_factory(socket, address):
     LOG.debug('connected socket:%s address:%s', socket, address)
-
-    datapath = Datapath(socket, address)
-    try:
+    with contextlib.closing(Datapath(socket, address)) as datapath:
         datapath.serve()
-    finally:
-        # tell this datapath is dead
-        datapath.ev_q.set_dispatcher(handler.DEAD_DISPATCHER)
diff --git a/ryu/controller/dispatcher.py b/ryu/controller/dispatcher.py
index b42d2877..32197fe3 100644
--- a/ryu/controller/dispatcher.py
+++ b/ryu/controller/dispatcher.py
@@ -63,10 +63,22 @@ class EventQueue(TrackInstances):
         # This can be called when python interpreter exiting.
         # At that time, other object like EventQueueCreate can be
         # already destructed. So we can't call it blindly.
+        assert self.aux == None
         ev_q = self._get_ev_q()
         if ev_q is not None and self != ev_q:
             self._queue_q_ev(EventQueueCreate(self, False))
 
+    def close(self):
+        """
+        Call this function before discarding this object.
+        This function unset self.aux in order to break potential circular
+        reference.
+
+        Sometimes self.aux results in cyclic reference.
+        So we need to break it explicitly. (Or use weakref)
+        """
+        self.aux = None
+
     def set_dispatcher(self, dispatcher):
         old = self.dispatcher
         new = dispatcher.clone()
diff --git a/ryu/controller/dpset.py b/ryu/controller/dpset.py
index 1156caa1..fe42b95a 100644
--- a/ryu/controller/dpset.py
+++ b/ryu/controller/dpset.py
@@ -93,7 +93,7 @@ class DPSet(object):
         if ev.ev_q.name != handler.QUEUE_NAME_OFP_MSG:
             return
 
-        datapath = ev.ev_q.aux()
+        datapath = ev.ev_q.aux
         assert datapath is not None
         if ev.new_dispatcher.name == handler.DISPATCHER_NAME_OFP_MAIN:
             LOG.debug('DPSET: register datapath %s', datapath)