-
-
Notifications
You must be signed in to change notification settings - Fork 32k
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
base: main
Are you sure you want to change the base?
Changes from 1 commit
0fec860
f3b96bf
8e409b7
a92158a
44d24fb
07129e5
48a3c0d
5832655
7f3481b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
- Loading branch information
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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") | ||
|
||
waiters = self._waiters | ||
self._waiters = None | ||
|
@@ -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) | ||
|
@@ -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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 I pushed an initial fix with a simple |
||
|
||
sockets = self._sockets | ||
if sockets is None: | ||
|
There was a problem hiding this comment.
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
orINITIALIZED
.There was a problem hiding this comment.
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 theINITIALIZED
/NOT_STARTED
state)Made a fix in: 7f3481b