Skip to content

Commit

Permalink
python/aqmp: add socket bind step to legacy.py
Browse files Browse the repository at this point in the history
The synchronous QMP library would bind to the server address during
__init__(). The new library delays this to the accept() call, because
binding occurs inside of the call to start_[unix_]server(), which is an
async method -- so it cannot happen during __init__ anymore.

Python 3.7+ adds the ability to create the server (and thus the bind()
call) and begin the active listening in separate steps, but we don't
have that functionality in 3.6, our current minimum.

Therefore ... Add a temporary workaround that allows the synchronous
version of the client to bind the socket in advance, guaranteeing that
there will be a UNIX socket in the filesystem ready for the QEMU client
to connect to without a race condition.

(Yes, it's a bit ugly. Fixing it more nicely will have to wait until our
minimum Python version is 3.7+.)

Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Message-id: 20220201041134.1237016-5-jsnow@redhat.com
Signed-off-by: John Snow <jsnow@redhat.com>
  • Loading branch information
jnsnow committed Feb 2, 2022
1 parent 74a1505 commit b0b662b
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 3 deletions.
3 changes: 3 additions & 0 deletions python/qemu/aqmp/legacy.py
Expand Up @@ -56,6 +56,9 @@ def __init__(self, address: SocketAddrT,
self._address = address
self._timeout: Optional[float] = None

if server:
self._aqmp._bind_hack(address) # pylint: disable=protected-access

_T = TypeVar('_T')

def _sync(
Expand Down
41 changes: 38 additions & 3 deletions python/qemu/aqmp/protocol.py
Expand Up @@ -15,6 +15,7 @@
from enum import Enum
from functools import wraps
import logging
import socket
from ssl import SSLContext
from typing import (
Any,
Expand Down Expand Up @@ -238,6 +239,9 @@ 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

def __repr__(self) -> str:
cls_name = type(self).__name__
tokens = []
Expand Down Expand Up @@ -427,6 +431,34 @@ async def _establish_connection(
else:
await self._do_connect(address, ssl)

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_accept(self, address: SocketAddrT,
ssl: Optional[SSLContext] = None) -> None:
Expand Down Expand Up @@ -464,24 +496,27 @@ async def _client_connected_cb(reader: asyncio.StreamReader,
if isinstance(address, tuple):
coro = asyncio.start_server(
_client_connected_cb,
host=address[0],
port=address[1],
host=None if self._sock else address[0],
port=None if self._sock else address[1],
ssl=ssl,
backlog=1,
limit=self._limit,
sock=self._sock,
)
else:
coro = asyncio.start_unix_server(
_client_connected_cb,
path=address,
path=None if self._sock else address,
ssl=ssl,
backlog=1,
limit=self._limit,
sock=self._sock,
)

server = await coro # Starts listening
await connected.wait() # Waits for the callback to fire (and finish)
assert server is None
self._sock = None

self.logger.debug("Connection accepted.")

Expand Down

0 comments on commit b0b662b

Please sign in to comment.