[Spice-devel] [PATCH 07/19] Free dispatchers at exit

[Frediano Ziglio]

> 
> On Fri, 2016-02-19 at 06:54 -0500, Frediano Ziglio wrote:
> > > 
> > > From: Jonathon Jongsma <jjongsma at redhat.com>
> > > 
> > > ---
> > >  server/red-dispatcher.c | 7 +++++++
> > >  server/red-dispatcher.h | 1 +
> > >  server/reds.c           | 2 ++
> > >  3 files changed, 10 insertions(+)
> > > 
> > > diff --git a/server/red-dispatcher.c b/server/red-dispatcher.c
> > > index 4b839a9..0425d6c 100644
> > > --- a/server/red-dispatcher.c
> > > +++ b/server/red-dispatcher.c
> > > @@ -904,6 +904,13 @@ void red_dispatcher_async_complete(struct
> > > RedDispatcher
> > > *dispatcher,
> > >      free(async_command);
> > >  }
> > >  
> > > +void red_dispatcher_free(RedDispatcher *self)
> > > +{
> > > +    g_object_unref(self->dispatcher);
> > 
> > See comment on previous patch!
> > 
> > (1) ... here you are shooting at your feet!
> 
> sorry, can you explain a bit more here?
> 

I replied on the previous mail. See reference counting.

> > 
> > > +    /* FIXME: free other stuff */
> > > +    free(self);
> > > +}
> > > +
> > >  void red_dispatcher_init(RedsState *reds, QXLInstance *qxl)
> > >  {
> > >      RedDispatcher *red_dispatcher;
> > > diff --git a/server/red-dispatcher.h b/server/red-dispatcher.h
> > > index 14a8079..2fe8cd9 100644
> > > --- a/server/red-dispatcher.h
> > > +++ b/server/red-dispatcher.h
> > > @@ -26,6 +26,7 @@ typedef struct RedDispatcher RedDispatcher;
> > >  typedef struct AsyncCommand AsyncCommand;
> > >  
> > >  void red_dispatcher_init(SpiceServer *reds, QXLInstance *qxl);
> > > +void red_dispatcher_free(RedDispatcher *self);
> > >  
> > >  void red_dispatcher_set_mm_time(RedDispatcher *dispatcher, uint32_t);
> > >  void red_dispatcher_on_ic_change(RedDispatcher *dispatcher,
> > >  SpiceImageCompression ic);
> > > diff --git a/server/reds.c b/server/reds.c
> > > index d1b7d3b..b784310 100644
> > > --- a/server/reds.c
> > > +++ b/server/reds.c
> > > @@ -2757,9 +2757,11 @@ static int reds_init_ssl(RedsState *reds)
> > >  static void reds_destroy_internal(RedsState *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->dispatchers,
> > > (GDestroyNotify)red_dispatcher_free);
> > >  #ifdef RED_STATISTICS
> > >      if (reds->stat_shm_name) {
> > >          shm_unlink(reds->stat_shm_name);
> > 
> > I'm wondering if would be better to add reference counting to RedsState
> > instead and automatically free RedsState when reference reach 0 instead.
> > You could still free the statistic files before program ends.
> > 
> > The correct way to run code at exit on libraries is to use destructor
> > attribute, not calling atexit.
> 
> I agree, My eventual goal was to make RedsState a gobject. But I wanted to
> convert things one at a time.
> 

Could be that's easier to convert starting from the botton of the hierarchy
instead of the bottom.

> Jonathon
> 
> 

Frediano