gevent monkey patch before any other imports

[diegoholiveira]

I'm already using gunicorn with python 3.7 successfully in some of my apps in production. \o/

In those productions servers, since I'm running py3.7, I was forced to use gevent 1.3.4, and after the 1.3.x release, they introduce this warning:

/usr/local/lib/python3.7/site-packages/gunicorn/workers/ggevent.py:65: 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.
monkey.patch_all(subprocess=True)

Other issues already point this error also, like #1566. I don't know if this fix is naive and can cause other issues, but I hope we can be at least a first draft of a solution.

[jamadden]

Is there more to the error message? gevent 1.3.4 only issues that warning when it detects that there is likely to be a problem with SSL support (such as a RecursionError), and it is supposed to print the causes of that problem. I would expect to see "Modules that had direct imports (NOT patched):" or "Subclasses (NOT patched):"

In #1816 a different user reported that 1.3.4 silenced the warning, which is what I would generally expect; and if not silenced I would expect more details.

[diegoholiveira]

@jamadden here is the full error message:

/usr/local/lib/python3.7/site-packages/gunicorn/workers/ggevent.py:65: 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): ['newrelic.packages.requests.packages.urllib3.util (/usr/local/lib/python3.7/site-packages/newrelic/packages/requests/packages/urllib3/util/__init__.py)'].

I notice know that the warning is caused by the newrelic command that I use to run my apps:

newrelic-admin run-program gunicorn \
  --bind 0.0.0.0:9000 \
  -k gevent \
  --worker-connections 1001 \
  -w 5 \
  app:app

When I run the gunicorn directly, those warnings go away.

When I run with the newrelic command and my patch I got only one warning instead of one warning per worker without the patch.

[benoitc]

It's not possible to patch outside the setup function. Doing it would also patch the arbiter and break all the supervision logic. There is probably another way to remove the warning.

[jamadden]

doing this would also patch the arbiter, wich is not possible since the arbiter needs to block

I'll note that we successfully run the entire gunicorn stack, master/arbiter and workers, fully monkey-patched by gevent. The arbiter still blocks and still supervises correctly.

[benoitc]

@jamadden hrm i removed that comment to add that review (github crap) but that's odd. Since which versions of gevent are we talking about?

Anyway I think it would be better to only patch the worker and not the arbiter since they are isolated from each others.

[jamadden]

Since which versions of gevent are we talking about?

We've been running like this since September 2012, so 1.0b4?

[tilgovi]

I very much support early monkey patching if it's safe to do. I worry a little bit about anyone who might be starting background threads in server hooks.

[tilgovi]

I worry a little bit about anyone who might be starting background threads in server hooks.

This could be addressed through very good documentation with examples of how to create a real thread for CPU-bound work even when threading is already patched.

[jamadden]

I worry a little bit about anyone who might be starting background threads in server hooks.

Aside from CPU or blocking issues, there's another important difference, which is that (real, native) threads do not survive a fork, but greenlets (and thus greenlet-backed monkey-patched threads) do.

[benoitc]

I very much support early monkey patching if it's safe to do.

I think we should be careful there since it will break the isolation between the workers and the arbiter. What advantage would it brings to patch early outside the worker process?

[jamadden]

What advantage would it brings to patch early outside the worker process?

Our specific use case is that we have a large app that uses gevent and has a significant load time. It must be preloaded before the fork, it's simply not tenable to have workers individually load it. It instantiates a number of objects that gevent monkey-patches. Obviously that doesn't work if the monkey-patch happens after the fork.

In general, though, the more dependencies gunicorn has (or even the more of the standard library it uses) the chances of it importing and using something before the fork that gevent needs to patch first go up. Patching so late in the lifecycle of the application is contrary to the gevent advice to "make monkey-patching the very first thing the program does". Worse, this changes from one Python version to the next, as we saw with SSL+requests: before Python 3.6, you could get away with importing them prior to monkey-patching, but after that you can't.