python/aqmp: drop _bind_hack()
authorJohn Snow <jsnow@redhat.com>
Fri, 25 Feb 2022 20:59:48 +0000 (15:59 -0500)
committerJohn Snow <jsnow@redhat.com>
Mon, 7 Mar 2022 19:36:41 +0000 (14:36 -0500)
_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 <jsnow@redhat.com>
Acked-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Message-id: 20220225205948.3693480-11-jsnow@redhat.com
[Expanded commit message. --js]
Signed-off-by: John Snow <jsnow@redhat.com>
python/qemu/aqmp/legacy.py
python/qemu/aqmp/protocol.py

index cb50e60564823fdc0aeeb194c5e3d41cfc09b1ed..46026e9fdc6c8859a8c944076c7166b5f6ed671e 100644 (file)
@@ -57,7 +57,7 @@ class QEMUMonitorProtocol(qemu.qmp.QEMUMonitorProtocol):
         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')
 
index 2ecba1455571a35e0e6c565e3641d8a26e8d8b76..36fae57f277826d5cd4026e0d079964cb9ed29b7 100644 (file)
@@ -18,7 +18,6 @@ from asyncio import StreamReader, StreamWriter
 from enum import Enum
 from functools import wraps
 import logging
-import socket
 from ssl import SSLContext
 from typing import (
     Any,
@@ -242,9 +241,6 @@ class AsyncProtocol(Generic[T]):
         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 @@ class AsyncProtocol(Generic[T]):
         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 @@ class AsyncProtocol(Generic[T]):
         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 @@ class AsyncProtocol(Generic[T]):
         await self._accepted.wait()
         assert self._server is None
         self._accepted = None
-        self._sock = None
 
         self.logger.debug("Connection accepted.")