_bind_hack() was a quick fix to allow async QMP to call bind(2) prior to calling listen(2) and accept(2). This wasn't sufficient to fully address the race condition present in synchronous clients.
With the race condition in legacy.py fixed (see the previous commit), there are no longer any users of _bind_hack(). Drop it. Fixes: b0b662bb2b3 Signed-off-by: John Snow <js...@redhat.com> Acked-by: Kevin Wolf <kw...@redhat.com> Reviewed-by: Daniel P. Berrangé <berra...@redhat.com> Message-id: 20220225205948.3693480-11-js...@redhat.com [Expanded commit message. --js] Signed-off-by: John Snow <js...@redhat.com> --- python/qemu/aqmp/legacy.py | 2 +- python/qemu/aqmp/protocol.py | 41 +++--------------------------------- 2 files changed, 4 insertions(+), 39 deletions(-) diff --git a/python/qemu/aqmp/legacy.py b/python/qemu/aqmp/legacy.py index cb50e60564..46026e9fdc 100644 --- a/python/qemu/aqmp/legacy.py +++ b/python/qemu/aqmp/legacy.py @@ -57,7 +57,7 @@ def __init__(self, address: SocketAddrT, self._timeout: Optional[float] = None if server: - self._sync(self._aqmp.start_server(address)) + self._sync(self._aqmp.start_server(self._address)) _T = TypeVar('_T') diff --git a/python/qemu/aqmp/protocol.py b/python/qemu/aqmp/protocol.py index 2ecba14555..36fae57f27 100644 --- a/python/qemu/aqmp/protocol.py +++ b/python/qemu/aqmp/protocol.py @@ -18,7 +18,6 @@ from enum import Enum from functools import wraps import logging -import socket from ssl import SSLContext from typing import ( Any, @@ -242,9 +241,6 @@ def __init__(self, name: Optional[str] = None) -> None: self._runstate = Runstate.IDLE self._runstate_changed: Optional[asyncio.Event] = None - # Workaround for bind() - self._sock: Optional[socket.socket] = None - # Server state for start_server() and _incoming() self._server: Optional[asyncio.AbstractServer] = None self._accepted: Optional[asyncio.Event] = None @@ -535,34 +531,6 @@ async def _incoming(self, self._reader, self._writer = (reader, writer) self._accepted.set() - def _bind_hack(self, address: Union[str, Tuple[str, int]]) -> None: - """ - Used to create a socket in advance of accept(). - - This is a workaround to ensure that we can guarantee timing of - precisely when a socket exists to avoid a connection attempt - bouncing off of nothing. - - Python 3.7+ adds a feature to separate the server creation and - listening phases instead, and should be used instead of this - hack. - """ - if isinstance(address, tuple): - family = socket.AF_INET - else: - family = socket.AF_UNIX - - sock = socket.socket(family, socket.SOCK_STREAM) - sock.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1) - - try: - sock.bind(address) - except: - sock.close() - raise - - self._sock = sock - @upper_half async def _do_start_server(self, address: SocketAddrT, ssl: Optional[SSLContext] = None) -> None: @@ -589,21 +557,19 @@ async def _do_start_server(self, address: SocketAddrT, if isinstance(address, tuple): coro = asyncio.start_server( self._incoming, - host=None if self._sock else address[0], - port=None if self._sock else address[1], + host=address[0], + port=address[1], ssl=ssl, backlog=1, limit=self._limit, - sock=self._sock, ) else: coro = asyncio.start_unix_server( self._incoming, - path=None if self._sock else address, + path=address, ssl=ssl, backlog=1, limit=self._limit, - sock=self._sock, ) # Allow runstate watchers to witness 'CONNECTING' state; some @@ -630,7 +596,6 @@ async def _do_accept(self) -> None: await self._accepted.wait() assert self._server is None self._accepted = None - self._sock = None self.logger.debug("Connection accepted.") -- 2.34.1