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