From 5153c7e15b5e453b862c964fd35260e3895ef9ae Mon Sep 17 00:00:00 2001 From: Pablo Galindo Date: Mon, 9 Mar 2020 01:44:00 +0000 Subject: [PATCH 1/7] bpo-39850: Add support for abstract sockets in multiprocessing --- Lib/multiprocessing/connection.py | 10 +++++++-- Lib/multiprocessing/forkserver.py | 6 ++++-- Lib/multiprocessing/managers.py | 6 +++++- Lib/multiprocessing/util.py | 21 +++++++++++++++++++ Lib/test/_test_multiprocessing.py | 13 ++++++++++++ .../2020-03-09-01-45-06.bpo-39850.eaJNIE.rst | 5 +++++ 6 files changed, 56 insertions(+), 5 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2020-03-09-01-45-06.bpo-39850.eaJNIE.rst diff --git a/Lib/multiprocessing/connection.py b/Lib/multiprocessing/connection.py index c9f995e5fa7f57..bd8f9505e56b35 100644 --- a/Lib/multiprocessing/connection.py +++ b/Lib/multiprocessing/connection.py @@ -73,6 +73,11 @@ def arbitrary_address(family): if family == 'AF_INET': return ('localhost', 0) elif family == 'AF_UNIX': + # Prefer abstract sockets if possible to avoid problems with the address + # size. When coding portable applications, some implementations have + # sun_path as short as 92 bytes in the sockaddr_un struct. + if util.abstract_sockets_supported: + return f"\0listener-{os.getpid()}-{next(_mmap_counter)}" return tempfile.mktemp(prefix='listener-', dir=util.get_temp_dir()) elif family == 'AF_PIPE': return tempfile.mktemp(prefix=r'\\.\pipe\pyc-%d-%d-' % @@ -102,7 +107,7 @@ def address_type(address): return 'AF_INET' elif type(address) is str and address.startswith('\\\\'): return 'AF_PIPE' - elif type(address) is str: + elif type(address) is str or util.is_abstract_socket_namespace(address): return 'AF_UNIX' else: raise ValueError('address type of %r unrecognized' % address) @@ -597,7 +602,8 @@ def __init__(self, address, family, backlog=1): self._family = family self._last_accepted = None - if family == 'AF_UNIX': + if family == 'AF_UNIX' and not util.is_abstract_socket_namespace(address): + # Linux abstract socket namespaces do not need to be unlinked self._unlink = util.Finalize( self, os.unlink, args=(address,), exitpriority=0 ) diff --git a/Lib/multiprocessing/forkserver.py b/Lib/multiprocessing/forkserver.py index 87ebef6588acd3..215ac39afcaa07 100644 --- a/Lib/multiprocessing/forkserver.py +++ b/Lib/multiprocessing/forkserver.py @@ -55,7 +55,8 @@ def _stop_unlocked(self): os.waitpid(self._forkserver_pid, 0) self._forkserver_pid = None - os.unlink(self._forkserver_address) + if not util.is_abstract_socket_namespace(self._forkserver_address): + os.unlink(self._forkserver_address) self._forkserver_address = None def set_forkserver_preload(self, modules_names): @@ -135,7 +136,8 @@ def ensure_running(self): with socket.socket(socket.AF_UNIX) as listener: address = connection.arbitrary_address('AF_UNIX') listener.bind(address) - os.chmod(address, 0o600) + if not util.is_abstract_socket_namespace(address): + os.chmod(address, 0o600) listener.listen() # all client processes own the write end of the "alive" pipe; diff --git a/Lib/multiprocessing/managers.py b/Lib/multiprocessing/managers.py index 1f9c2daa25d97f..110d3925cdd8e8 100644 --- a/Lib/multiprocessing/managers.py +++ b/Lib/multiprocessing/managers.py @@ -1262,8 +1262,12 @@ class SharedMemoryServer(Server): def __init__(self, *args, **kwargs): Server.__init__(self, *args, **kwargs) + address = self.address + # The address of Linux abstract namespaces can be bytes + if isinstance(address, bytes): + address = address.decode() self.shared_memory_context = \ - _SharedMemoryTracker(f"shmm_{self.address}_{getpid()}") + _SharedMemoryTracker(f"shmm_{address}_{getpid()}") util.debug(f"SharedMemoryServer started by pid {getpid()}") def create(self, c, typeid, /, *args, **kwargs): diff --git a/Lib/multiprocessing/util.py b/Lib/multiprocessing/util.py index 4bc7782c00c158..c80c52b05b9242 100644 --- a/Lib/multiprocessing/util.py +++ b/Lib/multiprocessing/util.py @@ -102,6 +102,27 @@ def log_to_stderr(level=None): _log_to_stderr = True return _logger + +# Abstract socket support + +def _platform_supports_abstract_sockets(): + if "linux" in sys.platform: + return True + if hasattr(sys, 'getandroidapilevel'): + return True + return False + + +def is_abstract_socket_namespace(address): + if isinstance(address, bytes): + return address[0] == 0 + elif isinstance(address, str): + return address[0] == "\0" + raise ValueError('address type of %r unrecognized' % address) + + +abstract_sockets_supported = _platform_supports_abstract_sockets() + # # Function returning a temp directory which will be removed on exit # diff --git a/Lib/test/_test_multiprocessing.py b/Lib/test/_test_multiprocessing.py index 73dc75d34a6f01..e7020c616fb215 100644 --- a/Lib/test/_test_multiprocessing.py +++ b/Lib/test/_test_multiprocessing.py @@ -3274,6 +3274,19 @@ def test_context(self): if self.TYPE == 'processes': self.assertRaises(OSError, l.accept) + @unittest.skipUnless(util.abstract_sockets_supported, + "test needs abstract socket support") + def test_abstract_socket(self): + with self.connection.Listener("\0something") as l: + with self.connection.Client(l.address) as c: + with l.accept() as d: + c.send(1729) + self.assertEqual(d.recv(), 1729) + + if self.TYPE == 'processes': + self.assertRaises(OSError, l.accept) + + class _TestListenerClient(BaseTestCase): ALLOWED_TYPES = ('processes', 'threads') diff --git a/Misc/NEWS.d/next/Library/2020-03-09-01-45-06.bpo-39850.eaJNIE.rst b/Misc/NEWS.d/next/Library/2020-03-09-01-45-06.bpo-39850.eaJNIE.rst new file mode 100644 index 00000000000000..09c87be57d1e0b --- /dev/null +++ b/Misc/NEWS.d/next/Library/2020-03-09-01-45-06.bpo-39850.eaJNIE.rst @@ -0,0 +1,5 @@ +:mod:`multiprocessing` now supports abstract socket addresses (if abstract sockets +are supported in the running platform). When creating arbitrary addresses (like when +default-constructing :class:`multiprocessing.connection.Listener` objects) abstract +sockets are preferred to avoid the case when the temporary-file-generated address is +too large for an AF_UNIX socket address). Patch by Pablo Galindo. From 5957b9bcbee01fa4c8dadd3b55170c348daea6fc Mon Sep 17 00:00:00 2001 From: Pablo Galindo Date: Mon, 9 Mar 2020 10:17:58 +0000 Subject: [PATCH 2/7] Update Lib/multiprocessing/util.py Co-Authored-By: Victor Stinner --- Lib/multiprocessing/util.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/multiprocessing/util.py b/Lib/multiprocessing/util.py index c80c52b05b9242..8bb8c3f82f976a 100644 --- a/Lib/multiprocessing/util.py +++ b/Lib/multiprocessing/util.py @@ -118,7 +118,7 @@ def is_abstract_socket_namespace(address): return address[0] == 0 elif isinstance(address, str): return address[0] == "\0" - raise ValueError('address type of %r unrecognized' % address) + raise TypeError('address type of {address!r} unrecognized') abstract_sockets_supported = _platform_supports_abstract_sockets() From 9a862b5f2d7feb6efa9595d995718a04a68a98d0 Mon Sep 17 00:00:00 2001 From: Pablo Galindo Date: Mon, 9 Mar 2020 10:18:22 +0000 Subject: [PATCH 3/7] Apply suggestions from code review Co-Authored-By: Victor Stinner --- Lib/test/_test_multiprocessing.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Lib/test/_test_multiprocessing.py b/Lib/test/_test_multiprocessing.py index e7020c616fb215..e7fd58d20f9a0b 100644 --- a/Lib/test/_test_multiprocessing.py +++ b/Lib/test/_test_multiprocessing.py @@ -3277,8 +3277,8 @@ def test_context(self): @unittest.skipUnless(util.abstract_sockets_supported, "test needs abstract socket support") def test_abstract_socket(self): - with self.connection.Listener("\0something") as l: - with self.connection.Client(l.address) as c: + with self.connection.Listener("\0something") as listener: + with self.connection.Client(l.address) as client: with l.accept() as d: c.send(1729) self.assertEqual(d.recv(), 1729) From a8017bee2b482bacd78c68c01f601bd4e6906038 Mon Sep 17 00:00:00 2001 From: Pablo Galindo Date: Mon, 9 Mar 2020 10:19:57 +0000 Subject: [PATCH 4/7] Address feedback --- Lib/multiprocessing/util.py | 4 +++- Lib/test/_test_multiprocessing.py | 6 +++--- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/Lib/multiprocessing/util.py b/Lib/multiprocessing/util.py index 8bb8c3f82f976a..32c7a96d2534d9 100644 --- a/Lib/multiprocessing/util.py +++ b/Lib/multiprocessing/util.py @@ -106,7 +106,7 @@ def log_to_stderr(level=None): # Abstract socket support def _platform_supports_abstract_sockets(): - if "linux" in sys.platform: + if sys.platform == "linux": return True if hasattr(sys, 'getandroidapilevel'): return True @@ -114,6 +114,8 @@ def _platform_supports_abstract_sockets(): def is_abstract_socket_namespace(address): + if not address: + return False if isinstance(address, bytes): return address[0] == 0 elif isinstance(address, str): diff --git a/Lib/test/_test_multiprocessing.py b/Lib/test/_test_multiprocessing.py index e7fd58d20f9a0b..b48a101199ccd4 100644 --- a/Lib/test/_test_multiprocessing.py +++ b/Lib/test/_test_multiprocessing.py @@ -3279,12 +3279,12 @@ def test_context(self): def test_abstract_socket(self): with self.connection.Listener("\0something") as listener: with self.connection.Client(l.address) as client: - with l.accept() as d: - c.send(1729) + with listener.accept() as d: + client.send(1729) self.assertEqual(d.recv(), 1729) if self.TYPE == 'processes': - self.assertRaises(OSError, l.accept) + self.assertRaises(OSError, listener.accept) class _TestListenerClient(BaseTestCase): From 25ac99094dd9d7b033b2f73a059ec9fb4e9c9c95 Mon Sep 17 00:00:00 2001 From: Pablo Galindo Date: Mon, 9 Mar 2020 11:13:32 +0000 Subject: [PATCH 5/7] Remove extra 'm' --- Lib/multiprocessing/managers.py | 2 +- Lib/test/_test_multiprocessing.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Lib/multiprocessing/managers.py b/Lib/multiprocessing/managers.py index 110d3925cdd8e8..7668040cec773f 100644 --- a/Lib/multiprocessing/managers.py +++ b/Lib/multiprocessing/managers.py @@ -1267,7 +1267,7 @@ def __init__(self, *args, **kwargs): if isinstance(address, bytes): address = address.decode() self.shared_memory_context = \ - _SharedMemoryTracker(f"shmm_{address}_{getpid()}") + _SharedMemoryTracker(f"shm_{address}_{getpid()}") util.debug(f"SharedMemoryServer started by pid {getpid()}") def create(self, c, typeid, /, *args, **kwargs): diff --git a/Lib/test/_test_multiprocessing.py b/Lib/test/_test_multiprocessing.py index b48a101199ccd4..b985d51508cb75 100644 --- a/Lib/test/_test_multiprocessing.py +++ b/Lib/test/_test_multiprocessing.py @@ -3278,7 +3278,7 @@ def test_context(self): "test needs abstract socket support") def test_abstract_socket(self): with self.connection.Listener("\0something") as listener: - with self.connection.Client(l.address) as client: + with self.connection.Client(listener.address) as client: with listener.accept() as d: client.send(1729) self.assertEqual(d.recv(), 1729) From e062f12d369135ab5d3ba7fef84867d95db09a08 Mon Sep 17 00:00:00 2001 From: Pablo Galindo Date: Mon, 9 Mar 2020 13:15:25 +0000 Subject: [PATCH 6/7] fixup! Remove extra 'm' --- Lib/multiprocessing/connection.py | 2 +- .../next/Library/2020-03-09-01-45-06.bpo-39850.eaJNIE.rst | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Lib/multiprocessing/connection.py b/Lib/multiprocessing/connection.py index bd8f9505e56b35..510e4b5aba44a6 100644 --- a/Lib/multiprocessing/connection.py +++ b/Lib/multiprocessing/connection.py @@ -603,7 +603,7 @@ def __init__(self, address, family, backlog=1): self._last_accepted = None if family == 'AF_UNIX' and not util.is_abstract_socket_namespace(address): - # Linux abstract socket namespaces do not need to be unlinked + # Linux abstract socket namespaces do not need to be explicitly unlinked self._unlink = util.Finalize( self, os.unlink, args=(address,), exitpriority=0 ) diff --git a/Misc/NEWS.d/next/Library/2020-03-09-01-45-06.bpo-39850.eaJNIE.rst b/Misc/NEWS.d/next/Library/2020-03-09-01-45-06.bpo-39850.eaJNIE.rst index 09c87be57d1e0b..bb00dd8eb4b2e9 100644 --- a/Misc/NEWS.d/next/Library/2020-03-09-01-45-06.bpo-39850.eaJNIE.rst +++ b/Misc/NEWS.d/next/Library/2020-03-09-01-45-06.bpo-39850.eaJNIE.rst @@ -2,4 +2,4 @@ are supported in the running platform). When creating arbitrary addresses (like when default-constructing :class:`multiprocessing.connection.Listener` objects) abstract sockets are preferred to avoid the case when the temporary-file-generated address is -too large for an AF_UNIX socket address). Patch by Pablo Galindo. +too large for an AF_UNIX socket address. Patch by Pablo Galindo. From 24710867235fe535d658c90afd03d8809afc5fee Mon Sep 17 00:00:00 2001 From: Pablo Galindo Date: Mon, 9 Mar 2020 13:17:52 +0000 Subject: [PATCH 7/7] Use os.fsdecode() --- Lib/multiprocessing/managers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/multiprocessing/managers.py b/Lib/multiprocessing/managers.py index 7668040cec773f..1668220c099571 100644 --- a/Lib/multiprocessing/managers.py +++ b/Lib/multiprocessing/managers.py @@ -1265,7 +1265,7 @@ def __init__(self, *args, **kwargs): address = self.address # The address of Linux abstract namespaces can be bytes if isinstance(address, bytes): - address = address.decode() + address = os.fsdecode(address) self.shared_memory_context = \ _SharedMemoryTracker(f"shm_{address}_{getpid()}") util.debug(f"SharedMemoryServer started by pid {getpid()}")