At Tue, 15 Dec 2015 14:30:53 +0900,
Takashi Yamamoto wrote:
> 
> On Mon, Dec 14, 2015 at 5:57 PM, IWAMOTO Toshihiro
> <[email protected]> wrote:
> > At Mon, 14 Dec 2015 17:38:40 +0900,
> > Takashi Yamamoto wrote:
> >>
> >> On Mon, Dec 14, 2015 at 5:31 PM, IWAMOTO Toshihiro
> >> <[email protected]> wrote:
> >> > At Mon, 14 Dec 2015 17:19:28 +0900,
> >> > Takashi Yamamoto wrote:
> >> >>
> >> >> On Mon, Dec 14, 2015 at 5:00 PM, IWAMOTO Toshihiro
> >> >> <[email protected]> wrote:
> >> >> > Otherwise, AppManager.run_apps fails to terminate.
> >> >>
> >> >> run_apps tries to kill threads in "services" by itself.
> >> >> can you explain why it isn't enough?
> >> >
> >> >
> >> > from ryu/base/app_manager.py:
> >> >
> >> > : class AppManager(object):
> >> > :     # singletone
> >> > :     _instance = None
> >> > :
> >> > :     @staticmethod
> >> > :     def run_apps(app_lists):
> >> > :         """Run a set of Ryu applications
> >> > :
> >> > :         A convenient method to load and instantiate apps.
> >> > :         This blocks until all relevant apps stop.
> >> > :         """
> >> > :         app_mgr = AppManager.get_instance()
> >> > :         app_mgr.load_apps(app_lists)
> >> > :         contexts = app_mgr.create_contexts()
> >> > :         services = app_mgr.instantiate_apps(**contexts)
> >> > :         webapp = wsgi.start_service(app_mgr)
> >> > :         if webapp:
> >> > :             services.append(hub.spawn(webapp))
> >> > :         try:
> >> > :             hub.joinall(services)
> >> >
> >> > When AppManager.close() is called, the process gets stuck here.
> >>
> >> the close method is called in the following finally block.
> >> i guess you are looking at a wrong place.
> >
> > Please look at:
> > https://review.openstack.org/#/c/257204/
> 
> ok, i think i understand what you want to do.
> can you consider to manage the extra threads consistently at RyuApp level
> rather than making each subclasses maintain its own members
> like "vrrp_thread"?

Good idea.
I'll respin a patch.

