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

[Frediano Ziglio]

> 
> 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.

Frediano