diff --git a/cheroot/_compat.py b/cheroot/_compat.py index 63df078119..3ebbf2a81d 100644 --- a/cheroot/_compat.py +++ b/cheroot/_compat.py @@ -1,3 +1,4 @@ +# pylint: disable=unused-import """Compatibility code for using Cheroot with various versions of Python.""" from __future__ import absolute_import, division, print_function @@ -8,6 +9,11 @@ import six +try: + import selectors # lgtm [py/unused-import] +except ImportError: + import selectors2 as selectors # noqa: F401 # lgtm [py/unused-import] + try: import ssl IS_ABOVE_OPENSSL10 = ssl.OPENSSL_VERSION_INFO >= (1, 1) diff --git a/cheroot/connections.py b/cheroot/connections.py index 28bc973b61..57438386e9 100644 --- a/cheroot/connections.py +++ b/cheroot/connections.py @@ -5,11 +5,11 @@ import io import os -import select import socket import time from . import errors +from ._compat import selectors from .makefile import MakeFile import six @@ -62,6 +62,7 @@ def __init__(self, server): """ self.server = server self.connections = [] + self._selector = selectors.DefaultSelector() def put(self, conn): """Put idle connection into the ConnectionManager to be managed. @@ -133,10 +134,12 @@ def get_conn(self, server_socket): ss_fileno = server_socket.fileno() socket_dict[ss_fileno] = server_socket try: - rlist, _, _ = select.select(list(socket_dict), [], [], 0.1) - # No available socket. - if not rlist: - return None + for fno in socket_dict: + self._selector.register(fno, selectors.EVENT_READ) + rlist = [ + key.fd for key, _event + in self._selector.select(timeout=0.1) + ] except OSError: # Mark any connection which no longer appears valid. for fno, conn in list(socket_dict.items()): @@ -155,14 +158,22 @@ def get_conn(self, server_socket): # Wait for the next tick to occur. return None + finally: + for fno in socket_dict: + self._selector.unregister(fno) try: # See if we have a new connection coming in. rlist.remove(ss_fileno) except ValueError: - # No new connection, but reuse existing socket. + # If we didn't get any readable sockets, wait for the next tick + if not rlist: + return None + + # No new connection, but reuse an existing socket. conn = socket_dict[rlist.pop()] else: + # If we have a new connection, reuse the server socket conn = server_socket # All remaining connections in rlist should be marked as ready. diff --git a/cheroot/test/test_server.py b/cheroot/test/test_server.py index 2108e2bb9a..7b0ef9137b 100644 --- a/cheroot/test/test_server.py +++ b/cheroot/test/test_server.py @@ -5,6 +5,7 @@ from __future__ import absolute_import, division, print_function __metaclass__ = type +from contextlib import closing import os import socket import tempfile @@ -19,7 +20,7 @@ from six.moves import urllib from .._compat import bton, ntob -from .._compat import IS_LINUX, IS_MACOS, SYS_PLATFORM +from .._compat import IS_LINUX, IS_MACOS, IS_WINDOWS, SYS_PLATFORM from ..server import IS_UID_GID_RESOLVABLE, Gateway, HTTPServer from ..testing import ( ANY_INTERFACE_IPV4, @@ -236,3 +237,106 @@ def test_peercreds_unix_sock_with_lookup(peercreds_enabled_server): peercreds_text_resp = requests.get(unix_base_uri + PEERCRED_TEXTS_URI) peercreds_text_resp.raise_for_status() assert peercreds_text_resp.text == expected_textcreds + + +@pytest.mark.skipif( + IS_WINDOWS, + reason='This regression test is for a Linux bug, ' + 'and the resource module is not available on Windows', +) +@pytest.mark.parametrize( + 'resource_limit', + ( + 1024, + 2048, + ), + indirect=('resource_limit',), +) +@pytest.mark.usefixtures('many_open_sockets') +def test_high_number_of_file_descriptors(resource_limit): + """Test the server does not crash with a high file-descriptor value. + + This test shouldn't cause a server crash when trying to access + file-descriptor higher than 1024. + + The earlier implementation used to rely on ``select()`` syscall that + doesn't support file descriptors with numbers higher than 1024. + """ + # We want to force the server to use a file-descriptor with + # a number above resource_limit + + # Create our server + httpserver = HTTPServer( + bind_addr=(ANY_INTERFACE_IPV4, EPHEMERAL_PORT), gateway=Gateway, + ) + httpserver.prepare() + + try: + # This will trigger a crash if select() is used in the implementation + httpserver.tick() + except: # noqa: E722 + raise # only needed for `else` to work + else: + # We use closing here for py2-compat + with closing(socket.socket()) as sock: + # Check new sockets created are still above our target number + assert sock.fileno() >= resource_limit + finally: + # Stop our server + httpserver.stop() + + +if not IS_WINDOWS: + test_high_number_of_file_descriptors = pytest.mark.forked( + test_high_number_of_file_descriptors, + ) + + +@pytest.fixture +def resource_limit(request): + """Set the resource limit two times bigger then requested.""" + resource = pytest.importorskip( + 'resource', + reason='The "resource" module is Unix-specific', + ) + + # Get current resource limits to restore them later + soft_limit, hard_limit = resource.getrlimit(resource.RLIMIT_NOFILE) + + # We have to increase the nofile limit above 1024 + # Otherwise we see a 'Too many files open' error, instead of + # an error due to the file descriptor number being too high + resource.setrlimit( + resource.RLIMIT_NOFILE, + (request.param * 2, hard_limit), + ) + + try: # noqa: WPS501 + yield request.param + finally: + # Reset the resource limit back to the original soft limit + resource.setrlimit(resource.RLIMIT_NOFILE, (soft_limit, hard_limit)) + + +@pytest.fixture +def many_open_sockets(resource_limit): + """Allocate a lot of file descriptors by opening dummy sockets.""" + # Hoard a lot of file descriptors by opening and storing a lot of sockets + test_sockets = [] + # Open a lot of file descriptors, so the next one the server + # opens is a high number + try: + for i in range(resource_limit): + sock = socket.socket() + test_sockets.append(sock) + # If we reach a high enough number, we don't need to open more + if sock.fileno() >= resource_limit: + break + # Check we opened enough descriptors to reach a high number + the_highest_fileno = test_sockets[-1].fileno() + assert the_highest_fileno >= resource_limit + yield the_highest_fileno + finally: + # Close our open resources + for test_socket in test_sockets: + test_socket.close() diff --git a/docs/spelling_wordlist.txt b/docs/spelling_wordlist.txt index bfd2eb8713..9b872b9f3f 100644 --- a/docs/spelling_wordlist.txt +++ b/docs/spelling_wordlist.txt @@ -45,6 +45,7 @@ ssl stdout subclasses submodules +syscall systemd threadpool Tidelift diff --git a/setup.cfg b/setup.cfg index 1c0b9425b8..cf2df16f9a 100644 --- a/setup.cfg +++ b/setup.cfg @@ -65,6 +65,7 @@ setup_requires = # These are required in actual runtime: install_requires = backports.functools_lru_cache; python_version < '3.3' + selectors2; python_version< '3.4' six>=1.11.0 more_itertools>=2.6 jaraco.functools @@ -85,6 +86,8 @@ docs = testing = pytest>=4.6.6 + pytest-forked>=1.1.3; sys_platform != "win32" and python_version >= '3.0' and python_version <= '3.4' + pytest-forked>=1.2.0; sys_platform != "win32" and (python_version < '3.0' or python_version > '3.4') pytest-mock>=1.11.0 pytest-sugar>=0.9.3 pytest-testmon<1.0.0