On Wed, May 23, 2018 at 01:36:35PM -0400, Frediano Ziglio wrote:
> Testing this, so far is working correctly:
>
> Subject: [PATCH spice-server] rcc: Make red_channel_client_is_connected return
> correct information during initialization
>
> Use stream->watch as flag to check if connected or not initializing
> it during construction, not during init (after all hierarchy is built).
If there are any failures during red_channel_client_initable_init(),
red_channel_client_is_connected() will still return TRUE with this
patch.
In general, we should consider RedChannelClient and its derivative to be
non-functional until red_channel_client_initable_init() successfully
returns. This change makes RedChannelClient::is_connected become TRUE
much earlier in the instantiation process, imo we should not make the
object "look ready" until red_channel_client_initable_init() has
returned, I'm worried we'd end up doing some eg network operations
because the socket looks ready, while some other important things for
the channel haven't been initialized yet. SndChannelClient is the only
class trying to do network operations, or 'complex' stuff as part of its
instantiation btw, maybe I should move more of it to _initable_init.
With that said, and apart from that "early init" issue, the patch
makes sense.
Christophe
>
> Signed-off-by: Frediano Ziglio <fziglio@xxxxxxxxxx>
> ---
> server/red-channel-client.c | 33 ++++++++++++++++++++++-----------
> 1 file changed, 22 insertions(+), 11 deletions(-)
>
> diff --git a/server/red-channel-client.c b/server/red-channel-client.c
> index 893a764d..0057108f 100644
> --- a/server/red-channel-client.c
> +++ b/server/red-channel-client.c
> @@ -173,6 +173,7 @@ static void red_channel_client_clear_sent_item(RedChannelClient *rcc);
> static void red_channel_client_initable_interface_init(GInitableIface *iface);
> static void red_channel_client_set_message_serial(RedChannelClient *channel, uint64_t);
> static bool red_channel_client_config_socket(RedChannelClient *rcc);
> +static void red_channel_client_event(int fd, int event, void *data);
>
> /*
> * When an error occurs over a channel, we treat it as a warning
> @@ -293,6 +294,22 @@ red_channel_client_get_property(GObject *object,
> }
> }
>
> +static void
> +red_channel_client_init_watch(RedChannelClient *rcc)
> +{
> + if (!rcc->priv->stream || rcc->priv->stream->watch || !rcc->priv->channel) {
> + return;
> + }
> +
> + SpiceCoreInterfaceInternal *core = red_channel_get_core_interface(rcc->priv->channel);
> + red_stream_set_core_interface(rcc->priv->stream, core);
> + rcc->priv->stream->watch =
> + core->watch_add(core, rcc->priv->stream->socket,
> + SPICE_WATCH_EVENT_READ,
> + red_channel_client_event,
> + rcc);
> +}
> +
> static void
> red_channel_client_set_property(GObject *object,
> guint property_id,
> @@ -305,11 +322,13 @@ red_channel_client_set_property(GObject *object,
> {
> case PROP_STREAM:
> self->priv->stream = g_value_get_pointer(value);
> + red_channel_client_init_watch(self);
> break;
> case PROP_CHANNEL:
> if (self->priv->channel)
> g_object_unref(self->priv->channel);
> self->priv->channel = g_value_dup_object(value);
> + red_channel_client_init_watch(self);
> break;
> case PROP_CLIENT:
> self->priv->client = g_value_get_object(value);
> @@ -913,7 +932,6 @@ static gboolean red_channel_client_initable_init(GInitable *initable,
> GError **error)
> {
> GError *local_error = NULL;
> - SpiceCoreInterfaceInternal *core;
> RedChannelClient *self = RED_CHANNEL_CLIENT(initable);
>
> if (!self->priv->stream) {
> @@ -932,16 +950,10 @@ static gboolean red_channel_client_initable_init(GInitable *initable,
> goto cleanup;
> }
>
> - core = red_channel_get_core_interface(self->priv->channel);
> - red_stream_set_core_interface(self->priv->stream, core);
> - self->priv->stream->watch =
> - core->watch_add(core, self->priv->stream->socket,
> - SPICE_WATCH_EVENT_READ,
> - red_channel_client_event,
> - self);
> -
> if (self->priv->monitor_latency
> && red_stream_get_family(self->priv->stream) != AF_UNIX) {
> + SpiceCoreInterfaceInternal *core = red_channel_get_core_interface(self->priv->channel);
> +
> self->priv->latency_monitor.timer =
> core->timer_add(core, red_channel_client_ping_timer, self);
>
> @@ -1665,8 +1677,7 @@ bool red_channel_client_is_mini_header(RedChannelClient *rcc)
>
> gboolean red_channel_client_is_connected(RedChannelClient *rcc)
> {
> - return rcc->priv->channel
> - && (g_list_find(red_channel_get_clients(rcc->priv->channel), rcc) != NULL);
> + return rcc->priv->stream->watch != NULL;
> }
>
> static void red_channel_client_clear_sent_item(RedChannelClient *rcc)
> --
>
>
> Frediano
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel