From 2e73a47ffa744dfcf5b362787b065255c20da7bb Mon Sep 17 00:00:00 2001 From: David Szotten Date: Fri, 30 Jan 2015 14:53:30 +0000 Subject: [PATCH 1/6] keep an eventlet-friendly reference to time.sleep otherwise, when eventlet.monkey_patch replaces time.sleep, this raises a TypeError when called, since while adding the builtin sleep to the ReloaderLoop we stil end up with a function, but when replaced, we end up with a method, which now expects a self argument. --- werkzeug/_reloader.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/werkzeug/_reloader.py b/werkzeug/_reloader.py index e0ea56647..990d55b3e 100644 --- a/werkzeug/_reloader.py +++ b/werkzeug/_reloader.py @@ -107,9 +107,13 @@ def _walk(node, path): class ReloaderLoop(object): - _sleep = staticmethod(time.sleep) # monkeypatched by testsuite name = None + # monkeypatched by testsuite + @staticmethod + def _sleep(secs): + time.sleep(secs) + def __init__(self, extra_files=None, interval=1): self.extra_files = set(os.path.abspath(x) for x in extra_files or ()) From c03aacf5453472f70bec7e60254e3b1c984fa8c4 Mon Sep 17 00:00:00 2001 From: David Szotten Date: Sat, 31 Jan 2015 09:18:39 +0000 Subject: [PATCH 2/6] add explaining comment --- werkzeug/_reloader.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/werkzeug/_reloader.py b/werkzeug/_reloader.py index 990d55b3e..260b994f1 100644 --- a/werkzeug/_reloader.py +++ b/werkzeug/_reloader.py @@ -109,7 +109,12 @@ def _walk(node, path): class ReloaderLoop(object): name = None - # monkeypatched by testsuite + # monkeypatched by testsuite. this used to just say `_sleep = time.sleep`, + # but because c functions behave differently to python functions when + # attached to classes, this breaks if sleep has been already replaced with + # a python function (e.g. by eventlet.monkey_patch). python functions are + # descriptors and become methods when attahed to classes, whereas c + # functions remain as functions (thus behaving like staticmethods) @staticmethod def _sleep(secs): time.sleep(secs) From 6a7c1ea24f1690bcd20ae4325e349821325b087e Mon Sep 17 00:00:00 2001 From: David Szotten Date: Sat, 31 Jan 2015 09:18:46 +0000 Subject: [PATCH 3/6] add test --- tests/test_serving.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/tests/test_serving.py b/tests/test_serving.py index d4a4e8015..ed33cfa16 100644 --- a/tests/test_serving.py +++ b/tests/test_serving.py @@ -10,6 +10,7 @@ """ import os import ssl +import subprocess import textwrap @@ -171,3 +172,21 @@ def real_app(environ, start_response): r = requests.get(server.url) assert r.status_code == 200 assert r.content == b'hello' + + +def test_monkeypached_sleep(tmpdir): + script = tmpdir.mkdir('app').join('test.py') + script.write(textwrap.dedent(''' + import time + + def sleep(secs): + pass + + # simulate eventlet.monkey_patch by replacing the builtin sleep + # with a regular function + time.sleep = sleep + + from werkzeug._reloader import ReloaderLoop + ReloaderLoop()._sleep(0) + ''')) + subprocess.check_call(['python', str(script)]) From 8396d1eac5fa36b4f970854359c4c736b9ba9cf5 Mon Sep 17 00:00:00 2001 From: Markus Unterwaditzer Date: Sat, 31 Jan 2015 10:54:28 +0100 Subject: [PATCH 4/6] add changelog for #663 --- CHANGES | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGES b/CHANGES index 22e5f11ed..149e244dc 100644 --- a/CHANGES +++ b/CHANGES @@ -7,6 +7,8 @@ Version 0.10.1 (bugfix release, release date to be decided) - Fixed regression with multiple query values for URLs (pull request ``#667``). +- Fix issues with eventlet's monkeypatching and the builtin server (pull + request ``#663``). Version 0.10 ------------ From 5cc17f05fa041175cf8b8a5a69f76e182de9d211 Mon Sep 17 00:00:00 2001 From: ThiefMaster Date: Sat, 31 Jan 2015 11:08:04 +0100 Subject: [PATCH 5/6] Fix typo --- werkzeug/_reloader.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/werkzeug/_reloader.py b/werkzeug/_reloader.py index 260b994f1..425fb1efa 100644 --- a/werkzeug/_reloader.py +++ b/werkzeug/_reloader.py @@ -113,7 +113,7 @@ class ReloaderLoop(object): # but because c functions behave differently to python functions when # attached to classes, this breaks if sleep has been already replaced with # a python function (e.g. by eventlet.monkey_patch). python functions are - # descriptors and become methods when attahed to classes, whereas c + # descriptors and become methods when attached to classes, whereas c # functions remain as functions (thus behaving like staticmethods) @staticmethod def _sleep(secs): From 8bc8b1a46622075ca39523cbae09afa7f442b1d4 Mon Sep 17 00:00:00 2001 From: David Szotten Date: Sat, 31 Jan 2015 10:06:22 +0000 Subject: [PATCH 6/6] nicer implementation also use briefer description in the code, and move the longer explanation to the test --- tests/test_serving.py | 9 ++++++++- werkzeug/_reloader.py | 13 ++++--------- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/tests/test_serving.py b/tests/test_serving.py index ed33cfa16..55ac707b8 100644 --- a/tests/test_serving.py +++ b/tests/test_serving.py @@ -175,6 +175,13 @@ def real_app(environ, start_response): def test_monkeypached_sleep(tmpdir): + # removing the staticmethod wrapper in the definition of + # ReloaderLoop._sleep works most of the time, since `sleep` is a c + # function, and unlike python functions which are descriptors, doesn't + # become a method when attached to a class. however, if the user has called + # `eventlet.monkey_patch` before importing `_reloader`, `time.sleep` is a + # python function, and subsequently calling `ReloaderLoop._sleep` fails + # with a TypeError. This test checks that _sleep is attached correctly. script = tmpdir.mkdir('app').join('test.py') script.write(textwrap.dedent(''' import time @@ -183,7 +190,7 @@ def sleep(secs): pass # simulate eventlet.monkey_patch by replacing the builtin sleep - # with a regular function + # with a regular function before _reloader is imported time.sleep = sleep from werkzeug._reloader import ReloaderLoop diff --git a/werkzeug/_reloader.py b/werkzeug/_reloader.py index 425fb1efa..83716f16e 100644 --- a/werkzeug/_reloader.py +++ b/werkzeug/_reloader.py @@ -109,15 +109,10 @@ def _walk(node, path): class ReloaderLoop(object): name = None - # monkeypatched by testsuite. this used to just say `_sleep = time.sleep`, - # but because c functions behave differently to python functions when - # attached to classes, this breaks if sleep has been already replaced with - # a python function (e.g. by eventlet.monkey_patch). python functions are - # descriptors and become methods when attached to classes, whereas c - # functions remain as functions (thus behaving like staticmethods) - @staticmethod - def _sleep(secs): - time.sleep(secs) + # monkeypatched by testsuite. wrapping with `staticmethod` is required in + # case time.sleep has been replaced by a non-c function (e.g. by + # `eventlet.monkey_patch`) before we get here + _sleep = staticmethod(time.sleep) def __init__(self, extra_files=None, interval=1): self.extra_files = set(os.path.abspath(x)