> >> > :         finally:
> >> > :             app_mgr.close()
> >> > :             for t in services:
> >> > :                 t.kill()
> >> >
> >> > Is this where you meant by "run_app tries to kill thread"?
> >>
> >> yes.
> >>
> >> > I think an RyuApp can throw an exception to reach here, but it doesn't
> >> > seem to be useful for clean shutdown.
> >>
> >> AppManager.close() is also called in the same finally block.
> >> i failed to understand why your changes in stop() can be useful
> >> while these kill() are not useful.
> >
> >> >
> >> >> > Signed-off-by: IWAMOTO Toshihiro <[email protected]>
> >> >> > ---
> >> >> >  ryu/controller/ofp_handler.py             |  9 ++++++++-
> >> >> >  ryu/services/protocols/bgp/application.py | 15 +++++++++++----
> >> >> >  ryu/services/protocols/ovsdb/manager.py   |  7 +++++--
> >> >> >  ryu/services/protocols/vrrp/manager.py    | 10 ++++++++--
> >> >> >  4 files changed, 32 insertions(+), 9 deletions(-)
> >> >> >
> >> >> > diff --git a/ryu/controller/ofp_handler.py 
> >> >> > b/ryu/controller/ofp_handler.py
> >> >> > index cc64b1f..6b0a9f5 100644
> >> >> > --- a/ryu/controller/ofp_handler.py
> >> >> > +++ b/ryu/controller/ofp_handler.py
> >> >> > @@ -51,11 +51,18 @@ from ryu.ofproto import ofproto_parser
> >> >> >  class OFPHandler(ryu.base.app_manager.RyuApp):
> >> >> >      def __init__(self, *args, **kwargs):
> >> >> >          super(OFPHandler, self).__init__(*args, **kwargs)
> >> >> > +        self.ofp_thread = None
> >> >> >          self.name = 'ofp_event'
> >> >> >
> >> >> >      def start(self):
> >> >> >          super(OFPHandler, self).start()
> >> >> > -        return hub.spawn(OpenFlowController())
> >> >> > +        self.ofp_thread = hub.spawn(OpenFlowController())
> >> >> > +        return self.ofp_thread
> >> >> > +
> >> >> > +    def stop(self):
> >> >> > +        if self.ofp_thread:
> >> >> > +            hub.kill(self.ofp_thread)
> >> >> > +        super(OFPHandler, self).stop()
> >> >> >
> >> >> >      def _hello_failed(self, datapath, error_desc):
> >> >> >          self.logger.error(error_desc)
> >> >> > diff --git a/ryu/services/protocols/bgp/application.py 
> >> >> > b/ryu/services/protocols/bgp/application.py
> >> >> > index 8284444..7a77c32 100644
> >> >> > --- a/ryu/services/protocols/bgp/application.py
> >> >> > +++ b/ryu/services/protocols/bgp/application.py
> >> >> > @@ -78,6 +78,7 @@ class RyuBGPSpeaker(RyuApp):
> >> >> >          self.bind_ip = RyuBGPSpeaker.validate_rpc_ip(CONF.bind_ip)
> >> >> >          self.bind_port = 
> >> >> > RyuBGPSpeaker.validate_rpc_port(CONF.bind_port)
> >> >> >          self.config_file = CONF.bgp_config_file
> >> >> > +        self.bgp_thread = None
> >> >> >          super(RyuBGPSpeaker, self).__init__(*args, **kwargs)
> >> >> >
> >> >> >      def start(self):
> >> >> > @@ -101,14 +102,20 @@ class RyuBGPSpeaker(RyuApp):
> >> >> >              if getattr(settings, 'SSH', None) is not None:
> >> >> >                  hub.spawn(ssh.SSH_CLI_CONTROLLER.start, None, 
> >> >> > **settings.SSH)
> >> >> >          # Start Network Controller to server RPC peers.
> >> >> > -        t = hub.spawn(net_ctrl.NET_CONTROLLER.start, *[],
> >> >> > -                      **{net_ctrl.NC_RPC_BIND_IP: self.bind_ip,
> >> >> > -                      net_ctrl.NC_RPC_BIND_PORT: self.bind_port})
> >> >> > +        self.bgp_thread = hub.spawn(
> >> >> > +            net_ctrl.NET_CONTROLLER.start, *[],
> >> >> > +            **{net_ctrl.NC_RPC_BIND_IP: self.bind_ip,
> >> >> > +               net_ctrl.NC_RPC_BIND_PORT: self.bind_port})
> >> >> >          LOG.debug('Started Network Controller')
> >> >> >
> >> >> >          super(RyuBGPSpeaker, self).start()
> >> >> >
> >> >> > -        return t
> >> >> > +        return self.bgp_thread
> >> >> > +
> >> >> > +    def stop(self):
> >> >> > +        if self.bgp_thread:
> >> >> > +            hub.kill(self.bgp_thread)
> >> >> > +        super(RyuBGPSpeaker, self).stop()
> >> >> >
> >> >> >      @classmethod
> >> >> >      def validate_rpc_ip(cls, ip):
> >> >> > diff --git a/ryu/services/protocols/ovsdb/manager.py 
> >> >> > b/ryu/services/protocols/ovsdb/manager.py
> >> >> > index 7b1d5c8..0933f97 100644
> >> >> > --- a/ryu/services/protocols/ovsdb/manager.py
> >> >> > +++ b/ryu/services/protocols/ovsdb/manager.py
> >> >> > @@ -44,6 +44,7 @@ class OVSDB(app_manager.RyuApp):
> >> >> >          self._address = self.CONF.ovsdb.address
> >> >> >          self._port = self.CONF.ovsdb.port
> >> >> >          self._clients = {}
> >> >> > +        self._accept_thread = None
> >> >> >
> >> >> >      def _accept(self, server):
> >> >> >          if self.CONF.ovsdb.whitelist:
> >> >> > @@ -113,11 +114,13 @@ class OVSDB(app_manager.RyuApp):
> >> >> >
> >> >> >          self.logger.info('Listening on %s:%s for clients' % 
> >> >> > (self._address,
> >> >> >                                                               
> >> >> > self._port))
> >> >> > -        t = hub.spawn(self._accept, self._server)
> >> >> > +        self._accept_thread = hub.spawn(self._accept, self._server)
> >> >> >          super(OVSDB, self).start()
> >> >> > -        return t
> >> >> > +        return self._accept_thread
> >> >> >
> >> >> >      def stop(self):
> >> >> > +        if self._accept_thread:
> >> >> > +            hub.kill(self._accept_thread)
> >> >> >          for client in self._clients.values():
> >> >> >              client.stop()
> >> >> >
> >> >> > diff --git a/ryu/services/protocols/vrrp/manager.py 
> >> >> > b/ryu/services/protocols/vrrp/manager.py
> >> >> > index 40c730d..f9bdce4 100644
> >> >> > --- a/ryu/services/protocols/vrrp/manager.py
> >> >> > +++ b/ryu/services/protocols/vrrp/manager.py
> >> >> > @@ -61,11 +61,17 @@ class VRRPManager(app_manager.RyuApp):
> >> >> >          self.name = vrrp_event.VRRP_MANAGER_NAME
> >> >> >          self._instances = {}    # name -> VRRPInstance
> >> >> >          self.shutdown = hub.Queue()
> >> >> > +        self.vrrp_thread = None
> >> >> >
> >> >> >      def start(self):
> >> >> > -        t = hub.spawn(self._shutdown_loop)
> >> >> > +        self.vrrp_thread = hub.spawn(self._shutdown_loop)
> >> >> >          super(VRRPManager, self).start()
> >> >> > -        return t
> >> >> > +        return self.vrrp_thread
> >> >> > +
> >> >> > +    def stop(self):
> >> >> > +        if self.vrrp_thread:
> >> >> > +            hub.kill(self.vrrp_thread)
> >> >> > +        super(VRRPManager, self).stop()
> >> >> >
> >> >> >      @handler.set_ev_cls(vrrp_event.EventVRRPConfigRequest)
> >> >> >      def config_request_handler(self, ev):
> >> >> > --
> >> >> > 2.1.4
> >> >> >
> >> >> >
> >> >> > ------------------------------------------------------------------------------
> >> >> > _______________________________________________
> >> >> > Ryu-devel mailing list
> >> >> > [email protected]
> >> >> > https://lists.sourceforge.net/lists/listinfo/ryu-devel
> >> >>
> >>
> 

------------------------------------------------------------------------------
_______________________________________________
Ryu-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/ryu-devel

Reply via email to