jupyterhub / jupyterhub/simpervisor

Simpervisor overrides the existing signal handlers

Open
#38 2 comments 1 reaction 0 assignees View on GitHub
bug
Dominant language
Python
Stars
14
Forks
9
PR merge metrics
No merged PRs in 30d

Description

### Proposed change
Simpervisor defines its own [atexit](https://github.com/jupyterhub/simpervisor/blob/main/simpervisor/atexitasync.py) implementation to register signal handlers. The issue is that sets its own signal handler for `SIGINT` and `SIGTERM` "potentially" overriding any existing handlers.

An example use case when we use [jupyter-server-proxy](https://github.com/jupyterhub/jupyter-server-proxy) to start managed applications. It uses simpervisor behind the hood to supervise the proxy application. By the time simpervisor registers signal handlers, there are already existing handlers for [sigint](https://github.com/jupyter-server/jupyter_server/blob/32df893ef2fdd40a102e73a361c4280dbf042b71/jupyter_server/serverapp.py#L2185-L2193) and [sigterm](https://github.com/jupyter-server/jupyter_server/blob/32df893ef2fdd40a102e73a361c4280dbf042b71/jupyter_server/serverapp.py#L2239-L2242) by `ServerApp`. So, simpervisor overrides these handlers registered by `ServerApp`. This can be easily verified by running a JupyterLab instance with `jupyter-server-proxy` installed. Start a simple web server from JupyterLab using `jupyter-server-proxy` and then doing a Ctrl+C on the terminal that is running JupyterLab will shutdown the Lab immediately without the usual prompt of shutdown [confirmation](https://github.com/jupyter-server/jupyter_server/blob/32df893ef2fdd40a102e73a361c4280dbf042b71/jupyter_server/serverapp.py#L2199-L2237).

The signal handlers of the `ServerApp` cleans up the [kernels](https://github.com/jupyter-server/jupyter_server/blob/32df893ef2fdd40a102e73a361c4280dbf042b71/jupyter_server/serverapp.py#L2558-L2571) and [extensions](https://github.com/jupyter-server/jupyter_server/blob/32df893ef2fdd40a102e73a361c4280dbf042b71/jupyter_server/serverapp.py#L2573-L2582) before terminating itself. So, we lose this cleaning up tasks due to simpervisor overriding the handlers. We noticed on our JupyterHub deployment on our HPC platform that this can have a undesirable side effects of proxy processes lingering around even after `ServerApp` is killed.

### Who would use this feature?
Anyone who is using `simpervisor` along with Jupyter `ServerApp`.

### (Optional): Suggest a solution
Ensure that we register the existing signal handlers in [_handlers](https://github.com/jupyterhub/simpervisor/blob/ffd8787fd440995b8dc542959d36f76fd05e65c0/simpervisor/atexitasync.py#L9) list, if they exist.

Contributor guide

No contributing guide indexed for this repository

Research direction

Start in simpervisor/atexitasync.py, especially the _handlers list and the signal registration path. Reproduce the issue with JupyterLab and jupyter-server-proxy by starting a proxied web server and pressing Ctrl+C. Done means existing SIGINT and SIGTERM handlers remain effective so ServerApp shutdown confirmation and cleanup still occur.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
backend, operating-systems
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Clearly specified
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.