[Spice-devel] [PATCH 02/13] Free dispatchers at exit

[Jonathon Jongsma]

On Wed, 2016-03-23 at 12:46 -0400, Frediano Ziglio wrote:
> > 
> > On Wed, Mar 23, 2016 at 12:48:28PM +0000, Frediano Ziglio wrote:
> > > From: Jonathon Jongsma <jjongsma at redhat.com>
> > > 
> > > ---
> > >  server/red-qxl.c | 7 +++++++
> > >  server/red-qxl.h | 1 +
> > >  server/reds.c    | 2 ++
> > >  3 files changed, 10 insertions(+)
> > > 
> > > diff --git a/server/red-qxl.c b/server/red-qxl.c
> > > index 8012b26..5233afd 100644
> > > --- a/server/red-qxl.c
> > > +++ b/server/red-qxl.c
> > > @@ -939,6 +939,13 @@ void red_qxl_gl_draw_async_complete(QXLInstance *qxl)
> > >      red_qxl_async_complete(qxl, async);
> > >  }
> > >  
> > > +void red_qxl_free(QXLState *qxl_state)
> > > +{
> > > +    g_object_unref(qxl_state->dispatcher);
> > > +    /* FIXME: free other stuff */
> > > +    free(qxl_state);
> > > +}
> > > +
> > >  void red_qxl_init(RedsState *reds, QXLInstance *qxl)
> > >  {
> > >      QXLState *qxl_state;
> > > diff --git a/server/red-qxl.h b/server/red-qxl.h
> > > index 7287740..a61c0b4 100644
> > > --- a/server/red-qxl.h
> > > +++ b/server/red-qxl.h
> > > @@ -25,6 +25,7 @@ typedef struct QXLState QXLState;
> > >  typedef struct AsyncCommand AsyncCommand;
> > >  
> > >  void red_qxl_init(SpiceServer *reds, QXLInstance *qxl);
> > > +void red_qxl_free(QXLState *qxl_state);
> > >  
> > >  void red_qxl_set_mm_time(QXLInstance *qxl, uint32_t);
> > >  void red_qxl_on_ic_change(QXLInstance *qxl, SpiceImageCompression ic);
> > > diff --git a/server/reds.c b/server/reds.c
> > > index 254840b..0e9641b 100644
> > > --- a/server/reds.c
> > > +++ b/server/reds.c
> > > @@ -3505,9 +3505,11 @@ SPICE_GNUC_VISIBLE int
> > > spice_server_init(SpiceServer
> > > *reds, SpiceCoreInterface *
> > >  SPICE_GNUC_VISIBLE void spice_server_destroy(SpiceServer *reds)
> > >  {
> > >      g_array_unref(reds->renderers);
> > > +    g_object_unref(reds->main_dispatcher);
> > >      if (reds->main_channel) {
> > >          main_channel_close(reds->main_channel);
> > >      }
> > > +    g_list_free_full(reds->qxl_instances, (GDestroyNotify)red_qxl_free);
> > >      reds_cleanup(reds);
> > 
> > This is not correct, red_qxl_free() is:
> > void red_qxl_free(QXLState *qxl_state);
> > but you are passing it a QXLInstance.
> > 
> > Christophe
> > 
> 
> I proposed couple of times to remove this patch entirely and proposed a
> different
> way to free these stuff using reference counting in RedsState (that's the
> reason
> I didn't fix).
> I had no time to code what I proposed but I would like to see all freed 
> (RedWorker and so on) so we could use replay utility and a memory debugger to
> remove possible leaks.
> It's not clear by public interface who should free interfaces like QXL.
> Should the user (like Qemu) call spice_server_remove_interface or
> spice_server_destroy
> should tear down everything? Public API has no way to change reference
> counting of
> interfaces.

Yeah, I agree that it's not clear who should free all of this stuff. The
mismatch in types was probably caused by recent changes and rebasing old patches
on top? 

Anyway, I'm OK dropping this patch if you'd like to. It could certainly be done
in a better way, and in reality it doesn't matter much since all memory is freed
when a process exits anyway...

> 
> Frediano
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel