Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BUG, EASY, PY38: web_protocol.RequestHandler mismatch _keepalive field with __slots__ #3644

Closed
cjrh opened this issue Mar 10, 2019 · 1 comment
Labels

Comments

@cjrh
Copy link

cjrh commented Mar 10, 2019

Long story short

In RequestHandler, the spelling in __slots__ is _keep_alive but in the default assignment in __init__(), the name of the field is _keepalive. The latter is what actually gets used in the module, not the slots name.

In Python 3.7, this appears to be ok because instances are getting a __dict__ from somewhere (presumably from an ancestor in the MRO chain). However, in Python 3.8 it seems that instances of RequestHandler are not getting a __dict__ attached to them, which is why an AttributeError gets raised.

The slots declaration in aiohttp/web_protocol.py:

class RequestHandler(BaseProtocol):
    KEEPALIVE_RESCHEDULE_DELAY = 1

    __slots__ = ('_request_count', '_keep_alive', '_manager',
                 '_request_handler', '_request_factory', '_tcp_keepalive',
                 '_keepalive_time', '_keepalive_handle', '_keepalive_timeout',
                 '_lingering_time', '_messages', '_message_tail',
                 '_waiter', '_error_handler', '_task_handler',
                 '_upgrade', '_payload_parser', '_request_parser',
                 '_reading_paused', 'logger', 'debug', 'access_log',
                 'access_logger', '_close', '_force_close')

    def __init__(self, manager: 'Server', *,
                 loop: asyncio.AbstractEventLoop,
                 keepalive_timeout: float=75.,  # NGINX default is 75 secs
                 tcp_keepalive: bool=True,
                 logger: Logger=server_logger,
                 access_log_class: Type[AbstractAccessLogger]=AccessLogger,
                 access_log: Logger=access_logger,
                 access_log_format: str=AccessLogger.LOG_FORMAT,
                 debug: bool=False,
                 max_line_size: int=8190,
                 max_headers: int=32768,
                 max_field_size: int=8190,
                 lingering_time: float=10.0):

        super().__init__(loop)

        self._request_count = 0

        self._keepalive = False      # <----- NAME DIFFERS TO _keep_alive IN SLOTS

        self._manager = manager  # type: Optional[Server]
        self._request_handler = manager.request_handler  # type: Optional[_RequestHandler]  # noqa
        self._request_factory = manager.request_factory  # type: Optional[_RequestFactory]  # noqa

This code was added in #3095.

Expected behaviour

In Python 3.8 requests to the aiohttp web server should succeed.

Actual behaviour

In Python 3.8, requests fail with this traceback:

Exception in callback BaseProactorEventLoop._start_serving.<locals>.loop(<_OverlappedF...0.1', 64284))>) at G:\Programs\Python38\lib\asyncio\proactor_events.py:651
handle: <Handle BaseProactorEventLoop._start_serving.<locals>.loop(<_OverlappedF...0.1', 64284))>) at G:\Programs\Python38\lib\asyncio\proactor_events.py:651>
Traceback (most recent call last):
  File "G:\Programs\Python38\lib\asyncio\events.py", line 81, in _run
    self._context.run(self._callback, *self._args)
  File "G:\Programs\Python38\lib\asyncio\proactor_events.py", line 658, in loop
    protocol = protocol_factory()
  File "G:\Documents\repos\blah\venv\lib\site-packages\aiohttp\web_server.py", line 57, in __call__
    return RequestHandler(self, loop=self._loop, **self._kwargs)
  File "G:\Documents\repos\blah\venv\lib\site-packages\aiohttp\web_protocol.py", line 136, in __init__
    self._keepalive = False
AttributeError: 'RequestHandler' object has no attribute '_keepalive'

Immediately at the offending line (in a debugger), the __mro__ for the RequestHandler instance appears to be the same when running under Python 3.7 and Python 3.8:

[
    <class 'tuple'>: (<class 'aiohttp.web_protocol.RequestHandler'>, 
    <class 'aiohttp.base_protocol.BaseProtocol'>, 
    <class 'asyncio.protocols.Protocol'>, 
    <class 'asyncio.protocols.BaseProtocol'>, 
    <class 'object'>)
]

This leads me to suspect that something might have changed with either Protocol or BaseProtocol in upstream asyncio (i.e. declaring slots so as to prevent __dict__ appearing on the instance), but I've not confirmed this.

Regardless, the name mismatch of the field must still be fixed.

Steps to reproduce

Server code with:

import sys
from aiohttp import web
routes = web.RouteTableDef()
FILENAME = 'reveal/reveal.js-3.6.0/index.html'  # Or whatever

@routes.get('/')
async def handle(request):
    return web.FileResponse(FILENAME)

app = web.Application()
app.router.add_static('/js', 'reveal/reveal.js-3.6.0/js')
app.router.add_static('/css', 'reveal/reveal.js-3.6.0/css')
app.router.add_static('/img', 'reveal/reveal.js-3.6.0/img')
app.router.add_static('/lib', 'reveal/reveal.js-3.6.0/lib')
app.router.add_static('/plugin', 'reveal/reveal.js-3.6.0/plugin')
app.router.add_routes(routes)
web.run_app(app)

Then open localhost:8080 in a browser.

Your environment

  • Windows 10 x64
  • Python 3.8.0a2 (downloaded installer from python.org), with requirements.txt:
aiohttp==3.5.4
async-timeout==3.0.1
attrs==19.1.0
chardet==3.0.4
Cython==0.29.6
dominate==2.3.5
idna==2.8
multidict==4.5.2
numpy==1.16.2
yarl==1.3.0
@cjrh cjrh changed the title BUG, EASY: web_protocol.RequestHandler mismatch _keepalive field with __slots__ BUG, EASY, PY38: web_protocol.RequestHandler mismatch _keepalive field with __slots__ Mar 10, 2019
@aio-libs-bot
Copy link

GitMate.io thinks the contributor most likely able to help you is @asvetlov.

Possibly related issues are #2752 (IndexError in RequestHandler), #103 (BasicAuth bugged ?), and #2620 (DNSCache BUG).

@webknjaz webknjaz added the good first issue Good for newcomers label Mar 11, 2019
asvetlov pushed a commit that referenced this issue May 6, 2019
asvetlov pushed a commit that referenced this issue May 6, 2019
…epalive field with __slots__ (#3727)

(cherry picked from commit bfb99eb)

Co-authored-by: Artem Yushkovskiy <artem.yushkovskiy@neuromation.io>
@asvetlov asvetlov closed this as completed May 6, 2019
asvetlov pushed a commit that referenced this issue May 8, 2019
…ch _keepalive field with __slots__ (#3727) (#3731)

(cherry picked from commit bfb99eb)

Co-authored-by: Artem Yushkovskiy <artem.yushkovskiy@neuromation.io>
(cherry picked from commit ff1c9de)

Co-authored-by: Artem Yushkovskiy <artem.yushkovskiy@neuromation.io>
asvetlov added a commit that referenced this issue May 8, 2019
…epalive field with __slots__ (#3727) (#3729)

(cherry picked from commit bfb99eb)

Co-authored-by: Artem Yushkovskiy <artem.yushkovskiy@neuromation.io>
@lock lock bot added the outdated label May 5, 2020
@lock lock bot locked as resolved and limited conversation to collaborators May 5, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

4 participants