[Spice-devel] [PATCH spice-server 3/5] red-qxl: Avoid using dandling pointers to RedClient

[Jonathon Jongsma]

In the summary: "dandling" -> "dangling"



On Fri, 2017-08-25 at 17:38 +0200, Christophe Fergeau wrote:
> On Thu, Aug 24, 2017 at 03:37:18PM +0100, Frediano Ziglio wrote:
> > A RedClient can be freed from the main thread following
> > a main channel disconnection.
> 
> I'd mention reds_client_disconnect or some concrete function name so
> that someone wanting to follow that code has something to start from.
> 
> > This can happen while
> > another thread is allocating a new channel client for
> > that client.
> > To prevent the usage of a pointer which can be invalid
> > take ownership of the pointer.
> > Note that we don't need this disconnecting as disconnection
> 
> "when disconnecting"
> 
> > is done synchronously (the dispatch messages are registered
> > with DISPATCH_ACK).
> 
> 
> > 
> > Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> > ---
> >  server/red-qxl.c    | 6 ++++++
> >  server/red-worker.c | 2 ++
> >  2 files changed, 8 insertions(+)
> > 
> > diff --git a/server/red-qxl.c b/server/red-qxl.c
> > index 53f3338b..168b9f0d 100644
> > --- a/server/red-qxl.c
> > +++ b/server/red-qxl.c
> > @@ -83,6 +83,9 @@ static void red_qxl_set_display_peer(RedChannel
> > *channel, RedClient *client,
> >  
> >      spice_debug("%s", "");
> >      dispatcher = (Dispatcher
> > *)g_object_get_data(G_OBJECT(channel), "dispatcher");
> > +    // get a reference potentially the main channel can be
> > destroyed in
> > +    // the main thread causing RedClient to be destroyed before
> > using it
> > +    g_object_ref(client);
> >      payload.client = client;
> 
> payload.client = g_object_ref(client); works too.
> 
> 
> Acked-by: Christophe Fergeau <cfergeau at redhat.com>
> 
> >      payload.stream = stream;
> >      payload.migration = migration;
> > @@ -141,6 +144,9 @@ static void red_qxl_set_cursor_peer(RedChannel
> > *channel, RedClient *client, Reds
> >      RedWorkerMessageCursorConnect payload = {0,};
> >      Dispatcher *dispatcher = (Dispatcher
> > *)g_object_get_data(G_OBJECT(channel), "dispatcher");
> >      spice_printerr("");
> > +    // get a reference potentially the main channel can be
> > destroyed in
> > +    // the main thread causing RedClient to be destroyed before
> > using it
> > +    g_object_ref(client);
> >      payload.client = client;
> >      payload.stream = stream;
> >      payload.migration = migration;
> > diff --git a/server/red-worker.c b/server/red-worker.c
> > index 8fd964ea..fa69da2f 100644
> > --- a/server/red-worker.c
> > +++ b/server/red-worker.c
> > @@ -724,6 +724,7 @@ static void handle_dev_display_connect(void
> > *opaque, void *payload)
> >  
> >      dcc = dcc_new(display, msg->client, msg->stream, msg-
> > >migration, &msg->caps,
> >                    worker->image_compression, worker->jpeg_state,
> > worker->zlib_glz_state);
> > +    g_object_unref(msg->client);
> >      red_channel_capabilities_reset(&msg->caps);
> >      if (!dcc) {
> >          return;
> > @@ -816,6 +817,7 @@ static void handle_dev_cursor_connect(void
> > *opaque, void *payload)
> >      cursor_channel_connect(worker->cursor_channel,
> >                             msg->client, msg->stream, msg-
> > >migration,
> >                             &msg->caps);
> > +    g_object_unref(msg->client);
> >      red_channel_capabilities_reset(&msg->caps);
> >  }
> >  
> > -- 
> > 2.13.5
> > 
> > _______________________________________________
> > Spice-devel mailing list
> > Spice-devel at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/spice-devel
> 
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel