diff --git a/jupyterhub/apihandlers/users.py b/jupyterhub/apihandlers/users.py index 1be43804..e5053b7b 100644 --- a/jupyterhub/apihandlers/users.py +++ b/jupyterhub/apihandlers/users.py @@ -694,14 +694,22 @@ class UserServerAPIHandler(APIHandler): asyncio.ensure_future(_remove_spawner(spawner._stop_future)) return - if spawner.pending: - raise web.HTTPError( - 400, - f"{spawner._log_name} is pending {spawner.pending}, please wait", - ) - stop_future = None - if spawner.ready: + if spawner.pending: + # we are interrupting a pending start + # hopefully nothing gets leftover + self.log.warning( + f"Interrupting spawner {spawner._log_name}, pending {spawner.pending}" + ) + spawn_future = spawner._spawn_future + if spawn_future: + spawn_future.cancel() + # Give cancel a chance to resolve? + # not sure what we would wait for here, + await asyncio.sleep(1) + stop_future = await self.stop_single_user(user, server_name) + + elif spawner.ready: # include notify, so that a server that died is noticed immediately status = await spawner.poll_and_notify() if status is None: @@ -837,7 +845,9 @@ class SpawnProgressAPIHandler(APIHandler): # not pending, no progress to fetch # check if spawner has just failed f = spawn_future - if f and f.done() and f.exception(): + if f and f.cancelled(): + failed_event['message'] = "Spawn cancelled" + elif f and f.done() and f.exception(): exc = f.exception() message = getattr(exc, "jupyterhub_message", str(exc)) failed_event['message'] = f"Spawn failed: {message}" @@ -876,7 +886,9 @@ class SpawnProgressAPIHandler(APIHandler): else: # what happened? Maybe spawn failed? f = spawn_future - if f and f.done() and f.exception(): + if f and f.cancelled(): + failed_event['message'] = "Spawn cancelled" + elif f and f.done() and f.exception(): exc = f.exception() message = getattr(exc, "jupyterhub_message", str(exc)) failed_event['message'] = f"Spawn failed: {message}" diff --git a/jupyterhub/tests/test_api.py b/jupyterhub/tests/test_api.py index 54185fc4..a32e674b 100644 --- a/jupyterhub/tests/test_api.py +++ b/jupyterhub/tests/test_api.py @@ -1558,23 +1558,20 @@ async def test_start_stop_race(app, no_patience, slow_spawn): r = await api_request(app, 'users', user.name, 'server', method='post') assert r.status_code == 202 assert spawner.pending == 'spawn' + spawn_future = spawner._spawn_future # additional spawns while spawning shouldn't trigger a new spawn with mock.patch.object(spawner, 'start') as m: r = await api_request(app, 'users', user.name, 'server', method='post') assert r.status_code == 202 assert m.call_count == 0 - # stop while spawning is not okay - r = await api_request(app, 'users', user.name, 'server', method='delete') - assert r.status_code == 400 - while not spawner.ready: - await asyncio.sleep(0.1) - + # stop while spawning is okay now spawner.delay = 3 - # stop the spawner r = await api_request(app, 'users', user.name, 'server', method='delete') assert r.status_code == 202 assert spawner.pending == 'stop' + assert spawn_future.cancelled() + assert spawner._spawn_future is None # make sure we get past deleting from the proxy await asyncio.sleep(1) # additional stops while stopping shouldn't trigger a new stop diff --git a/jupyterhub/user.py b/jupyterhub/user.py index dc008823..e6287fc1 100644 --- a/jupyterhub/user.py +++ b/jupyterhub/user.py @@ -1,13 +1,13 @@ # Copyright (c) Jupyter Development Team. # Distributed under the terms of the Modified BSD License. +import asyncio import json import warnings from collections import defaultdict -from datetime import timedelta from urllib.parse import quote, urlparse, urlunparse from sqlalchemy import inspect -from tornado import gen, web +from tornado import web from tornado.httputil import urlencode from tornado.log import app_log @@ -912,9 +912,13 @@ class User: spawner.cert_paths = await maybe_future(spawner.move_certs(hub_paths)) self.log.debug("Calling Spawner.start for %s", spawner._log_name) f = maybe_future(spawner.start()) - # commit any changes in spawner.start (always commit db changes before yield) + # commit any changes in spawner.start (always commit db changes before await) db.commit() - url = await gen.with_timeout(timedelta(seconds=spawner.start_timeout), f) + # gen.with_timeout protects waited-for tasks from cancellation, + # whereas wait_for cancels tasks that don't finish within timeout. + # we want this task to halt if it doesn't return in the time limit. + await asyncio.wait_for(f, timeout=spawner.start_timeout) + url = f.result() if url: # get ip, port info from return value of start() if isinstance(url, str):