[Spice-devel] [spice-server] reds: Call sasl_server_done() when destroying SpiceServer instance

[Christophe Fergeau]

On Thu, Jul 19, 2018 at 10:18:47AM -0400, Frediano Ziglio wrote:
> > 
> > > 
> > > This call will free most of the memory allocated by sasl_server_init().
> > > It's refcounted so should be safe to call from a library.
> > > ---
> > >  server/reds.c | 1 +
> > >  1 file changed, 1 insertion(+)
> > > 
> > > diff --git a/server/reds.c b/server/reds.c
> > > index 85043a88d..e195ce611 100644
> > > --- a/server/reds.c
> > > +++ b/server/reds.c
> > > @@ -3682,6 +3682,7 @@ SPICE_GNUC_VISIBLE void
> > > spice_server_destroy(SpiceServer *reds)
> > >          g_object_unref(reds->main_dispatcher);
> > >      }
> > >      reds_cleanup_net(reds);
> > > +    sasl_server_done();
> > >      g_clear_object(&reds->agent_dev);
> > >  
> > >      // NOTE: don't replace with g_list_free_full as this function that
> > >      passed callback
> > 
> > Would be great if they document this, see
> > https://www.cyrusimap.org/sasl/sasl/reference/manpages/library/sasl_server_done.html
> > 
> > Note that sasl_server_done is called during spice_server_destroy while
> > sasl_server_done is called during spice_server_init so in case of
> > new + destroy (which should be correct) you just call sasl_server_done
> > without any call to spice_server_init.
> > 
> > Had a brief look at sasl_server_init/sasl_server_done the the counter
> > is not updated atomically, potentially can be racy but is really
> > unlikely and is not up to us to fix it.
> > 
> 
> I agree with the refcount implementation, however at
> https://www.cyrusimap.org/sasl/sasl/reference/manpages/library/sasl_server_init.html#std:saslman-sasl_server_init(3)
> they state
> 
> "sasl_server_init() initializes SASL. It must be called before any calls to sasl_server_start, and only once per process."
> 
> Which seems to indicate the opposite.

Oh well, no big deal, I'm fine with dropping the patch as this does not
show up as 'definitely leaked' in valgrind anyway. 

Christophe
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20180719/26423522/attachment.sig>