bpo-41093: TCPServer's serve_forever() shuts down immediately when calling shutdown() #21094
bpo-41093: TCPServer's serve_forever() shuts down immediately when calling shutdown() #21094tontinton wants to merge 4 commits into
Conversation
64facaf to
e4987db
Compare
285bf33 to
49bcf7e
Compare
|
poke |
There was a problem hiding this comment.
Thanks so much for your commits and welcome to the cpython.
Your changes look good. Perhaps you would describe these changes in the documentation before other participants look at your code there. You can added docs for your changes in Doc/library/sockerserver.rst. And in my opinion you can added some tests for your changes in Lib/test/test_sockerserver.py.
There was a problem hiding this comment.
This method mark as overridden but if we override it we need to know that need to call self.server_close(). This is not obvious in my opinion.
There was a problem hiding this comment.
Fixed, from now server_shutdown will only shutdown the socket without closing it.
There was a problem hiding this comment.
In my opinion, don't need to remove this comment because the proposal has not yet been resolved.
There was a problem hiding this comment.
I added the comment again, but I do think that by implementing the server_shutdown() function this is resolved
There was a problem hiding this comment.
Please follow the comment style of this file. Comments are written before the comment line.
There was a problem hiding this comment.
In my opinion, it is worth leaving this method as pass. How this is done with the server_activate method since this is an unobvious behavior of the method.
a90e6b5 to
5c5fb9c
Compare
After adding a test I realized that this fixes the issue only on windows and linux, mac os works the same, nothing changes |
|
@tontinton Could you resolve the conflicts? |
server_shutdown() is called on BaseServer's shutdown method to make server_forever() return immediately on shutdown not waiting for select() to timeout or return on EVENT_READ.
Fixed issue `41093` as now every server inheriting TCPServer will shutdown immediately after calling shutdown(). Note: UDPServer doesn't call shutdown() on it's server socket as there is no need.
5c5fb9c to
a35ab1e
Compare
The test creates a TCPServer and calls server_forever on it with a high poll interval of 30 seconds, then calls shutdown on it and validates that it took less than a second.
a35ab1e to
a3e282b
Compare
@furkanonder fixed. |
|
This PR is stale because it has been open for 30 days with no activity. |
https://bugs.python.org/issue41093