From d8ef6d59c152f7630520fbc855b9435a1aa52f3c Mon Sep 17 00:00:00 2001 From: MinRK Date: Wed, 8 Oct 2014 13:18:31 -0700 Subject: [PATCH] adjustments to Spawner.stop - call start/stop_polling outside Spawner (avoids need for custom spawners to reimplement) - don't clear state when stopping Spawner (should enable spawners to resume) --- jupyterhub/orm.py | 11 +++-- jupyterhub/spawner.py | 95 ++++++++++++++++++++---------------- jupyterhub/tests/test_api.py | 5 +- 3 files changed, 66 insertions(+), 45 deletions(-) diff --git a/jupyterhub/orm.py b/jupyterhub/orm.py index 1354331a..9aa8a6af 100644 --- a/jupyterhub/orm.py +++ b/jupyterhub/orm.py @@ -313,6 +313,7 @@ class User(Base): api_token=api_token.token, ) yield spawner.start() + spawner.start_polling() # store state self.state = spawner.get_state() @@ -324,14 +325,18 @@ class User(Base): @gen.coroutine def stop(self): - """Stop the user's spawner""" + """Stop the user's spawner + + and cleanup after it. + """ if self.spawner is None: return + self.spawner.stop_polling() status = yield self.spawner.poll() if status is None: yield self.spawner.stop() - self.state = {} - self.spawner = None + self.state = self.spawner.get_state() + self.last_activity = datetime.utcnow() self.server = None inspect(self).session.commit() diff --git a/jupyterhub/spawner.py b/jupyterhub/spawner.py index 972b0b61..78ee31b4 100644 --- a/jupyterhub/spawner.py +++ b/jupyterhub/spawner.py @@ -14,7 +14,7 @@ from tornado.ioloop import IOLoop, PeriodicCallback from IPython.config import LoggingConfigurable from IPython.utils.traitlets import ( - Any, Bool, Dict, Enum, Instance, Integer, List, Unicode, + Any, Bool, Dict, Enum, Instance, Integer, Float, List, Unicode, ) from .utils import random_port @@ -93,9 +93,10 @@ class Spawner(LoggingConfigurable): def load_state(self, state): """load state from the database - This is the extensible part of state + This is the extensible part of state Override in a subclass if there is state to load. + Should call `super`. See Also -------- @@ -107,7 +108,8 @@ class Spawner(LoggingConfigurable): def get_state(self): """store the state necessary for load_state - A black box of extra state for custom spawners + A black box of extra state for custom spawners. + Should call `super`. Returns ------- @@ -200,6 +202,18 @@ class Spawner(LoggingConfigurable): add_callback = IOLoop.current().add_callback for callback in self._callbacks: add_callback(callback) + + death_interval = Float(0.1) + @gen.coroutine + def wait_for_death(self, timeout=10): + """wait for the process to die, up to timeout seconds""" + loop = IOLoop.current() + for i in range(int(timeout / self.poll_interval)): + status = yield self.poll() + if status is not None: + break + else: + yield gen.Task(loop.add_timeout, loop.time() + self.death_interval) def set_user_setuid(username): @@ -251,7 +265,7 @@ class LocalProcessSpawner(Spawner): ) proc = Instance(Popen) - pid = Integer() + pid = Integer(0) sudo_args = List(['-n'], config=True, help="""arguments to be passed to sudo (in addition to -u [username]) @@ -282,7 +296,8 @@ class LocalProcessSpawner(Spawner): def get_state(self): state = super(LocalProcessSpawner, self).get_state() - state['pid'] = self.pid + if self.pid: + state['pid'] = self.pid return state def sudo_cmd(self, user): @@ -311,17 +326,23 @@ class LocalProcessSpawner(Spawner): preexec_fn=self.make_preexec_fn(self.user.name), ) self.pid = self.proc.pid - self.start_polling() @gen.coroutine def poll(self): """Poll the process""" # if we started the process, poll with Popen if self.proc is not None: - raise gen.Return(self.proc.poll()) + status = self.proc.poll() + if status is not None: + # clear state if the process is done + self.pid = 0 + raise gen.Return(status) # if we resumed from stored state, # we don't have the Popen handle anymore + if not self.pid: + # no pid, not running + raise gen.Return(0) # this doesn't work on Windows, but that's okay because we don't support Windows. try: @@ -329,21 +350,23 @@ class LocalProcessSpawner(Spawner): except OSError as e: if e.errno == errno.ESRCH: # no such process, return exitcode == 0, since we don't know the exit status + self.pid = 0 raise gen.Return(0) + else: + raise else: # None indicates the process is running raise gen.Return(None) - @gen.coroutine - def _wait_for_death(self, timeout=10): - """wait for the process to die, up to timeout seconds""" - for i in range(int(timeout * 10)): - status = yield self.poll() - if status is not None: - break + def _signal(self, sig): + """send a signal, and ignore ERSCH because it just means it already died""" + try: + os.kill(self.pid, sig) + except OSError as e: + if e.errno == errno.ESRCH: + return else: - loop = IOLoop.current() - yield gen.Task(loop.add_timeout, loop.time() + 0.1) + raise @gen.coroutine def stop(self, now=False): @@ -351,39 +374,29 @@ class LocalProcessSpawner(Spawner): if `now`, skip waiting for clean shutdown """ - self.stop_polling() if not now: - # SIGINT to request clean shutdown + status = yield self.poll() + if status is not None: + return self.log.debug("Interrupting %i", self.pid) - try: - os.kill(self.pid, signal.SIGINT) - except OSError as e: - if e.errno == errno.ESRCH: - return - - yield self._wait_for_death(self.INTERRUPT_TIMEOUT) + self._signal(signal.SIGINT) + yield self.wait_for_death(self.INTERRUPT_TIMEOUT) # clean shutdown failed, use TERM status = yield self.poll() - if status is None: - self.log.debug("Terminating %i", self.pid) - try: - os.kill(self.pid, signal.SIGTERM) - except OSError as e: - if e.errno == errno.ESRCH: - return - yield self._wait_for_death(self.TERM_TIMEOUT) + if status is not None: + return + self.log.debug("Terminating %i", self.pid) + self._signal(signal.SIGTERM) + yield self.wait_for_death(self.TERM_TIMEOUT) # TERM failed, use KILL status = yield self.poll() - if status is None: - self.log.debug("Killing %i", self.pid) - try: - os.kill(self.pid, signal.SIGKILL) - except OSError as e: - if e.errno == errno.ESRCH: - return - yield self._wait_for_death(self.KILL_TIMEOUT) + if status is not None: + return + self.log.debug("Killing %i", self.pid) + self._signal(signal.SIGKILL) + yield self.wait_for_death(self.KILL_TIMEOUT) status = yield self.poll() if status is None: diff --git a/jupyterhub/tests/test_api.py b/jupyterhub/tests/test_api.py index b4c18404..90b0ab9b 100644 --- a/jupyterhub/tests/test_api.py +++ b/jupyterhub/tests/test_api.py @@ -155,6 +155,7 @@ def test_spawn(app, io_loop): user = add_user(db, name=name) r = api_request(app, 'users', name, 'server', method='post') assert r.status_code == 201 + assert 'pid' in user.state assert user.spawner is not None status = io_loop.run_sync(user.spawner.poll) assert status is None @@ -173,5 +174,7 @@ def test_spawn(app, io_loop): r = api_request(app, 'users', name, 'server', method='delete') assert r.status_code == 204 - assert user.spawner is None + assert 'pid' not in user.state + status = io_loop.run_sync(user.spawner.poll) + assert status == 0 \ No newline at end of file