encode / uvicorn

An ASGI web server, for Python. 🦄

https://www.uvicorn.org/

BSD 3-Clause "New" or "Revised" License

8.11k stars 701 forks source link

Remove forced `raise_signal` calls #2314

Closed kleschenko closed 1 month ago

kleschenko commented 2 months ago

Summary

This change removes the raise_signal call which breaks default signal handling for subprocesses.

Closes https://github.com/encode/uvicorn/issues/2289 (and possibly https://github.com/encode/uvicorn/issues/2294)

Checklist

[x] I understand that this PR may be closed in case there was no previous discussion. (This doesn't apply to typos!)
[x] I've added a test for each change that was introduced, and I tried as much as possible to make a single atomic change.
[ ] I've updated the documentation accordingly.

Kludex commented 2 months ago

Well... Then the goal of the previous PR is just ignored...

kleschenko commented 2 months ago

@Kludex I've just tested the snippet from the original issue https://github.com/encode/uvicorn/issues/1579 and it works as expected for me with this change

Kludex commented 2 months ago

Does this makes sense @maxfischer2781 ?

maxfischer2781 commented 2 months ago

I honestly don't know how the raise_signal is supposed to affect subprocesses in the first place; there is no clear repro for #2289 so I cannot say what is the actual issue.

2294 should definitely not be fixed by removing the raised signal. Yes, the server properly killing itself will lead to a traceback but if the traceback is undesired then only the traceback should be suppressed, not the proper killing.

maxfischer2781 commented 1 month ago

@Kludex I've created #2317 instead to fix the issues I could reproduce.