Several fixes for monkey-patching reported by users.

See changelog.rst for details.

changelog.rst

@@ -30,6 +30,16 @@
   requires having Cython installed first. (Note that the binary installation
   formats (wheels, exes, msis) are preferred on Windows.) Reported in
   :issue:`757` by Ned Batchelder.
+- Issue a warning when :func:`~gevent.monkey.patch_all` is called with
+  ``os`` set to False (*not* the default) but ``signal`` is still True
+  (the default). This combination of parameters will cause signal
+  handlers for ``SIGCHLD`` to not get called. In the future this might
+  raise an error. Reported by Josh Zuech.
+- Issue a warning when :func:`~gevent.monkey.patch_all` is called more
+  than once with different arguments. That causes the cumulative set of all True
+  arguments to be patched, which may cause unexpected results.
+- Fix returning the original values of certain ``threading``
+  attributes from :func:`gevent.monkey.get_original`.
 
 .. _bug 13502: http://bugs.python.org/issue13502
 

gevent/monkey.py

@@ -89,8 +89,8 @@ if sys.platform.startswith("win"):
 else:
     WIN = False
 
-# maps module name -> attribute name -> original item
-# e.g. "time" -> "sleep" -> built-in function sleep
+# maps module name -> {attribute name: original item}
+# e.g. "time" -> {"sleep": built-in function sleep}
 saved = {}
 
 
@@ -161,6 +161,25 @@ def patch_module(name, items=None):
             raise AttributeError('%r does not have __implements__' % gevent_module)
     for attr in items:
         patch_item(module, attr, getattr(gevent_module, attr))
+    return module
+
+
+_warnings = list()
+
+
+def _queue_warning(message):
+    # Queues a warning to show after the monkey-patching process is all done.
+    # Done this way to avoid extra imports during the process itself, just
+    # in case
+    _warnings.append(message)
+
+
+def _process_warnings():
+    import warnings
+    _w = list(_warnings)
+    del _warnings[:]
+    for warning in _w:
+        warnings.warn(warning, RuntimeWarning, stacklevel=3)
 
 
 def _patch_sys_std(name):
@@ -293,10 +312,20 @@ def patch_thread(threading=True, _threading_local=True, Event=False, logging=Tru
     #   return r, w
     # os.pipe = _pipe
 
+    # The 'threading' module copies some attributes from the
+    # thread module the first time it is imported. If we patch 'thread'
+    # before that happens, then we store the wrong values in 'saved',
+    # So if we're going to patch threading, we either need to import it
+    # before we patch thread, or manually clean up the attributes that
+    # are in trouble. The latter is tricky because of the different names
+    # on different versions.
+    if threading:
+        __import__('threading')
+
     patch_module('thread')
     if threading:
-        patch_module('threading')
-        threading = __import__('threading')
+        threading = patch_module('threading')
+
         if Event:
             from gevent.event import Event
             patch_item(threading, 'Event', Event)
@@ -348,11 +377,19 @@ def patch_thread(threading=True, _threading_local=True, Event=False, logging=Tru
                         sleep(0.01)
 
             main_thread.join = join
+
+            # Patch up the ident of the main thread to match. This
+            # matters if threading was imported before monkey-patching
+            # thread
+            oldid = main_thread.ident
+            main_thread._ident = threading.get_ident()
+            if oldid in threading._active:
+                threading._active[main_thread.ident] = threading._active[oldid]
+            if oldid != main_thread.ident:
+                del threading._active[oldid]
         else:
-            # TODO: Can we use warnings here or does that mess up monkey patching?
-            print("Monkey-patching not on the main thread; "
-                  "threading.main_thread().join() will hang from a greenlet",
-                  file=sys.stderr)
+            _queue_warning("Monkey-patching not on the main thread; "
+                           "threading.main_thread().join() will hang from a greenlet")
 
 
 def patch_socket(dns=True, aggressive=True):
@@ -482,11 +519,20 @@ def patch_signal():
     """
     patch_module("signal")
 
+def _check_repatching(**module_settings):
+    if saved.get('_gevent_saved_patch_all', module_settings) != module_settings:
+        _queue_warning("Patching more than once will result in the union of all True"
+                       " parameters being patched")
+
+    saved['_gevent_saved_patch_all'] = module_settings
 
 def patch_all(socket=True, dns=True, time=True, select=True, thread=True, os=True, ssl=True, httplib=False,
               subprocess=True, sys=False, aggressive=True, Event=False,
               builtins=True, signal=True):
     """Do all of the default monkey patching (calls every other applicable function in this module)."""
+    # Check to see if they're changing the patched list
+    _check_repatching(**locals())
+
     # order is important
     if os:
         patch_os()
@@ -511,8 +557,15 @@ def patch_all(socket=True, dns=True, time=True, select=True, thread=True, os=Tru
     if builtins:
         patch_builtins()
     if signal:
+        if not os:
+            _queue_warning('Patching signal but not os will result in SIGCHLD handlers'
+                           ' installed after this not being called and os.waitpid may not'
+                           ' function correctly if gevent.subprocess is used. This may raise an'
+                           ' error in the future.')
         patch_signal()
 
+    _process_warnings()
+
 
 def main():
     args = {}

greentest/test__monkey.py

@@ -39,3 +39,34 @@ for modname in monkey.saved:
 
     for objname in monkey.saved[modname]:
         assert monkey.is_object_patched(modname, objname)
+
+orig_saved = {}
+for k, v in monkey.saved.items():
+    orig_saved[k] = v.copy()
+
+import warnings
+with warnings.catch_warnings(record=True) as issued_warnings:
+    # Patch again, triggering two warnings, on for os=False/signal=True,
+    # one for repeated monkey-patching.
+    monkey.patch_all(os=False)
+    assert len(issued_warnings) == 2, len(issued_warnings)
+    assert 'SIGCHLD' in str(issued_warnings[-1].message), issued_warnings[-1]
+    assert 'more than once' in str(issued_warnings[0].message), issued_warnings[0]
+
+    # Patching with the exact same argument doesn't issue a second warning.
+    # (just repeats the signal warning)
+    del issued_warnings[:]
+    monkey.patch_all(os=False)
+    orig_saved['_gevent_saved_patch_all'] = monkey.saved['_gevent_saved_patch_all']
+
+    assert len(issued_warnings) == 1, len(issued_warnings)
+    assert 'SIGCHLD' in str(issued_warnings[-1].message), issued_warnings[-1]
+
+# Make sure that re-patching did not change the monkey.saved
+# attribute, overwriting the original functions
+assert orig_saved == monkey.saved
+
+# Make sure some problematic attributes stayed correct.
+# NOTE: This was only a problem if threading was not previously imported.
+for k, v in monkey.saved['threading'].items():
+    assert 'gevent' not in str(v), (k, v)