SIGTERM on multiple workers fails

[euri10]

Fix for #668

it essentially reverts #620 and applies what the python documentation recommends ie terminate or close a process before join

To summarize, before #620 processes in docker containers were not killed gracefully, so it fixed that, but it introduced a bug described in #668 where for non-docker containers the shutdown was too abrupt and children lifespan events were skipped.

I think this PR deals nicely with both issues, it's rather boring to test things inside and out container, so I hope this is ok

[rafalp]

Does this mean my worker can shutdown in middle of transaction happening in, say, Starlette's backgorund task?

[euri10]

not sure on this one,
one thing also it that I miread the doc, it says to terminate before join in case of a pool, we are not in this case....(https://docs.python.org/3/library/multiprocessing.html?highlight=join#multiprocessing.pool.Pool.join)

kind of stuck, dunno how to solve this, this is annoying to get the 20s timeout reached in a container

[aviramha]

@rafalp I think it wont shutdown in middle of transaction.
Uvicorn handles sigterm, and sends shutdown event to Starlette.
If it does, it's a Starlette issue (maybe on shutdown Starlette should wait for all background tasks..)

[euri10]

ok after putting loggers basically on every line it appear to me that the mutiprocess can hang indefinitely not only on docker but also normally.

docker sends a SIGTERM signal and not a SIGINT

now if your NOT in a container and kill -15 parentPID then the supervisor will correctly capture the signal, enter def signal_handler(self, sig, frame): that sets the event, then enter the shutdown method.

def shutdown(self):
        for process in self.processes:
            process.join()

now here is the deal, if the signal is a SIGTERM, it will hang indefinitely on process.join()

so I think in that case only we need to terminate() the process, maybe there would be a nicer way that handles that case

In case of one worker a SIGTERM is nicely dealt with the

uvicorn/uvicorn/main.py

Line 571 in a839da2

def handle_exit(self, sig, frame):

[tomchristie]

Similar comment wrt. the Pull Request name on this as in #636 😄