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"? > > >> > : 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
