benoitc / gunicorn

gunicorn 'Green Unicorn' is a WSGI HTTP Server for UNIX, fast clients and sleepy applications.
http://www.gunicorn.org
Other
9.73k stars 1.74k forks source link

module invocation breaks USR2 upgrading, as modified argv results in modified path #3281

Open pajod opened 3 weeks ago

pajod commented 3 weeks ago

Mixing sys.executable and parsed argv produces unintentional sys.path changes on re-exec:

# python -m gunicorn my:app
Booting worker with pid: 201117
[..]
# kill -USR2 $(pgrep --oldest -f "gunicorn")
Handling signal: usr2
[..]
  File "/venv/lib/python3.11/site-packages/werkzeug/serving.py", line 28, in <module>
    from http.server import BaseHTTPRequestHandler
ModuleNotFoundError: No module named 'http.server'

https://github.com/benoitc/gunicorn/blob/903792f152af6a27033d458020923cb2bcb11459/gunicorn/arbiter.py#L68-L76

(Side note: The p is wrong and scaring static analyzers, but harmless in cPython. I suggest cleaning it up while we are at it.)

https://github.com/benoitc/gunicorn/blob/903792f152af6a27033d458020923cb2bcb11459/gunicorn/arbiter.py#L430-L431

I think we just did not have the option to use the original arguments (added in 3.10):

diff a/gunicorn/arbiter.py b/gunicorn/arbiter.py
-    os.execve(sys.executable, [sys.executable, *sys.argv[:]], environ)
+    os.execve(sys.executable, sys.orig_argv, environ)

Objections against just doing just that, on Python versions where we can? Even if we cannot do that for some reason, I would still add a warning when launching with clearly unintended sys.path - it is a common source of confusion.

benoitc commented 2 weeks ago

what does mean Mixing sys.executable and parsed argv produces unintentional sys.path changes on re-exec: ? Can you provide an example?

pajod commented 2 weeks ago

Can you provide an example?

python -m gunicorn my:app is already the simplest and most obvious example. Add print(sys.path, sys.argv, sys.orig_argv) calls to your (cPython 3.10+) app if you want to see it in action.