Skip to content

Commit

Permalink
Use socket.sendfile() instead of os.sendfile().
Browse files Browse the repository at this point in the history
Fixes benoitc#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.
  • Loading branch information
jamadden committed Jan 4, 2020
1 parent 9c1438f commit 2d40e6d
Show file tree
Hide file tree
Showing 10 changed files with 77 additions and 58 deletions.
20 changes: 9 additions & 11 deletions .travis.yml
Original file line number Diff line number Diff line change
@@ -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
Expand Down
13 changes: 1 addition & 12 deletions gunicorn/http/wsgi.py
Original file line number Diff line number Diff line change
Expand Up @@ -360,12 +360,6 @@ def sendfile(self, respiter):
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
Expand All @@ -378,12 +372,7 @@ def sendfile(self, respiter):
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")
Expand Down
61 changes: 48 additions & 13 deletions gunicorn/workers/geventlet.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@
# See the NOTICE for more information.

from functools import partial
import errno
import os
import sys

try:
Expand All @@ -19,22 +17,49 @@

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):
Expand Down Expand Up @@ -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):
Expand Down
19 changes: 0 additions & 19 deletions gunicorn/workers/ggevent.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -30,21 +29,6 @@
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
Expand All @@ -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:
Expand Down
2 changes: 2 additions & 0 deletions requirements_test.txt
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
aiohttp
gevent
eventlet
coverage
pytest
pytest-cov
4 changes: 2 additions & 2 deletions setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,8 @@ def run_tests(self):
]

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'],
Expand Down
Empty file added tests/workers/__init__.py
Empty file.
7 changes: 7 additions & 0 deletions tests/workers/test_geventlet.py
Original file line number Diff line number Diff line change
@@ -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')
7 changes: 7 additions & 0 deletions tests/workers/test_ggevent.py
Original file line number Diff line number Diff line change
@@ -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')
2 changes: 1 addition & 1 deletion tox.ini
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ skipsdist = True

[testenv]
usedevelop = True
commands = py.test {posargs}
commands = py.test --cov=gunicorn {posargs}
deps =
-rrequirements_test.txt

Expand Down

0 comments on commit 2d40e6d

Please sign in to comment.