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

@@ -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.state = self.spawner.get_state()
-        self.spawner = None
+        self.last_activity = datetime.utcnow()
         self.server = None
         inspect(self).session.commit()
 

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 _signal(self, sig):
-    def _wait_for_death(self, timeout=10):
+        """send a signal, and ignore ERSCH because it just means it already died"""
-        """wait for the process to die, up to timeout seconds"""
+        try:
-        for i in range(int(timeout * 10)):
+            os.kill(self.pid, sig)
-            status = yield self.poll()
+        except OSError as e:
-            if status is not None:
+            if e.errno == errno.ESRCH:
-                break
+                return
             else:
-                loop = IOLoop.current()
+                raise
-                yield gen.Task(loop.add_timeout, loop.time() + 0.1)
     
     @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:
+            self._signal(signal.SIGINT)
-                os.kill(self.pid, signal.SIGINT)
+            yield self.wait_for_death(self.INTERRUPT_TIMEOUT)
-            except OSError as e:
-                if e.errno == errno.ESRCH:
-                    return
-            
-            yield self._wait_for_death(self.INTERRUPT_TIMEOUT)
         
         # clean shutdown failed, use TERM
         status = yield self.poll()
-        if status is None:
+        if status is not None:
-            self.log.debug("Terminating %i", self.pid)
+            return
-            try:
+        self.log.debug("Terminating %i", self.pid)
-                os.kill(self.pid, signal.SIGTERM)
+        self._signal(signal.SIGTERM)
-            except OSError as e:
+        yield self.wait_for_death(self.TERM_TIMEOUT)
-                if e.errno == errno.ESRCH:
-                    return
-            yield self._wait_for_death(self.TERM_TIMEOUT)
         
         # TERM failed, use KILL
         status = yield self.poll()
-        if status is None:
+        if status is not None:
-            self.log.debug("Killing %i", self.pid)
+            return
-            try:
+        self.log.debug("Killing %i", self.pid)
-                os.kill(self.pid, signal.SIGKILL)
+        self._signal(signal.SIGKILL)
-            except OSError as e:
+        yield self.wait_for_death(self.KILL_TIMEOUT)
-                if e.errno == errno.ESRCH:
-                    return
-            yield self._wait_for_death(self.KILL_TIMEOUT)
 
         status = yield self.poll()
         if status is None:

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