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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix shutdown event in Websocket Implementations #1738

Closed
wants to merge 6 commits into from
Closed
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
30 changes: 30 additions & 0 deletions tests/protocols/test_websocket.py
Expand Up @@ -5,6 +5,7 @@

from tests.protocols.test_http import HTTP_PROTOCOLS
from tests.utils import run_server
from uvicorn import Server
from uvicorn.config import Config
from uvicorn.protocols.websockets.wsproto_impl import WSProtocol

Expand Down Expand Up @@ -756,3 +757,32 @@ async def open_connection(url):
async with run_server(config):
headers = await open_connection("ws://127.0.0.1:8000")
assert headers.get_all("Server") == ["uvicorn", "over-ridden", "another-value"]


@pytest.mark.anyio
@pytest.mark.parametrize("ws_protocol_cls", WS_PROTOCOLS)
@pytest.mark.parametrize("http_protocol_cls", HTTP_PROTOCOLS)
async def test_server_shutdown_when_connection_active(
ws_protocol_cls, http_protocol_cls
):
class App(WebSocketResponse):
async def websocket_connect(self, message):
await self.send({"type": "websocket.accept"})

config = Config(
app=App,
ws=ws_protocol_cls,
http=http_protocol_cls,
lifespan="off",
)
server = Server(config=config)
task = asyncio.create_task(server.serve(sockets=None))
await asyncio.sleep(0.1)
async with websockets.connect("ws://127.0.0.1:8000") as websocket:
ws_conn = list(server.server_state.connections)[0]
ws_conn.shutdown()
await asyncio.sleep(0.1)
assert websocket.close_code == 1012
assert ws_conn.transport.is_closing()
await server.shutdown()
task.cancel()
4 changes: 3 additions & 1 deletion uvicorn/protocols/websockets/websockets_impl.py
Expand Up @@ -141,7 +141,9 @@ def connection_lost(self, exc: Optional[Exception]) -> None:

def shutdown(self) -> None:
self.ws_server.closing = True
self.transport.close()
task = asyncio.create_task(self.close(code=1012))
task.add_done_callback(self.on_task_complete)
self.tasks.add(task)

Copy link
Contributor Author

@iudeen iudeen Oct 28, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Help needed here.

self.transport.close() causes unexpected closure of connection at this place. However the transport is getting closed at a later stage. Is this okay?

def on_task_complete(self, task: asyncio.Task) -> None:
self.tasks.discard(task)
Expand Down