Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
pajod opened this issue Aug 19, 2024 · 2 comments · May be fixed by #3285
Open

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

pajod opened this issue Aug 19, 2024 · 2 comments · May be fixed by #3285

Comments

@pajod
Copy link
Contributor

pajod commented Aug 19, 2024

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'
  • this is not an issue when you run gunicorn my:app (with argv[0] == '[..]/gunicorn')
  • this is an issue when argv[0] is significant, such as python3 -m gunicorn my:app
    • then parsed sys.argv may point to [..]/__main__.py
    • then relaunching using parsed argv will result in sys.path listing directory containing __main__.py
    • then wsgi apps using werkzeug (such as flask) will crash because we clobbered the http name
    • which we never meant to do anyway
    • and crucially, did not do .. until we launched new master after USR2

args = sys.argv[:]
args.insert(0, sys.executable)
# init start context
self.START_CTX = {
"args": args,
"cwd": cwd,
0: sys.executable
}

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

# exec the process using the original environment
os.execvpe(self.START_CTX[0], self.START_CTX['args'], environ)

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
Copy link
Owner

benoitc commented Aug 27, 2024

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

@pajod
Copy link
Contributor Author

pajod commented Aug 27, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants