From 21ff6be9c98192116cebde3fa1a54bb112a8b3ac Mon Sep 17 00:00:00 2001
From: Pierre Ossman <ossman@cendio.se>
Date: Fri, 27 Oct 2023 16:35:29 +0200
Subject: [PATCH 1/6] gh-111246: Remove listening Unix socket on close
(#111246)
Try to clean up the socket file we create so we don't add unused noise
to the file system.
---
Doc/library/asyncio-eventloop.rst | 5 ++++
Doc/whatsnew/3.13.rst | 6 ++++
Lib/asyncio/unix_events.py | 21 ++++++++++++++
Lib/test/test_asyncio/test_server.py | 29 +++++++++++++++++++
...-10-30-14-47-23.gh-issue-111246.QJ_ehs.rst | 2 ++
5 files changed, 63 insertions(+)
create mode 100644 Misc/NEWS.d/next/Library/2023-10-30-14-47-23.gh-issue-111246.QJ_ehs.rst
diff --git a/Doc/library/asyncio-eventloop.rst b/Doc/library/asyncio-eventloop.rst
index 75c459c0cb601f..435df34d6971ba 100644
--- a/Doc/library/asyncio-eventloop.rst
+++ b/Doc/library/asyncio-eventloop.rst
@@ -800,6 +800,11 @@ Creating network servers
Added the *ssl_shutdown_timeout* parameter.
+ .. versionchanged:: 3.13
+
+ The Unix socket will automatically be removed from the filesystem
+ when the server is closed.
+
.. coroutinemethod:: loop.connect_accepted_socket(protocol_factory, \
sock, *, ssl=None, ssl_handshake_timeout=None, \
diff --git a/Doc/whatsnew/3.13.rst b/Doc/whatsnew/3.13.rst
index 34dd3ea8858ea2..b3e635b9d567fc 100644
--- a/Doc/whatsnew/3.13.rst
+++ b/Doc/whatsnew/3.13.rst
@@ -144,6 +144,12 @@ array
It can be used instead of ``'u'`` type code, which is deprecated.
(Contributed by Inada Naoki in :gh:`80480`.)
+asyncio
+-------
+
+* :meth:`asyncio.loop.create_unix_server` will now automatically remove
+ the Unix socket when the server is closed.
+
copy
----
diff --git a/Lib/asyncio/unix_events.py b/Lib/asyncio/unix_events.py
index 809f29eecba198..dad41b3f230032 100644
--- a/Lib/asyncio/unix_events.py
+++ b/Lib/asyncio/unix_events.py
@@ -460,6 +460,27 @@ def cb(fut):
self.remove_writer(fd)
fut.add_done_callback(cb)
+ def _stop_serving(self, sock):
+ # Is this a unix socket that needs cleanup?
+ if sock.family == socket.AF_UNIX:
+ path = sock.getsockname()
+ if path == '':
+ path = None
+ # Check for abstract socket. `str` and `bytes` paths are supported.
+ elif path[0] in (0, '\x00'):
+ path = None
+ else:
+ path = None
+
+ super()._stop_serving(sock)
+
+ if path is not None:
+ try:
+ os.unlink(path)
+ except OSError as err:
+ logger.error('Unable to clean up listening UNIX socket '
+ '%r: %r', path, err)
+
class _UnixReadPipeTransport(transports.ReadTransport):
diff --git a/Lib/test/test_asyncio/test_server.py b/Lib/test/test_asyncio/test_server.py
index 06d8b60f219f1a..b37bd3f8a34c8c 100644
--- a/Lib/test/test_asyncio/test_server.py
+++ b/Lib/test/test_asyncio/test_server.py
@@ -1,4 +1,6 @@
import asyncio
+import os
+import socket
import time
import threading
import unittest
@@ -147,6 +149,33 @@ async def serve(*args):
await task2
+class UnixServerCleanupTests(unittest.IsolatedAsyncioTestCase):
+ @socket_helper.skip_unless_bind_unix_socket
+ async def test_unix_server_addr_cleanup(self):
+ with test_utils.unix_socket_path() as addr:
+ async def serve(*args):
+ pass
+
+ srv = await asyncio.start_unix_server(serve, addr)
+
+ srv.close()
+ self.assertFalse(os.path.exists(addr))
+
+ @socket_helper.skip_unless_bind_unix_socket
+ async def test_unix_server_sock_cleanup(self):
+ with test_utils.unix_socket_path() as addr:
+ async def serve(*args):
+ pass
+
+ sock = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM)
+ sock.bind(addr)
+
+ srv = await asyncio.start_unix_server(serve, sock=sock)
+
+ srv.close()
+ self.assertFalse(os.path.exists(addr))
+
+
@unittest.skipUnless(hasattr(asyncio, 'ProactorEventLoop'), 'Windows only')
class ProactorStartServerTests(BaseStartServer, unittest.TestCase):
diff --git a/Misc/NEWS.d/next/Library/2023-10-30-14-47-23.gh-issue-111246.QJ_ehs.rst b/Misc/NEWS.d/next/Library/2023-10-30-14-47-23.gh-issue-111246.QJ_ehs.rst
new file mode 100644
index 00000000000000..a9630de5cc6947
--- /dev/null
+++ b/Misc/NEWS.d/next/Library/2023-10-30-14-47-23.gh-issue-111246.QJ_ehs.rst
@@ -0,0 +1,2 @@
+:meth:`asyncio.loop.create_unix_server` will now automatically remove the
+Unix socket when the server is closed.
From 21e9d8cec082e627cba95dbfbfe366deff7803e6 Mon Sep 17 00:00:00 2001
From: Pierre Ossman <ossman@cendio.se>
Date: Fri, 27 Oct 2023 16:37:12 +0200
Subject: [PATCH 2/6] gh-111246: Don't log on redundant socket cleanup
(#111246)
---
Lib/asyncio/unix_events.py | 2 ++
Lib/test/test_asyncio/test_server.py | 15 +++++++++++++++
2 files changed, 17 insertions(+)
diff --git a/Lib/asyncio/unix_events.py b/Lib/asyncio/unix_events.py
index dad41b3f230032..f397a1ed1d7b68 100644
--- a/Lib/asyncio/unix_events.py
+++ b/Lib/asyncio/unix_events.py
@@ -477,6 +477,8 @@ def _stop_serving(self, sock):
if path is not None:
try:
os.unlink(path)
+ except FileNotFoundError:
+ pass
except OSError as err:
logger.error('Unable to clean up listening UNIX socket '
'%r: %r', path, err)
diff --git a/Lib/test/test_asyncio/test_server.py b/Lib/test/test_asyncio/test_server.py
index b37bd3f8a34c8c..009b5093baf405 100644
--- a/Lib/test/test_asyncio/test_server.py
+++ b/Lib/test/test_asyncio/test_server.py
@@ -175,6 +175,21 @@ async def serve(*args):
srv.close()
self.assertFalse(os.path.exists(addr))
+ @socket_helper.skip_unless_bind_unix_socket
+ async def test_unix_server_cleanup_gone(self):
+ with test_utils.unix_socket_path() as addr:
+ async def serve(*args):
+ pass
+
+ sock = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM)
+ sock.bind(addr)
+
+ srv = await asyncio.start_unix_server(serve, sock=sock)
+
+ os.unlink(addr)
+
+ srv.close()
+
@unittest.skipUnless(hasattr(asyncio, 'ProactorEventLoop'), 'Windows only')
class ProactorStartServerTests(BaseStartServer, unittest.TestCase):
From df3b74edfe09de4322bf922430493d5fae24c82a Mon Sep 17 00:00:00 2001
From: Pierre Ossman <ossman@cendio.se>
Date: Fri, 27 Oct 2023 17:08:22 +0200
Subject: [PATCH 3/6] gh-111246: Don't remove stolen Unix socket address
(#111246)
We only want to clean up *our* socket, so try to determine if we still
own this address or if something else has replaced it.
---
Doc/library/asyncio-eventloop.rst | 3 ++-
Lib/asyncio/unix_events.py | 21 ++++++++++++++-------
Lib/test/test_asyncio/test_server.py | 15 +++++++++++++++
3 files changed, 31 insertions(+), 8 deletions(-)
diff --git a/Doc/library/asyncio-eventloop.rst b/Doc/library/asyncio-eventloop.rst
index 435df34d6971ba..398bd1ab8ce268 100644
--- a/Doc/library/asyncio-eventloop.rst
+++ b/Doc/library/asyncio-eventloop.rst
@@ -803,7 +803,8 @@ Creating network servers
.. versionchanged:: 3.13
The Unix socket will automatically be removed from the filesystem
- when the server is closed.
+ when the server is closed, unless the socket has been replaced
+ after the server has been created.
.. coroutinemethod:: loop.connect_accepted_socket(protocol_factory, \
diff --git a/Lib/asyncio/unix_events.py b/Lib/asyncio/unix_events.py
index f397a1ed1d7b68..ba2a9e7fc57d9a 100644
--- a/Lib/asyncio/unix_events.py
+++ b/Lib/asyncio/unix_events.py
@@ -64,6 +64,7 @@ class _UnixSelectorEventLoop(selector_events.BaseSelectorEventLoop):
def __init__(self, selector=None):
super().__init__(selector)
self._signal_handlers = {}
+ self._unix_server_sockets = {}
def close(self):
super().close()
@@ -340,6 +341,14 @@ async def create_unix_server(
raise ValueError(
f'A UNIX Domain Stream Socket was expected, got {sock!r}')
+ path = sock.getsockname()
+ # Check for abstract socket. `str` and `bytes` paths are supported.
+ if path[0] not in (0, '\x00'):
+ try:
+ self._unix_server_sockets[sock] = os.stat(path).st_ino
+ except FileNotFoundError:
+ pass
+
sock.setblocking(False)
server = base_events.Server(self, [sock], protocol_factory,
ssl, backlog, ssl_handshake_timeout,
@@ -462,21 +471,19 @@ def cb(fut):
def _stop_serving(self, sock):
# Is this a unix socket that needs cleanup?
- if sock.family == socket.AF_UNIX:
+ if sock in self._unix_server_sockets:
path = sock.getsockname()
- if path == '':
- path = None
- # Check for abstract socket. `str` and `bytes` paths are supported.
- elif path[0] in (0, '\x00'):
- path = None
else:
path = None
super()._stop_serving(sock)
if path is not None:
+ prev_ino = self._unix_server_sockets[sock]
+ del self._unix_server_sockets[sock]
try:
- os.unlink(path)
+ if os.stat(path).st_ino == prev_ino:
+ os.unlink(path)
except FileNotFoundError:
pass
except OSError as err:
diff --git a/Lib/test/test_asyncio/test_server.py b/Lib/test/test_asyncio/test_server.py
index 009b5093baf405..b8d35f8d213293 100644
--- a/Lib/test/test_asyncio/test_server.py
+++ b/Lib/test/test_asyncio/test_server.py
@@ -190,6 +190,21 @@ async def serve(*args):
srv.close()
+ @socket_helper.skip_unless_bind_unix_socket
+ async def test_unix_server_cleanup_replaced(self):
+ with test_utils.unix_socket_path() as addr:
+ async def serve(*args):
+ pass
+
+ srv = await asyncio.start_unix_server(serve, addr)
+
+ os.unlink(addr)
+ sock = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM)
+ sock.bind(addr)
+
+ srv.close()
+ self.assertTrue(os.path.exists(addr))
+
@unittest.skipUnless(hasattr(asyncio, 'ProactorEventLoop'), 'Windows only')
class ProactorStartServerTests(BaseStartServer, unittest.TestCase):
From 93571fe1f922b47996f171a8d7a41928ada0e9d7 Mon Sep 17 00:00:00 2001
From: Pierre Ossman <ossman@cendio.se>
Date: Fri, 27 Oct 2023 17:26:05 +0200
Subject: [PATCH 4/6] gh-111246: Add opt-out of Unix socket cleanup (#111246)
Allow the implicit cleanup to be disabled in case there are some odd
corner cases where this causes issues.
---
Doc/library/asyncio-eventloop.rst | 10 ++++++----
Lib/asyncio/unix_events.py | 17 +++++++++--------
Lib/test/test_asyncio/test_server.py | 11 +++++++++++
3 files changed, 26 insertions(+), 12 deletions(-)
diff --git a/Doc/library/asyncio-eventloop.rst b/Doc/library/asyncio-eventloop.rst
index 398bd1ab8ce268..8227ff7710484c 100644
--- a/Doc/library/asyncio-eventloop.rst
+++ b/Doc/library/asyncio-eventloop.rst
@@ -776,7 +776,7 @@ Creating network servers
*, sock=None, backlog=100, ssl=None, \
ssl_handshake_timeout=None, \
ssl_shutdown_timeout=None, \
- start_serving=True)
+ start_serving=True, cleanup_socket=True)
Similar to :meth:`loop.create_server` but works with the
:py:const:`~socket.AF_UNIX` socket family.
@@ -786,6 +786,10 @@ Creating network servers
:class:`str`, :class:`bytes`, and :class:`~pathlib.Path` paths
are supported.
+ If *cleanup_socket* is True then the Unix socket will automatically
+ be removed from the filesystem when the server is closed, unless the
+ socket has been replaced after the server has been created.
+
See the documentation of the :meth:`loop.create_server` method
for information about arguments to this method.
@@ -802,9 +806,7 @@ Creating network servers
.. versionchanged:: 3.13
- The Unix socket will automatically be removed from the filesystem
- when the server is closed, unless the socket has been replaced
- after the server has been created.
+ Added the *cleanup_socket* parameter.
.. coroutinemethod:: loop.connect_accepted_socket(protocol_factory, \
diff --git a/Lib/asyncio/unix_events.py b/Lib/asyncio/unix_events.py
index ba2a9e7fc57d9a..58f8a997182f00 100644
--- a/Lib/asyncio/unix_events.py
+++ b/Lib/asyncio/unix_events.py
@@ -285,7 +285,7 @@ async def create_unix_server(
sock=None, backlog=100, ssl=None,
ssl_handshake_timeout=None,
ssl_shutdown_timeout=None,
- start_serving=True):
+ start_serving=True, cleanup_socket=True):
if isinstance(ssl, bool):
raise TypeError('ssl argument must be an SSLContext or None')
@@ -341,13 +341,14 @@ async def create_unix_server(
raise ValueError(
f'A UNIX Domain Stream Socket was expected, got {sock!r}')
- path = sock.getsockname()
- # Check for abstract socket. `str` and `bytes` paths are supported.
- if path[0] not in (0, '\x00'):
- try:
- self._unix_server_sockets[sock] = os.stat(path).st_ino
- except FileNotFoundError:
- pass
+ if cleanup_socket:
+ path = sock.getsockname()
+ # Check for abstract socket. `str` and `bytes` paths are supported.
+ if path[0] not in (0, '\x00'):
+ try:
+ self._unix_server_sockets[sock] = os.stat(path).st_ino
+ except FileNotFoundError:
+ pass
sock.setblocking(False)
server = base_events.Server(self, [sock], protocol_factory,
diff --git a/Lib/test/test_asyncio/test_server.py b/Lib/test/test_asyncio/test_server.py
index b8d35f8d213293..d8ce3b43998665 100644
--- a/Lib/test/test_asyncio/test_server.py
+++ b/Lib/test/test_asyncio/test_server.py
@@ -205,6 +205,17 @@ async def serve(*args):
srv.close()
self.assertTrue(os.path.exists(addr))
+ @socket_helper.skip_unless_bind_unix_socket
+ async def test_unix_server_cleanup_prevented(self):
+ with test_utils.unix_socket_path() as addr:
+ async def serve(*args):
+ pass
+
+ srv = await asyncio.start_unix_server(serve, addr, cleanup_socket=False)
+
+ srv.close()
+ self.assertTrue(os.path.exists(addr))
+
@unittest.skipUnless(hasattr(asyncio, 'ProactorEventLoop'), 'Windows only')
class ProactorStartServerTests(BaseStartServer, unittest.TestCase):
From acadacbf57c83123d6c28ed291eb79735a49f936 Mon Sep 17 00:00:00 2001
From: Pierre Ossman <ossman@cendio.se>
Date: Mon, 6 Nov 2023 16:36:21 +0100
Subject: [PATCH 5/6] Mention gh issue in whatsnew
---
Doc/whatsnew/3.13.rst | 1 +
1 file changed, 1 insertion(+)
diff --git a/Doc/whatsnew/3.13.rst b/Doc/whatsnew/3.13.rst
index b3e635b9d567fc..e2af8f27f74988 100644
--- a/Doc/whatsnew/3.13.rst
+++ b/Doc/whatsnew/3.13.rst
@@ -149,6 +149,7 @@ asyncio
* :meth:`asyncio.loop.create_unix_server` will now automatically remove
the Unix socket when the server is closed.
+ (Contributed by Pierre Ossman in :gh:`111246`.)
copy
----
From 1db87b7ccd324e35d080715df3b3d8f0421efddc Mon Sep 17 00:00:00 2001
From: Pierre Ossman <ossman@cendio.se>
Date: Mon, 6 Nov 2023 16:37:52 +0100
Subject: [PATCH 6/6] Add comments to new unix server cleanup tests
---
Lib/test/test_asyncio/test_server.py | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/Lib/test/test_asyncio/test_server.py b/Lib/test/test_asyncio/test_server.py
index d8ce3b43998665..6b5c15b22bbea1 100644
--- a/Lib/test/test_asyncio/test_server.py
+++ b/Lib/test/test_asyncio/test_server.py
@@ -149,9 +149,11 @@ async def serve(*args):
await task2
+# Test the various corner cases of Unix server socket removal
class UnixServerCleanupTests(unittest.IsolatedAsyncioTestCase):
@socket_helper.skip_unless_bind_unix_socket
async def test_unix_server_addr_cleanup(self):
+ # Default scenario
with test_utils.unix_socket_path() as addr:
async def serve(*args):
pass
@@ -163,6 +165,7 @@ async def serve(*args):
@socket_helper.skip_unless_bind_unix_socket
async def test_unix_server_sock_cleanup(self):
+ # Using already bound socket
with test_utils.unix_socket_path() as addr:
async def serve(*args):
pass
@@ -177,6 +180,7 @@ async def serve(*args):
@socket_helper.skip_unless_bind_unix_socket
async def test_unix_server_cleanup_gone(self):
+ # Someone else has already cleaned up the socket
with test_utils.unix_socket_path() as addr:
async def serve(*args):
pass
@@ -192,6 +196,7 @@ async def serve(*args):
@socket_helper.skip_unless_bind_unix_socket
async def test_unix_server_cleanup_replaced(self):
+ # Someone else has replaced the socket with their own
with test_utils.unix_socket_path() as addr:
async def serve(*args):
pass
@@ -207,6 +212,7 @@ async def serve(*args):
@socket_helper.skip_unless_bind_unix_socket
async def test_unix_server_cleanup_prevented(self):
+ # Automatic cleanup explicitly disabled
with test_utils.unix_socket_path() as addr:
async def serve(*args):
pass