From 40097d74f6b28a17b429373c95d049aef499293b Mon Sep 17 00:00:00 2001 From: Andrew Svetlov Date: Tue, 6 Nov 2018 14:07:29 +0200 Subject: [PATCH 01/18] Refactor sendfile --- aiohttp/web_fileresponse.py | 61 +++++++++++++++++++-------------- tests/test_web_sendfile.py | 68 +------------------------------------ 2 files changed, 36 insertions(+), 93 deletions(-) diff --git a/aiohttp/web_fileresponse.py b/aiohttp/web_fileresponse.py index ccae5dac710..850ee09420c 100644 --- a/aiohttp/web_fileresponse.py +++ b/aiohttp/web_fileresponse.py @@ -2,6 +2,7 @@ import mimetypes import os import pathlib +from functools import partial from typing import (IO, TYPE_CHECKING, Any, Awaitable, Callable, List, # noqa Optional, Union, cast) @@ -35,9 +36,15 @@ class SendfileStreamWriter(StreamWriter): def __init__(self, protocol: BaseProtocol, loop: asyncio.AbstractEventLoop, + fobj: IO[Any], + count: int, on_chunk_sent: _T_OnChunkSent=None) -> None: super().__init__(protocol, loop, on_chunk_sent) self._sendfile_buffer = [] # type: List[bytes] + self._fobj = fobj + self._count = count + self._offset = fobj.tell() + self._in_fd = fobj.fileno() def _write(self, chunk: bytes) -> None: # we overwrite StreamWriter._write, so nothing can be appended to @@ -46,54 +53,54 @@ def _write(self, chunk: bytes) -> None: self.output_size += len(chunk) self._sendfile_buffer.append(chunk) - def _sendfile_cb(self, fut: 'asyncio.Future[None]', - out_fd: int, in_fd: int, - offset: int, count: int, - loop: asyncio.AbstractEventLoop, - registered: bool) -> None: - if registered: - loop.remove_writer(out_fd) + def _sendfile_cb(self, fut: 'asyncio.Future[None]', out_fd: int) -> None: if fut.cancelled(): return + try: + if self._do_sendfile(out_fd): + set_result(fut, None) + except Exception as exc: + set_exception(fut, exc) + def _do_sendfile(self, out_fd: int) -> bool: try: - n = os.sendfile(out_fd, in_fd, offset, count) + n = os.sendfile(out_fd, + self._in_fd, + self._offset, + self._count) if n == 0: # EOF reached - n = count + n = self._count except (BlockingIOError, InterruptedError): n = 0 - except Exception as exc: - set_exception(fut, exc) - return + self.output_size += n + self._offset += n + self._count -= n + return self._count <= 0 - if n < count: - loop.add_writer(out_fd, self._sendfile_cb, fut, out_fd, in_fd, - offset + n, count - n, loop, True) - else: - set_result(fut, None) + def _done_fut(self, fut: 'asyncio.Future[None]', out_fd: int) -> None: + self.loop.remove_writer(out_fd) - async def sendfile(self, fobj: IO[Any], count: int) -> None: + async def sendfile(self) -> None: assert self.transport is not None out_socket = self.transport.get_extra_info('socket').dup() out_socket.setblocking(False) out_fd = out_socket.fileno() - in_fd = fobj.fileno() - offset = fobj.tell() loop = self.loop data = b''.join(self._sendfile_buffer) try: await loop.sock_sendall(out_socket, data) - fut = loop.create_future() - self._sendfile_cb(fut, out_fd, in_fd, offset, count, loop, False) - await fut + if not self._do_sendfile(out_fd): + fut = loop.create_future() + fut.add_done_callback(partial(self._done_fut, out_fd)) + loop.add_writer(out_fd, self._sendfile_cb, fut, out_fd) + await fut except Exception: server_logger.debug('Socket error') self.transport.close() finally: out_socket.close() - self.output_size += count await super().write_eof() async def write_eof(self, chunk: bytes=b'') -> None: @@ -139,12 +146,14 @@ async def _sendfile_system(self, request: 'BaseRequest', else: writer = SendfileStreamWriter( request.protocol, - request._loop + request._loop, + fobj, + count ) request._payload_writer = writer await super().prepare(request) - await writer.sendfile(fobj, count) + await writer.sendfile() return writer diff --git a/tests/test_web_sendfile.py b/tests/test_web_sendfile.py index 241e7db74df..f0849f15c8a 100644 --- a/tests/test_web_sendfile.py +++ b/tests/test_web_sendfile.py @@ -2,73 +2,7 @@ from aiohttp import hdrs from aiohttp.test_utils import make_mocked_coro, make_mocked_request -from aiohttp.web_fileresponse import FileResponse, SendfileStreamWriter - - -def test_static_handle_eof(loop) -> None: - fake_loop = mock.Mock() - with mock.patch('aiohttp.web_fileresponse.os') as m_os: - out_fd = 30 - in_fd = 31 - fut = loop.create_future() - m_os.sendfile.return_value = 0 - writer = SendfileStreamWriter(mock.Mock(), mock.Mock(), fake_loop) - writer._sendfile_cb(fut, out_fd, in_fd, 0, 100, fake_loop, False) - m_os.sendfile.assert_called_with(out_fd, in_fd, 0, 100) - assert fut.done() - assert fut.result() is None - assert not fake_loop.add_writer.called - assert not fake_loop.remove_writer.called - - -def test_static_handle_again(loop) -> None: - fake_loop = mock.Mock() - with mock.patch('aiohttp.web_fileresponse.os') as m_os: - out_fd = 30 - in_fd = 31 - fut = loop.create_future() - m_os.sendfile.side_effect = BlockingIOError() - writer = SendfileStreamWriter(mock.Mock(), mock.Mock(), fake_loop) - writer._sendfile_cb(fut, out_fd, in_fd, 0, 100, fake_loop, False) - m_os.sendfile.assert_called_with(out_fd, in_fd, 0, 100) - assert not fut.done() - fake_loop.add_writer.assert_called_with(out_fd, - writer._sendfile_cb, - fut, out_fd, in_fd, 0, 100, - fake_loop, True) - assert not fake_loop.remove_writer.called - - -def test_static_handle_exception(loop) -> None: - fake_loop = mock.Mock() - with mock.patch('aiohttp.web_fileresponse.os') as m_os: - out_fd = 30 - in_fd = 31 - fut = loop.create_future() - exc = OSError() - m_os.sendfile.side_effect = exc - writer = SendfileStreamWriter(mock.Mock(), mock.Mock(), fake_loop) - writer._sendfile_cb(fut, out_fd, in_fd, 0, 100, fake_loop, False) - m_os.sendfile.assert_called_with(out_fd, in_fd, 0, 100) - assert fut.done() - assert exc is fut.exception() - assert not fake_loop.add_writer.called - assert not fake_loop.remove_writer.called - - -def test__sendfile_cb_return_on_cancelling(loop) -> None: - fake_loop = mock.Mock() - with mock.patch('aiohttp.web_fileresponse.os') as m_os: - out_fd = 30 - in_fd = 31 - fut = loop.create_future() - fut.cancel() - writer = SendfileStreamWriter(mock.Mock(), mock.Mock(), fake_loop) - writer._sendfile_cb(fut, out_fd, in_fd, 0, 100, fake_loop, False) - assert fut.done() - assert not fake_loop.add_writer.called - assert not fake_loop.remove_writer.called - assert not m_os.sendfile.called +from aiohttp.web_fileresponse import FileResponse def test_using_gzip_if_header_present_and_file_available(loop) -> None: From 7dd5a86d9d72c7eeac0c2868adbdc67b17ceb68a Mon Sep 17 00:00:00 2001 From: Andrew Svetlov Date: Tue, 6 Nov 2018 16:01:21 +0200 Subject: [PATCH 02/18] Fix notes --- aiohttp/web_fileresponse.py | 4 +++- tests/test_web_sendfile_functional.py | 16 ++++++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/aiohttp/web_fileresponse.py b/aiohttp/web_fileresponse.py index 850ee09420c..715d334a305 100644 --- a/aiohttp/web_fileresponse.py +++ b/aiohttp/web_fileresponse.py @@ -67,7 +67,7 @@ def _do_sendfile(self, out_fd: int) -> bool: n = os.sendfile(out_fd, self._in_fd, self._offset, - self._count) + min(self._count, 0xffffffff)) if n == 0: # EOF reached n = self._count except (BlockingIOError, InterruptedError): @@ -95,6 +95,8 @@ async def sendfile(self) -> None: fut.add_done_callback(partial(self._done_fut, out_fd)) loop.add_writer(out_fd, self._sendfile_cb, fut, out_fd) await fut + except asyncio.CancelledError: + raise except Exception: server_logger.debug('Socket error') self.transport.close() diff --git a/tests/test_web_sendfile_functional.py b/tests/test_web_sendfile_functional.py index 410878e9de9..aacaed5618b 100644 --- a/tests/test_web_sendfile_functional.py +++ b/tests/test_web_sendfile_functional.py @@ -751,3 +751,19 @@ async def handler(request): assert 'application/octet-stream' == resp.headers['Content-Type'] assert resp.headers.get('Content-Encoding') == 'deflate' await resp.release() + + +async def test_static_file_cancel(aiohttp_client) -> None: + filepath = pathlib.Path(__file__).parent / 'data.unknown_mime_type' + + async def handler(request): + ret = web.FileResponse(filepath) + request.task.cancel() + return ret + + app = web.Application() + app.router.add_get('/', handler) + client = await aiohttp_client(app) + + resp = await client.get('/') + assert resp.status == 200 From 816fdffc872a63fade98c3ac3a957ccce42ccdd6 Mon Sep 17 00:00:00 2001 From: Andrew Svetlov Date: Tue, 6 Nov 2018 16:23:10 +0200 Subject: [PATCH 03/18] More tests --- tests/test_web_sendfile_functional.py | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/tests/test_web_sendfile_functional.py b/tests/test_web_sendfile_functional.py index aacaed5618b..560527b81b4 100644 --- a/tests/test_web_sendfile_functional.py +++ b/tests/test_web_sendfile_functional.py @@ -766,4 +766,27 @@ async def handler(request): client = await aiohttp_client(app) resp = await client.get('/') - assert resp.status == 200 + assert resp.status == 200 # cancelled after sending response headers + + +async def test_static_file_huge_cancel(aiohttp_client, tmpdir) -> None: + filename = 'huge_data.unknown_mime_type' + + # fill 100MB file + with tmpdir.join(filename).open('w') as f: + for i in range(1024*20): + f.write(chr(i % 64 + 0x20) * 1024) + + async def handler(request): + ret = web.FileResponse(pathlib.Path(tmpdir.join(filename))) + loop = asyncio.get_event_loop() + loop.call_later(0.01, request.task.cancel) + return ret + + app = web.Application() + + app.router.add_get('/', handler) + client = await aiohttp_client(app) + + resp = await client.get('/') + assert resp.status == 200 # cancelled after sending response headers From 028a15e6db92a6c756854e7f455adfcb8f42aa11 Mon Sep 17 00:00:00 2001 From: Andrew Svetlov Date: Tue, 6 Nov 2018 16:46:54 +0200 Subject: [PATCH 04/18] Address comments --- aiohttp/web_fileresponse.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/aiohttp/web_fileresponse.py b/aiohttp/web_fileresponse.py index 715d334a305..41e4133b54a 100644 --- a/aiohttp/web_fileresponse.py +++ b/aiohttp/web_fileresponse.py @@ -68,14 +68,15 @@ def _do_sendfile(self, out_fd: int) -> bool: self._in_fd, self._offset, min(self._count, 0xffffffff)) - if n == 0: # EOF reached + if n == 0: # in_fd EOF reached n = self._count except (BlockingIOError, InterruptedError): n = 0 self.output_size += n self._offset += n self._count -= n - return self._count <= 0 + assert self._count >= 0 + return self._count == 0 def _done_fut(self, fut: 'asyncio.Future[None]', out_fd: int) -> None: self.loop.remove_writer(out_fd) From 8dcc6a946cfb9e201e29ca9a031fd9b37e39d7c8 Mon Sep 17 00:00:00 2001 From: Andrew Svetlov Date: Tue, 6 Nov 2018 16:51:58 +0200 Subject: [PATCH 05/18] Fix constant --- aiohttp/web_fileresponse.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/aiohttp/web_fileresponse.py b/aiohttp/web_fileresponse.py index 41e4133b54a..d7236dedcd8 100644 --- a/aiohttp/web_fileresponse.py +++ b/aiohttp/web_fileresponse.py @@ -67,7 +67,7 @@ def _do_sendfile(self, out_fd: int) -> bool: n = os.sendfile(out_fd, self._in_fd, self._offset, - min(self._count, 0xffffffff)) + min(self._count, 0x7fffffff)) if n == 0: # in_fd EOF reached n = self._count except (BlockingIOError, InterruptedError): From 9be2ccc9b842a2751bf8ae5f25623af0e591f961 Mon Sep 17 00:00:00 2001 From: Andrew Svetlov Date: Tue, 6 Nov 2018 17:10:20 +0200 Subject: [PATCH 06/18] Fix max sendifle size again --- aiohttp/web_fileresponse.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/aiohttp/web_fileresponse.py b/aiohttp/web_fileresponse.py index d7236dedcd8..b3051e60a9a 100644 --- a/aiohttp/web_fileresponse.py +++ b/aiohttp/web_fileresponse.py @@ -67,7 +67,7 @@ def _do_sendfile(self, out_fd: int) -> bool: n = os.sendfile(out_fd, self._in_fd, self._offset, - min(self._count, 0x7fffffff)) + min(self._count, 0x7ffff000)) if n == 0: # in_fd EOF reached n = self._count except (BlockingIOError, InterruptedError): From e1c029ece4fa24659c4ae5254c74a565f811fa7f Mon Sep 17 00:00:00 2001 From: Andrew Svetlov Date: Tue, 6 Nov 2018 20:03:37 +0200 Subject: [PATCH 07/18] Improve test --- tests/test_web_sendfile_functional.py | 31 +++++++++++---------------- 1 file changed, 12 insertions(+), 19 deletions(-) diff --git a/tests/test_web_sendfile_functional.py b/tests/test_web_sendfile_functional.py index 560527b81b4..90ef343074f 100644 --- a/tests/test_web_sendfile_functional.py +++ b/tests/test_web_sendfile_functional.py @@ -753,22 +753,6 @@ async def handler(request): await resp.release() -async def test_static_file_cancel(aiohttp_client) -> None: - filepath = pathlib.Path(__file__).parent / 'data.unknown_mime_type' - - async def handler(request): - ret = web.FileResponse(filepath) - request.task.cancel() - return ret - - app = web.Application() - app.router.add_get('/', handler) - client = await aiohttp_client(app) - - resp = await client.get('/') - assert resp.status == 200 # cancelled after sending response headers - - async def test_static_file_huge_cancel(aiohttp_client, tmpdir) -> None: filename = 'huge_data.unknown_mime_type' @@ -777,10 +761,12 @@ async def test_static_file_huge_cancel(aiohttp_client, tmpdir) -> None: for i in range(1024*20): f.write(chr(i % 64 + 0x20) * 1024) + task = None + async def handler(request): + nonlocal task + task = request.task ret = web.FileResponse(pathlib.Path(tmpdir.join(filename))) - loop = asyncio.get_event_loop() - loop.call_later(0.01, request.task.cancel) return ret app = web.Application() @@ -789,4 +775,11 @@ async def handler(request): client = await aiohttp_client(app) resp = await client.get('/') - assert resp.status == 200 # cancelled after sending response headers + task.cancel() + data = b'' + while True: + try: + data += await resp.read() + except aiohttp.ClientPayloadError: + break + assert len(data) < 1024 * 1024 * 20 From 457c65ce51dd3e151e78ed2f7b1b3979b91d4096 Mon Sep 17 00:00:00 2001 From: Andrew Svetlov Date: Tue, 6 Nov 2018 20:46:07 +0200 Subject: [PATCH 08/18] Try to fix test --- tests/test_web_sendfile_functional.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_web_sendfile_functional.py b/tests/test_web_sendfile_functional.py index 90ef343074f..84e448b59dc 100644 --- a/tests/test_web_sendfile_functional.py +++ b/tests/test_web_sendfile_functional.py @@ -758,7 +758,7 @@ async def test_static_file_huge_cancel(aiohttp_client, tmpdir) -> None: # fill 100MB file with tmpdir.join(filename).open('w') as f: - for i in range(1024*20): + for i in range(1024*100): f.write(chr(i % 64 + 0x20) * 1024) task = None From 37da0ad98790c5dbc6ef3f24d52b4596106a50ff Mon Sep 17 00:00:00 2001 From: Andrew Svetlov Date: Tue, 6 Nov 2018 22:20:28 +0200 Subject: [PATCH 09/18] Reduce write buffer size --- tests/test_web_sendfile_functional.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tests/test_web_sendfile_functional.py b/tests/test_web_sendfile_functional.py index 84e448b59dc..eb12721d7d2 100644 --- a/tests/test_web_sendfile_functional.py +++ b/tests/test_web_sendfile_functional.py @@ -1,6 +1,7 @@ import asyncio import os import pathlib +import socket import zlib import pytest @@ -758,7 +759,7 @@ async def test_static_file_huge_cancel(aiohttp_client, tmpdir) -> None: # fill 100MB file with tmpdir.join(filename).open('w') as f: - for i in range(1024*100): + for i in range(1024*20): f.write(chr(i % 64 + 0x20) * 1024) task = None @@ -766,6 +767,10 @@ async def test_static_file_huge_cancel(aiohttp_client, tmpdir) -> None: async def handler(request): nonlocal task task = request.task + # reduce send buffer size + tr = request.transport + sock = tr.get_extra_info('socket') + sock.setsockopt(socket.SOL_SOCKET, socket.SO_SNDBUF, 1024) ret = web.FileResponse(pathlib.Path(tmpdir.join(filename))) return ret From fa3d2016cc1e557049df4ce20f5a67222c4ac06c Mon Sep 17 00:00:00 2001 From: Andrew Svetlov Date: Tue, 6 Nov 2018 23:16:41 +0200 Subject: [PATCH 10/18] Increate sendfile test data file --- tests/test_web_sendfile_functional.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_web_sendfile_functional.py b/tests/test_web_sendfile_functional.py index eb12721d7d2..ccfcb6cd8f7 100644 --- a/tests/test_web_sendfile_functional.py +++ b/tests/test_web_sendfile_functional.py @@ -759,7 +759,7 @@ async def test_static_file_huge_cancel(aiohttp_client, tmpdir) -> None: # fill 100MB file with tmpdir.join(filename).open('w') as f: - for i in range(1024*20): + for i in range(1024*200): f.write(chr(i % 64 + 0x20) * 1024) task = None From 71873eb354278a08834e712e939b001a78ddf2f4 Mon Sep 17 00:00:00 2001 From: Andrew Svetlov Date: Tue, 6 Nov 2018 23:18:03 +0200 Subject: [PATCH 11/18] Add changenote --- CHANGES/3383.bugfix | 1 + 1 file changed, 1 insertion(+) create mode 100644 CHANGES/3383.bugfix diff --git a/CHANGES/3383.bugfix b/CHANGES/3383.bugfix new file mode 100644 index 00000000000..98bfe3d8c60 --- /dev/null +++ b/CHANGES/3383.bugfix @@ -0,0 +1 @@ +Fix task cancellation when ``sendfile()`` syscall is used by static file handling. From 7f52eb0ea20b34b968e89f9650fc6449ca45c4fe Mon Sep 17 00:00:00 2001 From: Andrew Svetlov Date: Wed, 7 Nov 2018 00:34:45 +0200 Subject: [PATCH 12/18] Drop redundant code --- aiohttp/web_fileresponse.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/aiohttp/web_fileresponse.py b/aiohttp/web_fileresponse.py index b3051e60a9a..71db2990fa3 100644 --- a/aiohttp/web_fileresponse.py +++ b/aiohttp/web_fileresponse.py @@ -67,7 +67,7 @@ def _do_sendfile(self, out_fd: int) -> bool: n = os.sendfile(out_fd, self._in_fd, self._offset, - min(self._count, 0x7ffff000)) + self._count) if n == 0: # in_fd EOF reached n = self._count except (BlockingIOError, InterruptedError): From 1a5b7892d3e45a0b73618cef481c91c7fcddd4dc Mon Sep 17 00:00:00 2001 From: Andrew Svetlov Date: Wed, 7 Nov 2018 00:42:07 +0200 Subject: [PATCH 13/18] Try to fix a test again --- tests/test_web_sendfile_functional.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/test_web_sendfile_functional.py b/tests/test_web_sendfile_functional.py index ccfcb6cd8f7..2672081608f 100644 --- a/tests/test_web_sendfile_functional.py +++ b/tests/test_web_sendfile_functional.py @@ -759,7 +759,7 @@ async def test_static_file_huge_cancel(aiohttp_client, tmpdir) -> None: # fill 100MB file with tmpdir.join(filename).open('w') as f: - for i in range(1024*200): + for i in range(1024*20): f.write(chr(i % 64 + 0x20) * 1024) task = None @@ -781,6 +781,7 @@ async def handler(request): resp = await client.get('/') task.cancel() + await asyncio.sleep(0) data = b'' while True: try: From 4f64f524880bd079cb777dc461f8b5a04d16ac90 Mon Sep 17 00:00:00 2001 From: Andrew Svetlov Date: Wed, 7 Nov 2018 10:50:31 +0200 Subject: [PATCH 14/18] Fix test --- tests/test_web_sendfile_functional.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/test_web_sendfile_functional.py b/tests/test_web_sendfile_functional.py index 2672081608f..7492a48550d 100644 --- a/tests/test_web_sendfile_functional.py +++ b/tests/test_web_sendfile_functional.py @@ -771,7 +771,7 @@ async def handler(request): tr = request.transport sock = tr.get_extra_info('socket') sock.setsockopt(socket.SOL_SOCKET, socket.SO_SNDBUF, 1024) - ret = web.FileResponse(pathlib.Path(tmpdir.join(filename))) + ret = web.FileResponse(pathlib.Path(str(tmpdir.join(filename)))) return ret app = web.Application() @@ -780,12 +780,13 @@ async def handler(request): client = await aiohttp_client(app) resp = await client.get('/') + assert resp.status == 200 task.cancel() await asyncio.sleep(0) data = b'' while True: try: - data += await resp.read() + data += await resp.content.read(1024) except aiohttp.ClientPayloadError: break assert len(data) < 1024 * 1024 * 20 From 644a7fed0db7c29308fb6d312f6d2fd785874eed Mon Sep 17 00:00:00 2001 From: Andrew Svetlov Date: Wed, 7 Nov 2018 12:33:21 +0200 Subject: [PATCH 15/18] Fix functools.partial call --- aiohttp/web_fileresponse.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/aiohttp/web_fileresponse.py b/aiohttp/web_fileresponse.py index 71db2990fa3..adebc3e9dd3 100644 --- a/aiohttp/web_fileresponse.py +++ b/aiohttp/web_fileresponse.py @@ -78,7 +78,7 @@ def _do_sendfile(self, out_fd: int) -> bool: assert self._count >= 0 return self._count == 0 - def _done_fut(self, fut: 'asyncio.Future[None]', out_fd: int) -> None: + def _done_fut(self, out_fd: int, fut: 'asyncio.Future[None]') -> None: self.loop.remove_writer(out_fd) async def sendfile(self) -> None: @@ -93,7 +93,7 @@ async def sendfile(self) -> None: await loop.sock_sendall(out_socket, data) if not self._do_sendfile(out_fd): fut = loop.create_future() - fut.add_done_callback(partial(self._done_fut, out_fd)) + fut.add_done_callback(partial(out_fd)) loop.add_writer(out_fd, self._sendfile_cb, fut, out_fd) await fut except asyncio.CancelledError: From f09f5c4590a6b7b11742b4f1cee2f52031492d91 Mon Sep 17 00:00:00 2001 From: Andrew Svetlov Date: Wed, 7 Nov 2018 13:48:51 +0200 Subject: [PATCH 16/18] Remove bad change back --- aiohttp/web_fileresponse.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/aiohttp/web_fileresponse.py b/aiohttp/web_fileresponse.py index adebc3e9dd3..be4dbc73bce 100644 --- a/aiohttp/web_fileresponse.py +++ b/aiohttp/web_fileresponse.py @@ -93,7 +93,7 @@ async def sendfile(self) -> None: await loop.sock_sendall(out_socket, data) if not self._do_sendfile(out_fd): fut = loop.create_future() - fut.add_done_callback(partial(out_fd)) + fut.add_done_callback(partial(self._done_fut, out_fd)) loop.add_writer(out_fd, self._sendfile_cb, fut, out_fd) await fut except asyncio.CancelledError: From 602b1d089cf839d93b9215c49c2cbcdd70659fd8 Mon Sep 17 00:00:00 2001 From: Andrew Svetlov Date: Wed, 7 Nov 2018 14:37:55 +0200 Subject: [PATCH 17/18] More tests --- tests/test_web_sendfile_functional.py | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/tests/test_web_sendfile_functional.py b/tests/test_web_sendfile_functional.py index 7492a48550d..4265d10b3d6 100644 --- a/tests/test_web_sendfile_functional.py +++ b/tests/test_web_sendfile_functional.py @@ -790,3 +790,30 @@ async def handler(request): except aiohttp.ClientPayloadError: break assert len(data) < 1024 * 1024 * 20 + + +async def test_static_file_huge_error(aiohttp_client, tmpdir) -> None: + filename = 'huge_data.unknown_mime_type' + + # fill 100MB file + with tmpdir.join(filename).open('w') as f: + for i in range(1024*20): + f.write(chr(i % 64 + 0x20) * 1024) + + async def handler(request): + # reduce send buffer size + tr = request.transport + sock = tr.get_extra_info('socket') + sock.setsockopt(socket.SOL_SOCKET, socket.SO_SNDBUF, 1024) + ret = web.FileResponse(pathlib.Path(str(tmpdir.join(filename)))) + return ret + + app = web.Application() + + app.router.add_get('/', handler) + client = await aiohttp_client(app) + + resp = await client.get('/') + assert resp.status == 200 + # raise an exception on server side + resp.close() From 7c4c57e047992ac2cba531de005f5804918ad43d Mon Sep 17 00:00:00 2001 From: Andrew Svetlov Date: Wed, 7 Nov 2018 20:03:50 +0200 Subject: [PATCH 18/18] Speed up test --- tests/test_web_sendfile_functional.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/test_web_sendfile_functional.py b/tests/test_web_sendfile_functional.py index 4265d10b3d6..3d71de99129 100644 --- a/tests/test_web_sendfile_functional.py +++ b/tests/test_web_sendfile_functional.py @@ -325,7 +325,7 @@ def test_static_route_path_existence_check() -> None: async def test_static_file_huge(aiohttp_client, tmpdir) -> None: filename = 'huge_data.unknown_mime_type' - # fill 100MB file + # fill 20MB file with tmpdir.join(filename).open('w') as f: for i in range(1024*20): f.write(chr(i % 64 + 0x20) * 1024) @@ -795,10 +795,10 @@ async def handler(request): async def test_static_file_huge_error(aiohttp_client, tmpdir) -> None: filename = 'huge_data.unknown_mime_type' - # fill 100MB file - with tmpdir.join(filename).open('w') as f: - for i in range(1024*20): - f.write(chr(i % 64 + 0x20) * 1024) + # fill 20MB file + with tmpdir.join(filename).open('wb') as f: + f.seek(20*1024*1024) + f.write(b'1') async def handler(request): # reduce send buffer size