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

Monkey-patching ssl after ssl has already been imported #2796

Closed
buxizhizhoum opened this issue May 12, 2022 · 5 comments
Closed

Monkey-patching ssl after ssl has already been imported #2796

buxizhizhoum opened this issue May 12, 2022 · 5 comments

Comments

@buxizhizhoum
Copy link

buxizhizhoum commented May 12, 2022

Recently we encountered a problem, if we use requests in a flask project, it will finally trigger an warning:
/Users/buxizhizhoum/.virtualenvs/gunicorn_monkey_patch/lib/python3.6/site-packages/gunicorn/workers/ggevent.py:38: MonkeyPatchWarning: Monkey-patching ssl after ssl has already been imported may lead to errors, including RecursionError on Python 3.6. It may also silently lead to incorrect behaviour on Python 3.7. Please monkey-patch earlier. See https://github.com/gevent/gevent/issues/1016. Modules that had direct imports (NOT patched): ['urllib3.util (/Users/bytedance/.virtualenvs/gunicorn_monkey_patch/lib/python3.6/site-packages/urllib3/util/__init__.py)', 'urllib3.util.ssl_ (/Users/bytedance/.virtualenvs/gunicorn_monkey_patch/lib/python3.6/site-packages/urllib3/util/ssl_.py)'].
This could be reproduced with the code below:
the app.py file

#!/usr/bin/python
# -*- coding: utf-8 -*-
from flask import Flask
import requests


app = Flask(__name__)


@app.route("/test")
def test():
    requests.get("https://www.baidu.com")
    return "test"


if __name__ == "__main__":
    app.run(debug=True)

the command to start it:
gunicorn -w 8 --worker-class gevent --preload -b 0.0.0.0:5000 app:app

A simplified start procedure of gunicorn is:
1.parse config
2.load app
3.init worker
gunicorn imported the ssl module(config.py in gevent) when loading config,however the monkey patch is called when init the worker which is definitely after import of ssl, this is why we get the warning that mentioned above.

We tired 3 ways to solve this problem

  1. monkey patch at the beginning of app.py
  2. use a config file for gunicorn, and monkey patch it at the beginning of config file.
  3. modify GeventWorker.patch in gevent.ggevent.py, change monkey.patch_all() to monkey.patch_all(ssl=False)

the method 1 has no use, since import ssl is before load app
the method 2 could work, if we regardless of the drawback that we patched 2 times, method 2 is almost the best work around that I could find.
the method 3 could also work, the drawback is ssl is blocking now.

And besides all of above, I just wonder could we provide a choice that allow users to set ssl patch to False, and it could solve the problem, considering the fact that patch ssl after import may cause severe problems, leave ssl not patched might be an solution for someone that could accept the performance lose?

And if wen change app.py file to lines below, it could trigger RecursionError: maximum recursion depth exceeded while calling a Python object
app.py that could trigger RecursionError

#!/usr/bin/python
# -*- coding: utf-8 -*-
from flask import Flask
import requests

app = Flask(__name__)

from requests.packages.urllib3.util.ssl_ import create_urllib3_context
ctx = create_urllib3_context()


@app.route("/test")
def test():
    requests.get("https://www.baidu.com")
    return "test"


if __name__ == "__main__":
    app.run(debug=True)

When we start it with gunicorn and call http://127.0.0.1/test it will raise RecursionError, which is mentioned at Gunicorn RecursionError with gevent and requests in python 3.6.2

The environment

Flask             1.1.2
gevent            1.5.0
gunicorn          20.1.0
requests          2.25.1
@panteparak
Copy link

panteparak commented Oct 5, 2022

create a file called geventlet-config.py with the following content

try:
    import gevent.monkey
    gevent.monkey.patch_all()
except ImportError:
    pass

now add the following argument to your gunicorn. --config geventlet-config.py

So if you started gunicorn using gunicorn --config geventlet-config.py -w 8 --worker-class gevent -b 0.0.0.0:5000 app:app. The patch should be applied

@marklitle
Copy link

delete --preload

@scmccarthy
Copy link

create a file called geventlet-config.py with the following content

try:
    import gevent.monkey
    gevent.monkey.patch_all()
except ImportError:
    pass

now add the following argument to your gunicorn. --config geventlet-config.py

So if you started gunicorn using gunicorn --config geventlet-config.py -w 8 --worker-class gevent -b 0.0.0.0:5000 app:app. The patch should be applied

Maybe I'm missing something? This solution doesn't work for me - I still get the same message about how "Monkey-patching ssl after ssl has already been imported may lead to errors" even when I do the monkey patch in the gunicorn config file. And when I look into gunicorn's code that processes config, it imports ssl, which makes me wonder how this could ever work.

python 3.10.6
Flask==3.0.2
gevent==24.2.1
gunicorn==22.0.0

@benoitc
Copy link
Owner

benoitc commented Aug 6, 2024

@scmccarthy try to import request later in your app rather than in the main module. This should be enough.

@benoitc benoitc closed this as completed Aug 6, 2024
@scmccarthy
Copy link

I don't understand, @benoitc . I'm not importing request in the main module.

From what I could tell, gunicorn will import ssl before my own code can do anything. And then if I ever call gevent.monkey.patch_all(), we get that "Monkey-patching ssl after ssl has already been imported may lead to errors" message.

I had to just give up on monkey patching.

JohnDoeAnonITA added a commit to JohnDoeAnonITA/Kraken that referenced this issue Sep 2, 2024
Fix:
[i] Enter the target: https://***
[*] Processing site input...
[*] Validating if https://*** is a WordPress site...
[-] Error during validation: maximum recursion depth exceeded
2024-09-02 13:55:10,721 - ERROR - Validation error: maximum recursion depth exceeded

on https sites, monkey tries to load the ssl when it is already loaded by requests

ref: benoitc/gunicorn#2796
MCBoarder289 added a commit to MCBoarder289/shelf-help that referenced this issue Sep 28, 2024
Updating to a gevent worker class for gunicorn caused the app to fail on its api requests.
It threw a maximum recursion depth exceeded, and this was a warning produced at the
top of the logs as well. Resolved by using the solution referenced here:
benoitc/gunicorn#2796 (comment)
MCBoarder289 added a commit to MCBoarder289/shelf-help that referenced this issue Sep 28, 2024
Updating to a gevent worker class for gunicorn caused the app to fail on its api requests.
It threw a maximum recursion depth exceeded, and this was a warning produced at the
top of the logs as well. Resolved by using the solution referenced here:
benoitc/gunicorn#2796 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants