bugfix: shutdown_server returns True when pid exists

[asreimer]

The Bug

check_pid returns True if the PID for a notebook server still exists. Therefore, the if check_pid(pid): statements here and here evaluate to True even though the notebook server is still running. This is not the expected behaviour, since this means that the shutdown_server function will return and return True even though the notebook server is still in the process of shutting down.

Reproducing the Bug

The following procedure was tested in a python3.7 virtual environment with jupyter 4.4.0

1. in a terminal, start a notebook server: jupyter notebook --port=8888
2. open a jupyter notebook and start an infinite loop, such as: while True: pass
3. in another terminal run the following: python -c 'from notebook.notebookapp import shutdown_server, list_running_servers; port=8888; servers = [x for x in list_running_servers() if x["port"] == port]; status = True if len(servers) == 0 else shutdown_server(servers[0]); print(status)' && bash -c 'ps -ef | grep "jupyter-notebook"'
4. Notice that the output of step 3) shows that:
   a) shutdown_server returns True and then,
   b) the jupyter-notebook process is still running`

In step 4, the expected behaviour would be for the output of the bash -c 'ps -ef | grep "jupyter-notebook"' command to not show a jupyter notebook server running.

Note: the while loop helps make this bug more reproducible. It's intermittent without the loop due to factors such as system load and machine specs. Also, that big command in step 3 is importing shutdown_server and then running it to kill the notebook server we started in step 1, that's all.

For step 3, here's the output I get on my system:

True
asreimer 21687 11734  8 15:56 pts/4    00:00:01 /home/asreimer/virtualenvs/env3.7/bin/python3.7 /home/asreimer/virtualenvs/env3.7/bin/jupyter-notebook --port=8888
asreimer 21794 15361  0 15:57 pts/5    00:00:00 bash -c ps -ef | grep "jupyter-notebook"
asreimer 21803 21794  0 15:57 pts/5    00:00:00 grep jupyter-notebook

Notice that the jupyter-notebook --port=8888 process is still running, despite shutdown_server telling us that the server was shutdown. Also notice that step 3 executes very fast, yet the jupyter notebook app will take several seconds to shutdown.

Note: the notebook server does eventually shutdown, [due to the api call on these] lines(https://github.com/jupyter/notebook/blob/master/notebook/notebookapp.py#L413-L420) succeeding. The bug here is that the shutdown happens after the shutdown_server function returns, instead of happening before shudown_server returns.

Who Cares?

I'm working on a project where we are running jupyter notebooks in docker containers. We need to be able to shutdown the notebooks by sending a shutdown command of some kind to the notebook server running in the docker container. Using the shutdown_server function didn't result in the expected behaviour, so I started digging in to why and found this bug.

The Fix

This commit adds a not to each line: if not check_pid(pid): so that the conditional only evaluates to True if check_pid returns False, which happens when the notebook server has shutdown, as expected.

After applying the fix, the output of steps 1 through 3 detailed above is as expected:

True
asreimer 21627 15361  0 16:08 pts/5    00:00:00 bash -c ps -ef | grep "jupyter-notebook"
asreimer 21636 21627  0 16:08 pts/5    00:00:00 grep jupyter-notebook

you will also notice that it step 3 will take as much time to execute as it takes for the jupyter notebook server to shutdown.

[takluyver]

Thanks, this looks totally sensible. Thanks for the detailed description!