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

start_tls: AssertionError if writing is paused #564

Open
sorcio opened this issue Sep 11, 2023 · 0 comments
Open

start_tls: AssertionError if writing is paused #564

sorcio opened this issue Sep 11, 2023 · 0 comments

Comments

@sorcio
Copy link

sorcio commented Sep 11, 2023

First found in the CPython port of sslproto (see python/cpython#109051 but disregard the title, it's not platform-specific), and it can be reproduced in uvloop.

Traceback

protocol.resume_writing() failed
protocol: <uvloop.loop.SSLProtocol object at 0x1033910c0>
transport: <TCPTransport closed=False reading=True 0x1098351c0>
Traceback (most recent call last):
  File "uvloop/handles/basetransport.pyx", line 96, in uvloop.loop.UVBaseTransport._maybe_resume_protocol
    run_in_context(
  File "uvloop/loop.pyx", line 101, in uvloop.loop.run_in_context
    return context.run(method)
  File "uvloop/sslproto.pyx", line 922, in uvloop.loop.SSLProtocol.resume_writing
    assert self._ssl_writing_paused
AssertionError

Simplest way to reproduce is to increase payload size in test_start_tls_server_1. Send will block and transport will request to pause writing. After start_tls(), the transport will attempt to resume the now-switched protocol, which was not aware it was ever supposed to be paused.

I commented further in the original bug with some notes. The CPython implementation is a bit different (it has a redundant buffering mechanism with a deque which I believe is unnecessary) but the fundamental issue is the same.

Question: what is the motivation for the extra flow control layer in SSLProtocol? It seems it was introduced in uvloop first. I suggested it can be removed and let the original protocol and original transport deal with flow control (which would also fix the bug). But it wouldn't be a good idea to remove it if it's solving a known problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant