Skip to content

Commit ba13215

Browse files
authored
gh-113538: Revert "gh-113538: Add asycio.Server.{close,abort}_clients (#114432)" (#116632)
Revert "gh-113538: Add asycio.Server.{close,abort}_clients (#114432)" Reason: The new test doesn't always pass: #116423 (comment) This reverts commit 1d0d49a.
1 parent 2b67fc5 commit ba13215

File tree

8 files changed

+20
-152
lines changed

8 files changed

+20
-152
lines changed

Doc/library/asyncio-eventloop.rst

-25
Original file line numberDiff line numberDiff line change
@@ -1641,31 +1641,6 @@ Do not instantiate the :class:`Server` class directly.
16411641
coroutine to wait until the server is closed (and no more
16421642
connections are active).
16431643

1644-
.. method:: close_clients()
1645-
1646-
Close all existing incoming client connections.
1647-
1648-
Calls :meth:`~asyncio.BaseTransport.close` on all associated
1649-
transports.
1650-
1651-
:meth:`close` should be called before :meth:`close_clients` when
1652-
closing the server to avoid races with new clients connecting.
1653-
1654-
.. versionadded:: 3.13
1655-
1656-
.. method:: abort_clients()
1657-
1658-
Close all existing incoming client connections immediately,
1659-
without waiting for pending operations to complete.
1660-
1661-
Calls :meth:`~asyncio.WriteTransport.abort` on all associated
1662-
transports.
1663-
1664-
:meth:`close` should be called before :meth:`abort_clients` when
1665-
closing the server to avoid races with new clients connecting.
1666-
1667-
.. versionadded:: 3.13
1668-
16691644
.. method:: get_loop()
16701645

16711646
Return the event loop associated with the server object.

Doc/whatsnew/3.13.rst

-5
Original file line numberDiff line numberDiff line change
@@ -270,11 +270,6 @@ asyncio
270270
the buffer size.
271271
(Contributed by Jamie Phan in :gh:`115199`.)
272272

273-
* Add :meth:`asyncio.Server.close_clients` and
274-
:meth:`asyncio.Server.abort_clients` methods which allow to more
275-
forcefully close an asyncio server.
276-
(Contributed by Pierre Ossman in :gh:`113538`.)
277-
278273
base64
279274
---
280275

Lib/asyncio/base_events.py

+8-17
Original file line numberDiff line numberDiff line change
@@ -279,9 +279,7 @@ def __init__(self, loop, sockets, protocol_factory, ssl_context, backlog,
279279
ssl_handshake_timeout, ssl_shutdown_timeout=None):
280280
self._loop = loop
281281
self._sockets = sockets
282-
# Weak references so we don't break Transport's ability to
283-
# detect abandoned transports
284-
self._clients = weakref.WeakSet()
282+
self._active_count = 0
285283
self._waiters = []
286284
self._protocol_factory = protocol_factory
287285
self._backlog = backlog
@@ -294,13 +292,14 @@ def __init__(self, loop, sockets, protocol_factory, ssl_context, backlog,
294292
def __repr__(self):
295293
return f'<{self.__class__.__name__} sockets={self.sockets!r}>'
296294

297-
def _attach(self, transport):
295+
def _attach(self):
298296
assert self._sockets is not None
299-
self._clients.add(transport)
297+
self._active_count += 1
300298

301-
def _detach(self, transport):
302-
self._clients.discard(transport)
303-
if len(self._clients) == 0 and self._sockets is None:
299+
def _detach(self):
300+
assert self._active_count > 0
301+
self._active_count -= 1
302+
if self._active_count == 0 and self._sockets is None:
304303
self._wakeup()
305304

306305
def _wakeup(self):
@@ -349,17 +348,9 @@ def close(self):
349348
self._serving_forever_fut.cancel()
350349
self._serving_forever_fut = None
351350

352-
if len(self._clients) == 0:
351+
if self._active_count == 0:
353352
self._wakeup()
354353

355-
def close_clients(self):
356-
for transport in self._clients.copy():
357-
transport.close()
358-
359-
def abort_clients(self):
360-
for transport in self._clients.copy():
361-
transport.abort()
362-
363354
async def start_serving(self):
364355
self._start_serving()
365356
# Skip one loop iteration so that all 'loop.add_reader'

Lib/asyncio/events.py

-8
Original file line numberDiff line numberDiff line change
@@ -175,14 +175,6 @@ def close(self):
175175
"""Stop serving. This leaves existing connections open."""
176176
raise NotImplementedError
177177

178-
def close_clients(self):
179-
"""Close all active connections."""
180-
raise NotImplementedError
181-
182-
def abort_clients(self):
183-
"""Close all active connections immediately."""
184-
raise NotImplementedError
185-
186178
def get_loop(self):
187179
"""Get the event loop the Server object is attached to."""
188180
raise NotImplementedError

Lib/asyncio/proactor_events.py

+2-2
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ def __init__(self, loop, sock, protocol, waiter=None,
6363
self._called_connection_lost = False
6464
self._eof_written = False
6565
if self._server is not None:
66-
self._server._attach(self)
66+
self._server._attach()
6767
self._loop.call_soon(self._protocol.connection_made, self)
6868
if waiter is not None:
6969
# only wake up the waiter when connection_made() has been called
@@ -167,7 +167,7 @@ def _call_connection_lost(self, exc):
167167
self._sock = None
168168
server = self._server
169169
if server is not None:
170-
server._detach(self)
170+
server._detach()
171171
self._server = None
172172
self._called_connection_lost = True
173173

Lib/asyncio/selector_events.py

+2-4
Original file line numberDiff line numberDiff line change
@@ -791,7 +791,7 @@ def __init__(self, loop, sock, protocol, extra=None, server=None):
791791
self._paused = False # Set when pause_reading() called
792792

793793
if self._server is not None:
794-
self._server._attach(self)
794+
self._server._attach()
795795
loop._transports[self._sock_fd] = self
796796

797797
def __repr__(self):
@@ -868,8 +868,6 @@ def __del__(self, _warn=warnings.warn):
868868
if self._sock is not None:
869869
_warn(f"unclosed transport {self!r}", ResourceWarning, source=self)
870870
self._sock.close()
871-
if self._server is not None:
872-
self._server._detach(self)
873871

874872
def _fatal_error(self, exc, message='Fatal error on transport'):
875873
# Should be called from exception handler only.
@@ -908,7 +906,7 @@ def _call_connection_lost(self, exc):
908906
self._loop = None
909907
server = self._server
910908
if server is not None:
911-
server._detach(self)
909+
server._detach()
912910
self._server = None
913911

914912
def get_write_buffer_size(self):

Lib/test/test_asyncio/test_server.py

+8-88
Original file line numberDiff line numberDiff line change
@@ -125,12 +125,8 @@ async def main(srv):
125125
class TestServer2(unittest.IsolatedAsyncioTestCase):
126126

127127
async def test_wait_closed_basic(self):
128-
async def serve(rd, wr):
129-
try:
130-
await rd.read()
131-
finally:
132-
wr.close()
133-
await wr.wait_closed()
128+
async def serve(*args):
129+
pass
134130

135131
srv = await asyncio.start_server(serve, socket_helper.HOSTv4, 0)
136132
self.addCleanup(srv.close)
@@ -141,8 +137,7 @@ async def serve(rd, wr):
141137
self.assertFalse(task1.done())
142138

143139
# active count != 0, not closed: should block
144-
addr = srv.sockets[0].getsockname()
145-
(rd, wr) = await asyncio.open_connection(addr[0], addr[1])
140+
srv._attach()
146141
task2 = asyncio.create_task(srv.wait_closed())
147142
await asyncio.sleep(0)
148143
self.assertFalse(task1.done())
@@ -157,8 +152,7 @@ async def serve(rd, wr):
157152
self.assertFalse(task2.done())
158153
self.assertFalse(task3.done())
159154

160-
wr.close()
161-
await wr.wait_closed()
155+
srv._detach()
162156
# active count == 0, closed: should unblock
163157
await task1
164158
await task2
@@ -167,96 +161,22 @@ async def serve(rd, wr):
167161

168162
async def test_wait_closed_race(self):
169163
# Test a regression in 3.12.0, should be fixed in 3.12.1
170-
async def serve(rd, wr):
171-
try:
172-
await rd.read()
173-
finally:
174-
wr.close()
175-
await wr.wait_closed()
164+
async def serve(*args):
165+
pass
176166

177167
srv = await asyncio.start_server(serve, socket_helper.HOSTv4, 0)
178168
self.addCleanup(srv.close)
179169

180170
task = asyncio.create_task(srv.wait_closed())
181171
await asyncio.sleep(0)
182172
self.assertFalse(task.done())
183-
addr = srv.sockets[0].getsockname()
184-
(rd, wr) = await asyncio.open_connection(addr[0], addr[1])
173+
srv._attach()
185174
loop = asyncio.get_running_loop()
186175
loop.call_soon(srv.close)
187-
loop.call_soon(wr.close)
176+
loop.call_soon(srv._detach)
188177
await srv.wait_closed()
189178

190-
async def test_close_clients(self):
191-
async def serve(rd, wr):
192-
try:
193-
await rd.read()
194-
finally:
195-
wr.close()
196-
await wr.wait_closed()
197-
198-
srv = await asyncio.start_server(serve, socket_helper.HOSTv4, 0)
199-
self.addCleanup(srv.close)
200-
201-
addr = srv.sockets[0].getsockname()
202-
(rd, wr) = await asyncio.open_connection(addr[0], addr[1])
203-
self.addCleanup(wr.close)
204-
205-
task = asyncio.create_task(srv.wait_closed())
206-
await asyncio.sleep(0)
207-
self.assertFalse(task.done())
208-
209-
srv.close()
210-
srv.close_clients()
211-
await asyncio.sleep(0)
212-
await asyncio.sleep(0)
213-
self.assertTrue(task.done())
214-
215-
async def test_abort_clients(self):
216-
async def serve(rd, wr):
217-
nonlocal s_rd, s_wr
218-
s_rd = rd
219-
s_wr = wr
220-
await wr.wait_closed()
221-
222-
s_rd = s_wr = None
223-
srv = await asyncio.start_server(serve, socket_helper.HOSTv4, 0)
224-
self.addCleanup(srv.close)
225-
226-
addr = srv.sockets[0].getsockname()
227-
(c_rd, c_wr) = await asyncio.open_connection(addr[0], addr[1], limit=4096)
228-
self.addCleanup(c_wr.close)
229-
230-
# Limit the socket buffers so we can reliably overfill them
231-
s_sock = s_wr.get_extra_info('socket')
232-
s_sock.setsockopt(socket.SOL_SOCKET, socket.SO_SNDBUF, 65536)
233-
c_sock = c_wr.get_extra_info('socket')
234-
c_sock.setsockopt(socket.SOL_SOCKET, socket.SO_RCVBUF, 65536)
235-
236-
# Get the reader in to a paused state by sending more than twice
237-
# the configured limit
238-
s_wr.write(b'a' * 4096)
239-
s_wr.write(b'a' * 4096)
240-
s_wr.write(b'a' * 4096)
241-
while c_wr.transport.is_reading():
242-
await asyncio.sleep(0)
243-
244-
# Get the writer in a waiting state by sending data until the
245-
# socket buffers are full on both server and client sockets and
246-
# the kernel stops accepting more data
247-
s_wr.write(b'a' * c_sock.getsockopt(socket.SOL_SOCKET, socket.SO_RCVBUF))
248-
s_wr.write(b'a' * s_sock.getsockopt(socket.SOL_SOCKET, socket.SO_SNDBUF))
249-
self.assertNotEqual(s_wr.transport.get_write_buffer_size(), 0)
250-
251-
task = asyncio.create_task(srv.wait_closed())
252-
await asyncio.sleep(0)
253-
self.assertFalse(task.done())
254179

255-
srv.close()
256-
srv.abort_clients()
257-
await asyncio.sleep(0)
258-
await asyncio.sleep(0)
259-
self.assertTrue(task.done())
260180

261181

262182
# Test the various corner cases of Unix server socket removal

Misc/NEWS.d/next/Library/2024-01-22-15-50-58.gh-issue-113538.v2wrwg.rst

-3
This file was deleted.

0 commit comments

Comments
 (0)