chardev: fix race with client connections in tcp_chr_wait_connected
commit4b47373a0d9a77e5a2099fd4b8d7d03b75523a36
authorDaniel P. Berrangé <berrange@redhat.com>
Mon, 11 Feb 2019 18:24:40 +0000 (11 18:24 +0000)
committerMarc-André Lureau <marcandre.lureau@redhat.com>
Tue, 12 Feb 2019 16:35:56 +0000 (12 17:35 +0100)
treeddd6f80f1a4549d6a03bc1ff6b012e823c95accd
parentd1885e549d16103f57df5306f3dd53c626a10d05
chardev: fix race with client connections in tcp_chr_wait_connected

When the 'reconnect' option is given for a client connection, the
qmp_chardev_open_socket_client method will run an asynchronous
connection attempt. The QIOChannel socket executes this is a single use
background thread, so the connection will succeed immediately (assuming
the server is listening). The chardev, however, won't get the result
from this background thread until the main loop starts running and
processes idle callbacks.

Thus when tcp_chr_wait_connected is run s->ioc will be NULL, but the
state will be TCP_CHARDEV_STATE_CONNECTING, and there may already
be an established connection that will be associated with the chardev
by the pending idle callback. tcp_chr_wait_connected doesn't check the
state, only s->ioc, so attempts to establish another connection
synchronously.

If the server allows multiple connections this is unhelpful but not a
fatal problem as the duplicate connection will get ignored by the
tcp_chr_new_client method when it sees the state is already connected.

If the server only supports a single connection, however, the
tcp_chr_wait_connected method will hang forever because the server will
not accept its synchronous connection attempt until the first connection
is closed.

To deal with this tcp_chr_wait_connected needs to synchronize with the
completion of the background connection task. To do this it needs to
create the QIOTask directly and use the qio_task_wait_thread method.
This will cancel the pending idle callback and directly dispatch the
task completion callback, allowing the connection to be associated
with the chardev. If the background connection failed, it can still
attempt a new synchronous connection.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Message-Id: <20190211182442.8542-15-berrange@redhat.com>
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
chardev/char-socket.c