Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

GH-109564: add asyncio.Server state machine #131009

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

Open
wants to merge 9 commits into
base: main
Choose a base branch
Loading
from
Prev Previous commit
Next Next commit
fix: change match to if statements
  • Loading branch information
ordinary-jamie committed Mar 10, 2025
commit 8e409b7eb61062b8ec45672180965f6beacab9f2
42 changes: 19 additions & 23 deletions 42 Lib/asyncio/base_events.py
Original file line number Diff line number Diff line change
Expand Up @@ -322,16 +322,15 @@ def _detach(self, transport):
self._wakeup()

def _wakeup(self):
match self._state:
case _ServerState.SHUTDOWN:
# gh109564: the wakeup method has two possible call-sites,
# through an explicit call Server.close(), or indirectly through
# Server._detach() by the last connected client.
return
case _ServerState.INITIALIZED | _ServerState.SERVING:
raise RuntimeError("cannot wakeup server before closing")
case _ServerState.CLOSED:
self._state = _ServerState.SHUTDOWN
if self._state == _ServerState.CLOSED:
self._state = _ServerState.SHUTDOWN
elif self._state == _ServerState.SHUTDOWN:
# gh109564: the wakeup method has two possible call-sites,
# through an explicit call Server.close(), or indirectly through
# Server._detach() by the last connected client.
return
else:
raise RuntimeError(f"server {self!r} can only wakeup waiters after closing")
Copy link
Member

Choose a reason for hiding this comment

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

This error message isn't making much sense to me. We'll never reach here "after closing," right? This can only be triggered when the state is SERVING or INITIALIZED.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry this is bad wording, I was trying to say that the Server must be closed before it can be shutdown (i.e. we cannot shutdown from the SERVING state, nor the INITIALIZED/NOT_STARTED state)

Made a fix in: 7f3481b


waiters = self._waiters
self._waiters = None
Expand All @@ -340,13 +339,12 @@ def _wakeup(self):
waiter.set_result(None)

def _start_serving(self):
match self._state:
case _ServerState.SERVING:
return
case _ServerState.CLOSED | _ServerState.SHUTDOWN:
raise RuntimeError(f'server {self!r} is closed')
case _ServerState.INITIALIZED:
self._state = _ServerState.SERVING
if self._state == _ServerState.INITIALIZED:
self._state = _ServerState.SERVING
elif self._state == _ServerState.SERVING:
return
else:
raise RuntimeError(f'server {self!r} is closed')
ordinary-jamie marked this conversation as resolved.
Show resolved Hide resolved

for sock in self._sockets:
sock.listen(self._backlog)
Expand All @@ -368,12 +366,10 @@ def sockets(self):
return tuple(trsock.TransportSocket(s) for s in self._sockets)

def close(self):
match self._state:
case _ServerState.CLOSED | _ServerState.SHUTDOWN:
# Shutdown state can only be reached after closing.
return
case _:
self._state = _ServerState.CLOSED
if self._state == _ServerState.CLOSED or self._state == _ServerState.SHUTDOWN:
ordinary-jamie marked this conversation as resolved.
Show resolved Hide resolved
return
else:
self._state = _ServerState.CLOSED
Copy link
Member

Choose a reason for hiding this comment

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

What happens if something magically goes wrong after this has been set? Is the server still alive while thinking it's closed? It might be worth adding a try/except to undo this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah great point! Although I'm not sure how that would look from a user's point of view. Should we give them a warning to let them know they must try to close the server again? Alternatively, should we expose something like a force kwarg/flag?

I pushed an initial fix with a simple try/except to recover the previous state on fail in 48a3c0d


sockets = self._sockets
if sockets is None:
Expand Down
Loading
Morty Proxy This is a proxified and sanitized view of the page, visit original site.