From 2d40e6daceb9735d27bb91d9c32743695de8e01c Mon Sep 17 00:00:00 2001 From: Jason Madden Date: Sat, 4 Jan 2020 06:31:25 -0600 Subject: [PATCH] Use socket.sendfile() instead of os.sendfile(). Fixes #2223. Unfortunately, eventlet doesn't implement GreenSocket.sendfile, so we have to do it for it. Add gevent and eventlet to tox.ini and add tests to make sure we can at least import the workers. Some tests that this actually functions would be nice... Update the gevent and eventlet setup extras to require the versions that are enforced in their worker modules. --- .travis.yml | 20 +++++------ gunicorn/http/wsgi.py | 13 +------ gunicorn/workers/geventlet.py | 61 ++++++++++++++++++++++++++------- gunicorn/workers/ggevent.py | 19 ---------- requirements_test.txt | 2 ++ setup.py | 4 +-- tests/workers/__init__.py | 0 tests/workers/test_geventlet.py | 7 ++++ tests/workers/test_ggevent.py | 7 ++++ tox.ini | 2 +- 10 files changed, 77 insertions(+), 58 deletions(-) create mode 100644 tests/workers/__init__.py create mode 100644 tests/workers/test_geventlet.py create mode 100644 tests/workers/test_ggevent.py diff --git a/.travis.yml b/.travis.yml index f2d3c41a..2e11c55a 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,33 +1,31 @@ -sudo: false language: python matrix: include: - python: 3.8 env: TOXENV=lint - dist: xenial - sudo: true - python: 3.5 env: TOXENV=py35 - python: 3.6 env: TOXENV=py36 - python: 3.7 env: TOXENV=py37 - dist: xenial - sudo: true - python: pypy3 - env: TOXENV=pypy3 + env: + - TOXENV=pypy3 + # Embedded c-ares takes a long time to build and + # as-of 2020-01-04 there are no PyPy3 manylinux + # wheels for gevent on PyPI. + - GEVENTSETUP_EMBED_CARES=no dist: xenial - python: 3.8 env: TOXENV=py38 - dist: xenial - sudo: true - python: 3.8 env: TOXENV=docs-lint - dist: xenial - sudo: true -install: pip install tox +install: pip install -U tox coverage # TODO: https://github.com/tox-dev/tox/issues/149 script: tox --recreate +after_success: + - if [ -f .coverage ]; then coverage report ; fi cache: directories: - .tox diff --git a/gunicorn/http/wsgi.py b/gunicorn/http/wsgi.py index 32f6c0b4..17360826 100644 --- a/gunicorn/http/wsgi.py +++ b/gunicorn/http/wsgi.py @@ -360,12 +360,6 @@ class Response(object): offset = os.lseek(fileno, 0, os.SEEK_CUR) if self.response_length is None: filesize = os.fstat(fileno).st_size - - # The file may be special and sendfile will fail. - # It may also be zero-length, but that is okay. - if filesize == 0: - return False - nbytes = filesize - offset else: nbytes = self.response_length @@ -378,12 +372,7 @@ class Response(object): chunk_size = "%X\r\n" % nbytes self.sock.sendall(chunk_size.encode('utf-8')) - sockno = self.sock.fileno() - sent = 0 - - while sent != nbytes: - count = min(nbytes - sent, BLKSIZE) - sent += os.sendfile(sockno, fileno, offset + sent, count) + self.sock.sendfile(respiter.filelike, count=nbytes) if self.is_chunked(): self.sock.sendall(b"\r\n") diff --git a/gunicorn/workers/geventlet.py b/gunicorn/workers/geventlet.py index e4b425cd..ffdb206c 100644 --- a/gunicorn/workers/geventlet.py +++ b/gunicorn/workers/geventlet.py @@ -4,8 +4,6 @@ # See the NOTICE for more information. from functools import partial -import errno -import os import sys try: @@ -19,22 +17,49 @@ else: from eventlet import hubs, greenthread from eventlet.greenio import GreenSocket -from eventlet.hubs import trampoline from eventlet.wsgi import ALREADY_HANDLED as EVENTLET_ALREADY_HANDLED import greenlet from gunicorn.workers.base_async import AsyncWorker -def _eventlet_sendfile(fdout, fdin, offset, nbytes, _os_sendfile=os.sendfile): - while True: - try: - return _os_sendfile(fdout, fdin, offset, nbytes) - except OSError as e: - if e.args[0] == errno.EAGAIN: - trampoline(fdout, write=True) - else: - raise +def _eventlet_socket_sendfile(self, file, offset=0, count=None): + # Based on the implementation in gevent which in turn is slightly + # modified from the standard library implementation. + if self.gettimeout() == 0: + raise ValueError("non-blocking sockets are not supported") + if offset: + file.seek(offset) + blocksize = min(count, 8192) if count else 8192 + total_sent = 0 + # localize variable access to minimize overhead + file_read = file.read + sock_send = self.send + try: + while True: + if count: + blocksize = min(count - total_sent, blocksize) + if blocksize <= 0: + break + data = memoryview(file_read(blocksize)) + if not data: + break # EOF + while True: + try: + sent = sock_send(data) + except BlockingIOError: + continue + else: + total_sent += sent + if sent < len(data): + data = data[sent:] + else: + break + return total_sent + finally: + if total_sent > 0 and hasattr(file, 'seek'): + file.seek(offset + total_sent) + def _eventlet_serve(sock, handle, concurrency): @@ -79,7 +104,17 @@ def _eventlet_stop(client, server, conn): def patch_sendfile(): - setattr(os, "sendfile", _eventlet_sendfile) + # As of eventlet 0.25.1, GreenSocket.sendfile doesn't exist, + # meaning the native implementations of socket.sendfile will be used. + # If os.sendfile exists, it will attempt to use that, failing explicitly + # if the socket is in non-blocking mode, which the underlying + # socket object /is/. Even the regular _sendfile_use_send will + # fail in that way; plus, it would use the underlying socket.send which isn't + # properly cooperative. So we have to monkey-patch a working socket.sendfile() + # into GreenSocket; in this method, `self.send` will be the GreenSocket's + # send method which is properly cooperative. + if not hasattr(GreenSocket, 'sendfile'): + GreenSocket.sendfile = _eventlet_socket_sendfile class EventletWorker(AsyncWorker): diff --git a/gunicorn/workers/ggevent.py b/gunicorn/workers/ggevent.py index 1b964e58..57340221 100644 --- a/gunicorn/workers/ggevent.py +++ b/gunicorn/workers/ggevent.py @@ -3,7 +3,6 @@ # This file is part of gunicorn released under the MIT license. # See the NOTICE for more information. -import errno import os import sys from datetime import datetime @@ -30,21 +29,6 @@ from gunicorn.workers.base_async import AsyncWorker VERSION = "gevent/%s gunicorn/%s" % (gevent.__version__, gunicorn.__version__) -def _gevent_sendfile(fdout, fdin, offset, nbytes, _os_sendfile=os.sendfile): - while True: - try: - return _os_sendfile(fdout, fdin, offset, nbytes) - except OSError as e: - if e.args[0] == errno.EAGAIN: - socket.wait_write(fdout) - else: - raise - - -def patch_sendfile(): - setattr(os, "sendfile", _gevent_sendfile) - - class GeventWorker(AsyncWorker): server_class = None @@ -53,9 +37,6 @@ class GeventWorker(AsyncWorker): def patch(self): monkey.patch_all() - # monkey patch sendfile to make it none blocking - patch_sendfile() - # patch sockets sockets = [] for s in self.sockets: diff --git a/requirements_test.txt b/requirements_test.txt index cc595b77..03af4969 100644 --- a/requirements_test.txt +++ b/requirements_test.txt @@ -1,4 +1,6 @@ aiohttp +gevent +eventlet coverage pytest pytest-cov diff --git a/setup.py b/setup.py index 11079f86..9b76ddd3 100644 --- a/setup.py +++ b/setup.py @@ -76,8 +76,8 @@ install_requires = [ ] extras_require = { - 'gevent': ['gevent>=0.13'], - 'eventlet': ['eventlet>=0.9.7'], + 'gevent': ['gevent>=1.4.0'], + 'eventlet': ['eventlet>=0.24.1'], 'tornado': ['tornado>=0.2'], 'gthread': [], 'setproctitle': ['setproctitle'], diff --git a/tests/workers/__init__.py b/tests/workers/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/tests/workers/test_geventlet.py b/tests/workers/test_geventlet.py new file mode 100644 index 00000000..815dcec3 --- /dev/null +++ b/tests/workers/test_geventlet.py @@ -0,0 +1,7 @@ +# -*- coding: utf-8 - +# +# This file is part of gunicorn released under the MIT license. +# See the NOTICE for more information. + +def test_import(): + __import__('gunicorn.workers.geventlet') diff --git a/tests/workers/test_ggevent.py b/tests/workers/test_ggevent.py new file mode 100644 index 00000000..261ce40d --- /dev/null +++ b/tests/workers/test_ggevent.py @@ -0,0 +1,7 @@ +# -*- coding: utf-8 - +# +# This file is part of gunicorn released under the MIT license. +# See the NOTICE for more information. + +def test_import(): + __import__('gunicorn.workers.ggevent') diff --git a/tox.ini b/tox.ini index 26e314ab..db4f00f2 100644 --- a/tox.ini +++ b/tox.ini @@ -4,7 +4,7 @@ skipsdist = True [testenv] usedevelop = True -commands = py.test {posargs} +commands = py.test --cov=gunicorn {posargs} deps = -rrequirements_test.txt