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/ > > : 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
