diff mbox series

[10/10] python/aqmp: drop _bind_hack()

Message ID 20220225205948.3693480-11-jsnow@redhat.com
State New
Headers show
Series Python: Fix qmp race condition on accept() | expand

Commit Message

John Snow Feb. 25, 2022, 8:59 p.m. UTC
With the better fix in place, we don't need this anymore.

Fixes: b0b662bb2b3
Signed-off-by: John Snow <jsnow@redhat.com>
---
 python/qemu/aqmp/legacy.py   |  2 +-
 python/qemu/aqmp/protocol.py | 41 +++---------------------------------
 2 files changed, 4 insertions(+), 39 deletions(-)

Comments

Daniel P. Berrangé March 4, 2022, 6:03 p.m. UTC | #1
On Fri, Feb 25, 2022 at 03:59:48PM -0500, John Snow wrote:
> With the better fix in place, we don't need this anymore.

"the better fix" being the previous patch in this series. Perhaps bit
a little more verbose here.

> 
> Fixes: b0b662bb2b3
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>  python/qemu/aqmp/legacy.py   |  2 +-
>  python/qemu/aqmp/protocol.py | 41 +++---------------------------------
>  2 files changed, 4 insertions(+), 39 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


Regards,
Daniel
diff mbox series

Patch

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.")