Restore compatibility of serve_forever with uvloop.

This is also a much better implementation.

Refs #1608.

Author: aaugustin

docs/project/changelog.rst

@@ -25,13 +25,21 @@ fixing regressions shortly after a release.
 Only documented APIs are public. Undocumented, private APIs may change without
 notice.
 
-.. _17.1:
+.. _17.0.1:
 
-17.1
-----
+17.0.1
+------
 
 *In development*
 
+Bug fixes
+.........
+
+* Restored compatibility of :meth:`~asyncio.server.Server.serve_forever` in the
+  :mod:`asyncio` implementation with third-party event loops such as uvloop_.
+
+.. _uvloop: https://uvloop.readthedocs.io/
+
 .. _17.0:
 
 17.0

src/websockets/asyncio/server.py

@@ -422,33 +422,21 @@ async def serve_forever(self) -> None:
             await server.serve_forever()
 
         This is an alternative to using :func:`serve` as an asynchronous context
-        manager. Shutdown is triggered by canceling :meth:`serve_forever`
-        instead of exiting a :func:`serve` context.
+        manager. Shutdown is triggered by canceling :meth:`serve_forever` or by
+        calling :meth:`~Server.close` from another task.
 
         """
-        # This is a copy-paste of asyncio.Server.serve_forever(), with
-        # self.server instead of self, except it calls our close() and
-        # wait_closed() when canceled to ensure a graceful shutdown.
-        if self.server._serving_forever_fut is not None:  # type: ignore[attr-defined]
-            raise RuntimeError(
-                f"server {self.server!r} is already being awaited on serve_forever()"
-            )
-        if self.server._sockets is None:  # type: ignore[attr-defined]
-            raise RuntimeError(f"server {self.server!r} is closed")
-
-        self.server._start_serving()  # type: ignore[attr-defined]
-        self.server._serving_forever_fut = self.server._loop.create_future()  # type: ignore[attr-defined]
-
         try:
-            await self.server._serving_forever_fut  # type: ignore[attr-defined]
+            if not self.is_serving():
+                await self.start_serving()
+            # If close() is called, wait_closed() will return, and we'll exit.
+            await self.wait_closed()
         except asyncio.CancelledError:
             try:
                 self.close()
                 await self.wait_closed()
             finally:
                 raise
-        finally:
-            self.server._serving_forever_fut = None  # type: ignore[attr-defined]
 
     @property
     def sockets(self) -> tuple[socket.socket, ...]:

src/websockets/version.py

@@ -20,7 +20,7 @@
 
 released = False
 
-tag = version = commit = "17.1"
+tag = version = commit = "17.0.1"
 
 
 if not released:  # pragma: no cover

tests/asyncio/test_server.py

@@ -599,19 +599,23 @@ async def test_serve_forever(self):
 
         self.assertFalse(server.is_serving())
 
-    async def test_close_cancels_serve_forever(self):
-        """Closing the server cancels serve_forever()."""
-        server = await serve(*args)
+    async def test_serve_forever_starts_serving(self):
+        """serve_forever() starts serving when start_serving is False."""
+        server = await serve(*args, start_serving=False)
+        self.assertFalse(server.is_serving())
+
         serve_forever_task = asyncio.get_running_loop().create_task(
             server.serve_forever()
         )
         await asyncio.sleep(0)
 
-        server.close()
+        self.assertTrue(server.is_serving())
+
+        serve_forever_task.cancel()
         with self.assertRaises(asyncio.CancelledError):
             await serve_forever_task
 
-    async def test_serve_forever_closes_open_connections(self):
+    async def test_canceling_serve_forever_closes_open_connections(self):
         """Canceling serve_forever() closes open connections with code 1001."""
         server = await serve(*args)
         serve_forever_task = asyncio.get_running_loop().create_task(
@@ -631,29 +635,18 @@ async def test_serve_forever_closes_open_connections(self):
         with self.assertRaises(asyncio.CancelledError):
             await serve_forever_task
 
-    async def test_serve_forever_twice(self):
-        """Server rejects awaiting serve_forever() concurrently."""
+    async def test_close_exits_serve_forever(self):
+        """Closing the server exits serve_forever()."""
         server = await serve(*args)
         serve_forever_task = asyncio.get_running_loop().create_task(
             server.serve_forever()
         )
         await asyncio.sleep(0)
+        server.close()
 
-        with self.assertRaises(RuntimeError):
-            await server.serve_forever()
-
-        serve_forever_task.cancel()
-        with self.assertRaises(asyncio.CancelledError):
+        async with asyncio.timeout(MS):
             await serve_forever_task
 
-    async def test_serve_forever_after_close(self):
-        """Server rejects serve_forever() after close()."""
-        async with serve(*args) as server:
-            pass
-
-        with self.assertRaises(RuntimeError):
-            await server.serve_forever()
-
     async def test_sockets(self):
         """Server provides a sockets property."""
         async with serve(*args) as server: